Skip to content

Commit 3113ea0

Browse files
Jarno458Jarno458
committed
Refactoring + fixes for those refactorings
1 parent 5a60895 commit 3113ea0

File tree

8 files changed

+90
-31
lines changed

8 files changed

+90
-31
lines changed

‎Archipelago.MultiClient.Net.Tests/ItemInfoFixture.cs

+3-3
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,7 @@ public void Scout_should_be_related_when_player_is_active_player()
217217
Player = 1
218218
}, "Hollow Knight", "Hollow Knight", itemInfoResolver, playerHelper, activePlayer);
219219

220-
Assert.That(scoutedItem.IsRelatedToActivePlayer, Is.True);
220+
Assert.That(scoutedItem.IsReceiverRelatedToActivePlayer, Is.True);
221221
}
222222

223223
[Test]
@@ -247,7 +247,7 @@ public void Scout_should_be_related_when_player_is_relevant_group()
247247
Player = 3
248248
}, "Hollow Knight", "Hollow Knight", itemInfoResolver, playerHelper, group);
249249

250-
Assert.That(scoutedItem.IsRelatedToActivePlayer, Is.True);
250+
Assert.That(scoutedItem.IsReceiverRelatedToActivePlayer, Is.True);
251251
}
252252

253253
[Test]
@@ -278,7 +278,7 @@ public void Scout_should_not_be_related_when_player_is_shared_itemlink()
278278
Player = 2
279279
}, "Hollow Knight", "Hollow Knight", itemInfoResolver, playerHelper, otherPlayer);
280280

281-
Assert.That(scoutedItem.IsRelatedToActivePlayer, Is.False);
281+
Assert.That(scoutedItem.IsReceiverRelatedToActivePlayer, Is.False);
282282
}
283283

284284
static IArchipelagoSession CreateTestSession(bool scouted, string remotePlayerGame, string myGame, SerializableItemInfo itemInfo)

‎Archipelago.MultiClient.Net.Tests/LocationCheckHelperFixture.cs

-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
using System.Collections.Generic;
99
using System.Collections.ObjectModel;
1010
using System.Linq;
11-
using System.Net.Sockets;
1211
using System.Threading;
1312
using System.Threading.Tasks;
1413

‎Archipelago.MultiClient.Net.Tests/MessageLogHelperFixture.cs

+7-4
Original file line numberDiff line numberDiff line change
@@ -360,17 +360,20 @@ public void Should_preserve_extra_properties_on_ItemPrintJsonPacket()
360360
[Test]
361361
public void Should_preserve_extra_properties_on_ItemCheatPrintJsonPacket()
362362
{
363+
var playerColleciton = GetPlayerCollection(new List<PlayerInfo> {
364+
new PlayerInfo { Team = 0, Slot = 1, Game = "Game1" },
365+
new PlayerInfo { Team = 0, Slot = 2, Game = "Game2" },
366+
});
367+
363368
var socket = Substitute.For<IArchipelagoSocketHelper>();
364369
var itemInfoResolver = Substitute.For<IItemInfoResolver>();
365370
itemInfoResolver.GetItemName(Arg.Any<long>(), Arg.Any<string>()).Returns(_ => null);
366371
itemInfoResolver.GetLocationName(Arg.Any<long>(), Arg.Any<string>()).Returns(_ => null);
367372
var players = Substitute.For<IPlayerHelper>();
368373
players.GetPlayerAlias(2).Returns("LocalPlayer");
369-
players.Players.Returns(GetPlayerCollection(new List<PlayerInfo> {
370-
new PlayerInfo { Team = 0, Slot = 1, Game = "Game1" },
371-
new PlayerInfo { Team = 0, Slot = 2, Game = "Game2" },
372-
}));
374+
players.Players.Returns(playerColleciton);
373375
players.GetPlayerInfo(Arg.Any<int>(), Arg.Any<int>()).Returns(x => players.Players[x.ArgAt<int>(0)][x.ArgAt<int>(1)]);
376+
players.ActivePlayer.Returns(playerColleciton[0][2]);
374377
var connectionInfo = Substitute.For<IConnectionInfoProvider>();
375378
connectionInfo.Team.Returns(0);
376379
connectionInfo.Slot.Returns(2);

‎Archipelago.MultiClient.Net.Tests/PlayerInfoFixture.cs

+54
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
using Archipelago.MultiClient.Net.Helpers;
2+
using NUnit.Framework;
3+
4+
namespace Archipelago.MultiClient.Net.Tests
5+
{
6+
[TestFixture]
7+
class PlayerInfoFixture
8+
{
9+
[Test]
10+
public void IsRelatedTo_should_work_both_ways()
11+
{
12+
var PlayerA = new PlayerInfo { Team = 0, Slot = 1 };
13+
var PlayerB = new PlayerInfo { Team = 0, Slot = 2 };
14+
var GroupC = new PlayerInfo { Team = 0, Slot = 3, GroupMembers = [1,2] };
15+
16+
Assert.IsTrue(PlayerA.IsRelatedTo(PlayerA));
17+
Assert.IsTrue(PlayerB.IsRelatedTo(PlayerB));
18+
Assert.IsTrue(GroupC.IsRelatedTo(GroupC));
19+
20+
Assert.IsTrue(PlayerA.IsRelatedTo(GroupC));
21+
Assert.IsTrue(GroupC.IsRelatedTo(PlayerA));
22+
23+
Assert.IsTrue(PlayerB.IsRelatedTo(GroupC));
24+
Assert.IsTrue(GroupC.IsRelatedTo(PlayerB));
25+
26+
Assert.IsFalse(PlayerA.IsRelatedTo(PlayerB));
27+
Assert.IsFalse(PlayerB.IsRelatedTo(PlayerA));
28+
}
29+
30+
[Test]
31+
public void IsRelatedTo_should_not_work_cross_teams()
32+
{
33+
var Player = new PlayerInfo { Team = 0, Slot = 1 };
34+
var Group = new PlayerInfo { Team = 1, Slot = 2, GroupMembers = [1] };
35+
36+
Assert.IsFalse(Player.IsRelatedTo(Group));
37+
Assert.IsFalse(Group.IsRelatedTo(Player));
38+
}
39+
40+
[Test]
41+
public void Equality_should_check_slot_and_team()
42+
{
43+
var PlayerA = new PlayerInfo { Team = 0, Slot = 1, Name = "Player", Game = "GameOne" };
44+
var PlayerB = new PlayerInfo { Team = 0, Slot = 1, Name = "Different Player Name has no impact", GroupMembers = [1], Game = "Game2" };
45+
var PlayerC = new PlayerInfo { Team = 1, Slot = 1, Name = "Player", Game = "GameOne" };
46+
var PlayerD = new PlayerInfo { Team = 0, Slot = 2 };
47+
48+
Assert.That(PlayerA, Is.EqualTo(PlayerB));
49+
Assert.That(PlayerA, Is.Not.EqualTo(PlayerC));
50+
Assert.That(PlayerA, Is.Not.EqualTo(PlayerD));
51+
Assert.That((PlayerInfo)null, Is.EqualTo(null));
52+
}
53+
}
54+
}

‎Archipelago.MultiClient.Net/Helpers/PlayerHelper.cs

+12
Original file line numberDiff line numberDiff line change
@@ -291,6 +291,18 @@ public IEnumerable<PlayerInfo> GetGroupMembers(IPlayerHelper playerHelper)
291291
return GroupMembers.Select(g => playerHelper.GetPlayerInfo(Team, g));
292292
}
293293

294+
/// <summary>
295+
/// True if this player is the same player, or if either player is a group (e.g. itemlinks) that contains the other player
296+
/// </summary>
297+
/// <param name="other"></param>
298+
/// <returns></returns>
299+
public bool IsRelatedTo(PlayerInfo other) => this == other
300+
|| (Team == other.Team
301+
&& (
302+
other.GroupMembers != null && other.GroupMembers.Contains(this)
303+
|| (GroupMembers != null && GroupMembers.Contains(other))
304+
));
305+
294306
/// <summary>
295307
/// Converts the PlayerInfo to the slot
296308
/// </summary>

‎Archipelago.MultiClient.Net/MessageLog/Messages/ItemSendLogMessage.cs

+6-11
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
using Archipelago.MultiClient.Net.Helpers;
33
using Archipelago.MultiClient.Net.MessageLog.Parts;
44
using Archipelago.MultiClient.Net.Models;
5-
using System.Linq;
65

76
namespace Archipelago.MultiClient.Net.MessageLog.Messages
87
{
@@ -15,6 +14,8 @@ namespace Archipelago.MultiClient.Net.MessageLog.Messages
1514
/// </summary>
1615
public class ItemSendLogMessage : LogMessage
1716
{
17+
PlayerInfo ActivePlayer { get; }
18+
1819
/// <summary>
1920
/// The player who received the item
2021
/// </summary>
@@ -28,17 +29,17 @@ public class ItemSendLogMessage : LogMessage
2829
/// <summary>
2930
/// Checks if the Receiver is the current connected player
3031
/// </summary>
31-
public bool IsReceiverTheActivePlayer { get; }
32+
public bool IsReceiverTheActivePlayer => Receiver == ActivePlayer;
3233

3334
/// <summary>
3435
/// True if the Sender is the current connected player
3536
/// </summary>
36-
public bool IsSenderTheActivePlayer { get; }
37+
public bool IsSenderTheActivePlayer => Sender == ActivePlayer;
3738

3839
/// <summary>
3940
/// True if either the Receiver or Sender share any slot groups (e.g. itemlinks) with the current connected player
4041
/// </summary>
41-
public bool IsRelatedToActivePlayer { get; }
42+
public bool IsRelatedToActivePlayer => ActivePlayer.IsRelatedTo(Receiver) || ActivePlayer.IsRelatedTo(Sender);
4243

4344
/// <summary>
4445
/// The Item that was send
@@ -57,17 +58,11 @@ internal ItemSendLogMessage(MessagePart[] parts,
5758
int receiver, int sender, NetworkItem item, int team,
5859
IItemInfoResolver itemInfoResolver) : base(parts)
5960
{
61+
ActivePlayer = players.ActivePlayer ?? new PlayerInfo();
6062
Receiver = players.GetPlayerInfo(team, receiver) ?? new PlayerInfo();
6163
Sender = players.GetPlayerInfo(team, sender) ?? new PlayerInfo();
6264
var itemPlayer = players.GetPlayerInfo(team, item.Player) ?? new PlayerInfo();
6365

64-
IsReceiverTheActivePlayer = Receiver == players.ActivePlayer;
65-
IsSenderTheActivePlayer = Sender == players.ActivePlayer;
66-
67-
IsRelatedToActivePlayer = IsReceiverTheActivePlayer || IsSenderTheActivePlayer
68-
|| (Receiver?.GetGroupMembers(players)?.Contains(players.ActivePlayer) ?? false)
69-
|| (Sender?.GetGroupMembers(players)?.Contains(players.ActivePlayer) ?? false);
70-
7166
Item = new ItemInfo(item, Receiver.Game, Sender.Game, itemInfoResolver, itemPlayer);
7267
}
7368
}

‎Archipelago.MultiClient.Net/MessageLog/Messages/PlayerSpecificLogMessage.cs

+5-6
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
using Archipelago.MultiClient.Net.Helpers;
22
using Archipelago.MultiClient.Net.MessageLog.Parts;
3-
using System.Linq;
43

54
namespace Archipelago.MultiClient.Net.MessageLog.Messages
65
{
@@ -12,6 +11,8 @@ namespace Archipelago.MultiClient.Net.MessageLog.Messages
1211
/// </summary>
1312
public abstract class PlayerSpecificLogMessage : LogMessage
1413
{
14+
PlayerInfo ActivePlayer { get; }
15+
1516
/// <summary>
1617
/// The player information about the player this message is concerning
1718
/// </summary>
@@ -20,21 +21,19 @@ public abstract class PlayerSpecificLogMessage : LogMessage
2021
/// <summary>
2122
/// True if the player this message is concerning is the current connected player
2223
/// </summary>
23-
public bool IsActivePlayer { get; }
24+
public bool IsActivePlayer => Player == ActivePlayer;
2425

2526
/// <summary>
2627
/// True if the player this message is concerning any slot groups (e.g. itemlinks) with the current connected player
2728
/// </summary>
28-
public bool IsRelatedToActivePlayer { get; }
29+
public bool IsRelatedToActivePlayer => ActivePlayer.IsRelatedTo(Player);
2930

3031
internal PlayerSpecificLogMessage(MessagePart[] parts,
3132
IPlayerHelper players, int team, int slot)
3233
: base(parts)
3334
{
35+
ActivePlayer = players.ActivePlayer ?? new PlayerInfo();
3436
Player = players.GetPlayerInfo(team, slot) ?? new PlayerInfo();
35-
IsActivePlayer = Player == players.ActivePlayer;
36-
IsRelatedToActivePlayer = IsActivePlayer
37-
|| (Player.GetGroupMembers(players)?.Contains(players.ActivePlayer) ?? false);
3837
}
3938
}
4039
}

‎Archipelago.MultiClient.Net/Models/ItemInfo.cs

+3-6
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
using Archipelago.MultiClient.Net.DataPackage;
22
using Archipelago.MultiClient.Net.Enums;
33
using Archipelago.MultiClient.Net.Helpers;
4-
using System.Linq;
54

65
namespace Archipelago.MultiClient.Net.Models
76
{
@@ -108,9 +107,9 @@ public class ScoutedItemInfo : ItemInfo
108107
public new PlayerInfo Player => base.Player;
109108

110109
/// <summary>
111-
/// True if the player this message is concerning any slot groups (e.g. itemlinks) with the current connected player
110+
/// True if the receiver is concerning any slot groups (e.g. itemlinks) with the current connected player
112111
/// </summary>
113-
public bool IsRelatedToActivePlayer { get; }
112+
public bool IsReceiverRelatedToActivePlayer { get; }
114113

115114
/// <summary>
116115
/// The constructor what else did you expect it to be
@@ -119,9 +118,7 @@ public ScoutedItemInfo(NetworkItem item, string receiverGame, string senderGame,
119118
IPlayerHelper players, PlayerInfo player)
120119
: base(item, receiverGame, senderGame, itemInfoResolver, player)
121120
{
122-
var activePlayer = players.ActivePlayer;
123-
IsRelatedToActivePlayer = player == activePlayer
124-
|| (player?.GetGroupMembers(players)?.Contains(activePlayer) ?? false);
121+
IsReceiverRelatedToActivePlayer = (players.ActivePlayer ?? new PlayerInfo()).IsRelatedTo(player);
125122
}
126123
}
127124
}

0 commit comments

Comments
 (0)