Skip to content

fix(serializer): key the registry by Type so name-colliding packets serialize #505

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
erwan-joly merged 1 commit into from
Aug 30, 2026
Merged

fix(serializer): key the registry by Type so name-colliding packets serialize #505

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 4 additions & 13 deletions src/NosCore.Packets/Serializer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public class Serializer : ISerializer
{
private const string InjectionKey = "IPacketToInject";

private readonly Dictionary<string, Delegate> _packetSerializerDictionary = new Dictionary<string, Delegate>();
private readonly Dictionary<Type, Delegate> _packetSerializerDictionary = new Dictionary<Type, Delegate>();


public Serializer(IEnumerable<Type> types)
Expand All @@ -35,23 +35,14 @@ public void Initialize<T>() where T : PacketBase
var packetSerializerExpressionFalse = PacketSerializerExpression<T>();
if (packetSerializerExpressionFalse != null)
{
if (_packetSerializerDictionary.ContainsKey(typeof(T).Name))
{
if (typeof(T).Namespace!.Contains("ClientPackets"))
{
return;
}

_packetSerializerDictionary.Remove(typeof(T).Name);
}
_packetSerializerDictionary.Add(typeof(T).Name, packetSerializerExpressionFalse.Compile());
_packetSerializerDictionary[typeof(T)] = packetSerializerExpressionFalse.Compile();
}
}

public string Serialize(IPacket packet)
{
var realType = packet.GetType();
var deg = _packetSerializerDictionary[packet.GetType().Name];
var deg = _packetSerializerDictionary[packet.GetType()];
var fullString = (string)deg.DynamicInvoke(packet, false)!;
if (fullString.Contains(InjectionKey))
{
Expand All @@ -63,7 +54,7 @@ public string Serialize(IPacket packet)
fullString = fullString
.Remove(place, InjectionKey.Length)
.Insert(place,
(string)_packetSerializerDictionary[value!.GetType().Name].DynamicInvoke(value, true)!);
(string)_packetSerializerDictionary[value!.GetType()].DynamicInvoke(value, true)!);
}
}

Expand Down
98 changes: 52 additions & 46 deletions ...s/SerializerCorpusTests.ExpressionTreeSerializerOutputIsPinnedForEveryPacket.verified.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@ WhisperPacket#1 => / s15521
WhisperPacket#7 => / s15674
FamilyDeputyPacket#1 => %Familydeputy s15521
FamilyDeputyPacket#7 => %Familydeputy s15674
FamilyDismissPacket#1 => %Familydismiss s15521
FamilyDismissPacket#7 => %Familydismiss s15674
NosCore.Packets.ClientPackets.Commands.FamilyDismissPacket#1 => %Familydismiss s15521
NosCore.Packets.ClientPackets.Commands.FamilyDismissPacket#7 => %Familydismiss s15674
FamilyHeadPacket#1 => %Familyhead s15521
FamilyHeadPacket#7 => %Familyhead s15674
FamilyInvitePacket#1 => %Familyinvite s15521
Expand All @@ -80,8 +80,8 @@ PartyLocationPacket#1 => $Partylocation s15521
PartyLocationPacket#7 => $Partylocation s15674
TimeSpaceInvitePacket#1 => $Accompany s15521
TimeSpaceInvitePacket#7 => $Accompany s15674
DropPacket#1 => <<THREW ArgumentException>>
DropPacket#7 => <<THREW ArgumentException>>
NosCore.Packets.ClientPackets.Drops.DropPacket#1 => drop 21 44 83 53 21 1 87 55
NosCore.Packets.ClientPackets.Drops.DropPacket#7 => drop 74 75 77 8 43 1 37 53
GetPacket#1 => get 1 44 83 53
GetPacket#7 => get 9 75 77 8
FbPacket#1 => fb s15521
Expand All @@ -106,8 +106,8 @@ CreateFamilyPacket#1 => glmk s15521
CreateFamilyPacket#7 => glmk s15674
FAuthPacket#1 => fauth 1 44 83
FAuthPacket#7 => fauth 2 75 77
FamilyDismissPacket#1 => <<THREW ArgumentException>>
FamilyDismissPacket#7 => <<THREW ArgumentException>>
NosCore.Packets.ClientPackets.Families.FamilyDismissPacket#1 => glrm
NosCore.Packets.ClientPackets.Families.FamilyDismissPacket#7 => glrm
FamilyManagementPacket#1 => fmg 1 44
FamilyManagementPacket#7 => fmg 2 75
FhistCtsPacket#1 => fhis_cts
Expand All @@ -118,8 +118,8 @@ FwsPacket#1 => fws 21
FwsPacket#7 => fws 74
GListPacket#1 => glist 21
GListPacket#7 => glist 74
GidxPacket#1 => <<THREW ArgumentException>>
GidxPacket#7 => <<THREW ArgumentException>>
NosCore.Packets.ClientPackets.Families.GidxPacket#1 => gidx 1 44 83 s15853 s14221 82 0|0
NosCore.Packets.ClientPackets.Families.GidxPacket#7 => gidx 9 75 77 s30408 s14143 82 0|0
JoinFamilyPacket#1 => gjoin 21 44
JoinFamilyPacket#7 => gjoin 74 75
TodayPacket#1 => today_cts
Expand Down Expand Up @@ -200,8 +200,8 @@ MinigamePacket#1 => mg 21 44 83 53 21
MinigamePacket#7 => mg 74 75 77 8 43
RmvobjPacket#1 => rmvobj 21
RmvobjPacket#7 => rmvobj 74
UseObjPacket#1 => <<THREW ArgumentException>>
UseObjPacket#7 => <<THREW ArgumentException>>
NosCore.Packets.ClientPackets.Miniland.UseObjPacket#1 => useobj s15521 44
NosCore.Packets.ClientPackets.Miniland.UseObjPacket#7 => useobj s15674 75
ClientDirPacket#1 => dir 21 9 83
ClientDirPacket#7 => dir 74 0 77
PreqPacket#1 => preq 21
Expand All @@ -226,8 +226,8 @@ BpClosePacket#1 => bp_close
BpClosePacket#7 => bp_close
BpOpenPacket#1 => bp_open
BpOpenPacket#7 => bp_open
BpmPacket#1 => <<THREW ArgumentException>>
BpmPacket#7 => <<THREW ArgumentException>>
NosCore.Packets.ClientPackets.Player.BpmPacket#1 => bpm 21 1 83 53 21 82 20 1 52 90 80 24 27 24 0 90 20 18 35
NosCore.Packets.ClientPackets.Player.BpmPacket#7 => bpm 74 0 77 8 43 82 14 0 78 12 53 59 96 14 1 58 74 95 71
BpoPacket#1 => bpo
BpoPacket#7 => bpo
BppPacket#1 => bpp 21 44 0 53 21 82 87 55 1 1 1 24 27 23 69 90 1 1 0
Expand All @@ -238,10 +238,10 @@ ComplPacket#1 => compl 21
ComplPacket#7 => compl 74
EquipmentInfoPacket#1 => eqinfo 21 44 83 53
EquipmentInfoPacket#7 => eqinfo 74 75 77 8
NpInfoPacket#1 => <<THREW ArgumentException>>
NpInfoPacket#7 => <<THREW ArgumentException>>
RsfiPacket#1 => <<THREW ArgumentException>>
RsfiPacket#7 => <<THREW ArgumentException>>
NosCore.Packets.ClientPackets.Player.NpInfoPacket#1 => npinfo 21
NosCore.Packets.ClientPackets.Player.NpInfoPacket#7 => npinfo 74
NosCore.Packets.ClientPackets.Player.RsfiPacket#1 => rsfi 21 44 83 53 21 82
NosCore.Packets.ClientPackets.Player.RsfiPacket#7 => rsfi 74 75 77 8 43 82
SnapPacket#1 => snap
SnapPacket#7 => snap
TitEqPacket#1 => tit_eq 21 44
Expand Down Expand Up @@ -282,8 +282,8 @@ SellListPacket#1 => sell_list 21 44.83.53 21.82.87
SellListPacket#7 => sell_list 74 75.77.8 43.82.37
SellPacket#1 => sell 21 44 83
SellPacket#7 => sell 74 75 77
ShopClosePacket#1 => <<THREW ArgumentException>>
ShopClosePacket#7 => <<THREW ArgumentException>>
NosCore.Packets.ClientPackets.Shops.ShopClosePacket#1 => shopclose
NosCore.Packets.ClientPackets.Shops.ShopClosePacket#7 => shopclose
ShoppingPacket#1 => shopping 21 44 3 53
ShoppingPacket#7 => shopping 74 75 2 8
SCarrierPacket#1 => s_carrier 21 44 83
Expand All @@ -292,10 +292,10 @@ SpTransformPacket#1 => sl 10 44 83 53 21 82
SpTransformPacket#7 => sl 10 75 77 8 43 82
GopPacket#1 => gop 1 1
GopPacket#7 => gop 1 0
GuriPacket#1 => <<THREW ArgumentException>>
GuriPacket#7 => <<THREW ArgumentException>>
MallPacket#1 => <<THREW ArgumentException>>
MallPacket#7 => <<THREW ArgumentException>>
NosCore.Packets.ClientPackets.UI.GuriPacket#1 => guri 6 44 83 53 s14221
NosCore.Packets.ClientPackets.UI.GuriPacket#7 => guri 40 75 77 8 s14143
NosCore.Packets.ClientPackets.UI.MallPacket#1 => mall 21
NosCore.Packets.ClientPackets.UI.MallPacket#7 => mall 74
DepositPacket#1 => deposit 1 44 83 53 0
DepositPacket#7 => deposit 2 75 77 8 0
FDepositPacket#1 => f_deposit 1 44 83 53 21
Expand Down Expand Up @@ -344,12 +344,16 @@ CtPacket#1 => ct 1 44 3 53 21 82 87
CtPacket#7 => ct 9 75 2 8 43 82 37
DiePacket#1 => die 1 44 3 53
DiePacket#7 => die 9 75 2 8
DmPacket#1 => dm 1 44 83 3
DmPacket#7 => dm 9 75 77 3
MscPacket#1 => ms_c 21
MscPacket#7 => ms_c 74
MslotPacket#1 => mslot 21 0
MslotPacket#7 => mslot 74 -1
RankCoolPacket#1 => rank_cool 21 44 83
RankCoolPacket#7 => rank_cool 74 75 77
RcPacket#1 => rc 1 44 83 53
RcPacket#7 => rc 9 75 77 8
RevivePacket#1 => revive 1 44 83
RevivePacket#7 => revive 9 75 77
SkillResetPacket#1 => sr 21
Expand Down Expand Up @@ -414,8 +418,8 @@ CatPacket#1 => ca_t 21 44
CatPacket#7 => ca_t 74 75
CharScPacket#1 => char_sc 1 44 83
CharScPacket#7 => char_sc 9 75 77
DropPacket#1 => drop 21 44 83 53 21 1 87
DropPacket#7 => drop 74 75 77 8 43 1 37
NosCore.Packets.ServerPackets.Entities.DropPacket#1 => drop 21 44 83 53 21 1 87
NosCore.Packets.ServerPackets.Entities.DropPacket#7 => drop 74 75 77 8 43 1 37
MovePacket#1 => mv 1 44 83 53 21
MovePacket#7 => mv 9 75 77 8 43
OutPacket#1 => out 1 44
Expand All @@ -432,8 +436,12 @@ DgPacket#1 => dg 21 44 83 53
DgPacket#7 => dg 74 75 77 8
EsfPacket#1 => esf 21
EsfPacket#7 => esf 74
EventPacket#1 => evnt 10 1 83 53
EventPacket#7 => evnt 10 0 77 8
EventbPacket#1 => evtb 21 44 83 53 21 1 87 55 52 90 80 24 27 23 69 90 20 18 35 9 75 68 79 97 1 68 41 93
EventbPacket#7 => evtb 74 75 77 8 43 1 37 53 78 12 53 59 96 45 47 58 74 95 71 58 9 5 81 27 96 55 30 7
FbtPacket#1 => fbt 21 44 83 53
FbtPacket#7 => fbt 74 75 77 8
RaidPacket#1 => raid 21 44
RaidPacket#7 => raid 74 75
RaidbfPacket#1 => raidbf 21 44 83
Expand All @@ -460,8 +468,8 @@ GInfoPacket#1 => ginfo s15521 s30144 1 53 21 82 87 55 0 1 1 1 0 2 0 1 s920
GInfoPacket#7 => ginfo s15674 s275 1 8 43 82 37 53 2 1 0 0 1 0 0 1 s28374
GexpPacket#1 => gexp 21|44 83|53
GexpPacket#7 => gexp 74|75 77|8
GidxPacket#1 => gidx 1 44 83 s15853 21 1|0
GidxPacket#7 => gidx 9 75 77 s30408 43 1|0
NosCore.Packets.ServerPackets.Families.GidxPacket#1 => gidx 1 44 83 s15853 21 1|0
NosCore.Packets.ServerPackets.Families.GidxPacket#7 => gidx 9 75 77 s30408 43 1|0
GmsgPacket#1 => gmsg 21|s30144 83|s15853
GmsgPacket#7 => gmsg 74|s275 77|s30408
FtptPacket#1 => ftpt 21
Expand Down Expand Up @@ -570,8 +578,8 @@ MlobjPacket#1 => mlobj 0 44 83 53 21 82 87 55 1 1
MlobjPacket#7 => mlobj 1 75 77 8 43 82 37 53 1 1
MlobjlstPacket#1 => mlobjlst 21.1.83.53.21.82.87.55.1.1 80.1.27.23.69.90.20.18.0.0
MlobjlstPacket#7 => mlobjlst 74.0.77.8.43.82.37.53.1.1 53.0.96.45.47.58.74.95.0.1
UseObjPacket#1 => useobj s15521 44
UseObjPacket#7 => useobj s15674 75
NosCore.Packets.ServerPackets.Miniland.UseObjPacket#1 => useobj s15521 44
NosCore.Packets.ServerPackets.Miniland.UseObjPacket#7 => useobj s15674 75
DirPacket#1 => dir 1 44 83
DirPacket#7 => dir 9 75 77
NpcReqPacket#1 => npc_req 1 44 83
Expand All @@ -584,8 +592,8 @@ PstPacket#1 => pst 1 44 83 53 21 82 87 55 0 90 80.24
PstPacket#7 => pst 9 75 77 8 43 82 37 53 0 12 53.59
BnPacket#1 => bn 21 s30144
BnPacket#7 => bn 74 s275
BpmPacket#1 => bpm 21 44 83
BpmPacket#7 => bpm 74 75 77
NosCore.Packets.ServerPackets.Player.BpmPacket#1 => bpm 21 44 83
NosCore.Packets.ServerPackets.Player.BpmPacket#7 => bpm 74 75 77
CInfoPacket#1 => c_info s15521 s30144 83 53 s14221 82 -1 1 2 90 0 24 27 23 0 90 20 1 35 9
CInfoPacket#7 => c_info s15674 s275 77 8 s14143 82 1 1 3 24 3 59 96 45 0 58 74 0 71 58
CInfoResetPacket#1 => c_info_reset
Expand All @@ -610,16 +618,14 @@ LevPacket#1 => lev 21 44 83 53 21 82 87 55 52 90 80 24
LevPacket#7 => lev 74 75 77 8 43 82 37 53 78 12 53 59
LevelUpPacket#1 => levelup 21
LevelUpPacket#7 => levelup 74
NpInfoPacket#1 => npinfo 21
NpInfoPacket#7 => npinfo 74
NosCore.Packets.ServerPackets.Player.NpInfoPacket#1 => npinfo 21
NosCore.Packets.ServerPackets.Player.NpInfoPacket#7 => npinfo 74
PSexPacket#1 => p_sex 1
PSexPacket#7 => p_sex 0
RagePacket#1 => rage 21 44
RagePacket#7 => rage 74 75
RcPacket#1 => rc 21 44 83 53
RcPacket#7 => rc 74 75 77 8
RsfiPacket#1 => rsfi 21 44 83 53 21 82
RsfiPacket#7 => rsfi 74 75 77 8 43 82
NosCore.Packets.ServerPackets.Player.RsfiPacket#1 => rsfi 21 44 83 53 21 82
NosCore.Packets.ServerPackets.Player.RsfiPacket#7 => rsfi 74 75 77 8 43 82
ScPacket#1 => sc 21 44 83 53 21 82 87 55 52 90 80 24 27 23 69 90 20 18 35 9 75 68 79 97
ScPacket#7 => sc 74 75 77 8 43 82 37 53 78 12 53 59 96 45 47 58 74 95 71 58 9 5 81 27
ScrPacket#1 => scr 21 44 83 53 21 82
Expand Down Expand Up @@ -734,8 +740,8 @@ DlgiPacket#1 => dlgi #gbox^1^44^0 #gbox^1^21^1 2167 55 0
DlgiPacket#7 => dlgi #gbox^2^75^0 #gbox^0^43^1 2157 53 0
FishOpenPacket#1 => fishopen
FishOpenPacket#7 => fishopen
GuriPacket#1 => guri 6 44 83 53 21 82
GuriPacket#7 => guri 40 75 77 8 43 82
NosCore.Packets.ServerPackets.UI.GuriPacket#1 => guri 6 44 83 53 21 82
NosCore.Packets.ServerPackets.UI.GuriPacket#7 => guri 40 75 77 8 43 82
IconPacket#1 => icon 1 44 83 53
IconPacket#7 => icon 9 75 77 8
InfoPacket#1 => info s15521
Expand All @@ -744,8 +750,8 @@ Infoi2Packet#1 => infoi2 1371 44 0
Infoi2Packet#7 => infoi2 1524 75 0
InfoiPacket#1 => infoi 1371 0 0 44
InfoiPacket#7 => infoi 1524 0 0 75
MallPacket#1 => mall 21
MallPacket#7 => mall 74
NosCore.Packets.ServerPackets.UI.MallPacket#1 => mall 21
NosCore.Packets.ServerPackets.UI.MallPacket#7 => mall 74
ModalPacket#1 => modal 21 s30144
ModalPacket#7 => modal 74 s275
ModaliPacket#1 => modali 21 1844 83 0 0
Expand All @@ -766,8 +772,8 @@ QnaiPacket#1 => qnai #gbox^1^44^0 1703 21 0 0
QnaiPacket#7 => qnai #gbox^2^75^0 2108 43 0 0
RdiPacket#1 => rdi 21 44
RdiPacket#7 => rdi 74 75
ShopClosePacket#1 => shopclose 21
ShopClosePacket#7 => shopclose 74
NosCore.Packets.ServerPackets.UI.ShopClosePacket#1 => shopclose 21
NosCore.Packets.ServerPackets.UI.ShopClosePacket#7 => shopclose 74
TARankPacket#1 => ta_rank 21.44.83.s15853.1.0.87 55.52.90.s13380.4.1.23 s30369
TARankPacket#7 => ta_rank 74.75.77.s30408.3.0.37 53.78.12.s27953.4.0.45 s21847
TeamArenaClosePacket#1 => ta_close
Expand All @@ -788,10 +794,10 @@ FStashAllPacket#1 => f_stash_all 21 44.83.53.21.82.87.55 52.90.80.24.27.23.69
FStashAllPacket#7 => f_stash_all 74 75.77.8.43.82.37.53 78.12.53.59.96.45.47
FStashClientPacket#1 => f_stash 21.44.83.53.21.82.87 55.52.90.80.24.27.23
FStashClientPacket#7 => f_stash 74.75.77.8.43.82.37 53.78.12.53.59.96.45
PStashAllPacket#1 => p_stash_all 21 44.83.53.21.82.87.55 52.90.80.24.27.23.69
PStashAllPacket#7 => p_stash_all 74 75.77.8.43.82.37.53 78.12.53.59.96.45.47
PStashClientPacket#1 => p_stash 21.44.83.53.21.82.87 55.52.90.80.24.27.23
PStashClientPacket#7 => p_stash 74.75.77.8.43.82.37 53.78.12.53.59.96.45
PStashAllPacket#1 => pstash_all 21 44.83.53.21.82.87.55 52.90.80.24.27.23.69
PStashAllPacket#7 => pstash_all 74 75.77.8.43.82.37.53 78.12.53.59.96.45.47
PStashClientPacket#1 => pstash 21.44.83.53.21.82.87 55.52.90.80.24.27.23
PStashClientPacket#7 => pstash 74.75.77.8.43.82.37 53.78.12.53.59.96.45
StashAllPacket#1 => stash_all 21 44.83.53.21.82.87.55 52.90.80.24.27.23.69
StashAllPacket#7 => stash_all 74 75.77.8.43.82.37.53 78.12.53.59.96.45.47
StashClientPacket#1 => stash 21.44.83.53.21.82.87 55.52.90.80.24.27.23
Expand Down
35 changes: 14 additions & 21 deletions test/NosCore.Packets.Tests/SerializerCorpusTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,23 +47,17 @@ public Task ExpressionTreeSerializerOutputIsPinnedForEveryPacket()
private static string SnapshotDirectory => Path.Combine(
Directory.GetCurrentDirectory(), "..", "..", "..");

// Serializer keys _packetSerializerDictionary by typeof(T).Name - the SIMPLE
// name - so two packets sharing a class name across namespaces overwrite each
// other, and Initialize resolves the clash by letting the non-ClientPackets one
// win. Serializing the loser then invokes the winner's delegate and throws
// ArgumentException. Each server currently registers a filtered subset of packet
// types, which is why this has not bitten in production.
//
// Pinned rather than fixed here: generated per-type methods cannot collide by
// simple name, so porting to the generator removes the failure mode outright.
// Ten packet class names exist in both a ClientPackets and a ServerPackets
// namespace. The serializer registry is keyed by Type, so both variants keep
// their own delegate; this pin only guards against new accidental duplicates.
private static readonly string[] KnownSimpleNameCollisions =
{
"BpmPacket", "DropPacket", "FamilyDismissPacket", "GidxPacket", "GuriPacket",
"MallPacket", "NpInfoPacket", "RsfiPacket", "ShopClosePacket", "UseObjPacket"
};

[TestMethod]
public void OnlyTheKnownSimpleNameCollisionsFailToSerialize()
public void EveryPacketSerializes()
{
var failures = new List<string>();

Expand All @@ -75,18 +69,13 @@ public void OnlyTheKnownSimpleNameCollisionsFailToSerialize()
}
catch (Exception ex)
{
failures.Add($"{type.Name}: {ex.GetType().Name} {ex.Message}");
failures.Add($"{type.FullName}: {ex.GetType().Name} {ex.Message}");
}
}

var unexpected = failures
.Where(f => !KnownSimpleNameCollisions.Any(known =>
f.StartsWith(known + ":", StringComparison.Ordinal)))
.ToList();

Assert.AreEqual(0, unexpected.Count,
$"{unexpected.Count} packets threw for a reason other than the known name " +
$"collisions:{Environment.NewLine}{string.Join(Environment.NewLine, unexpected)}");
Assert.AreEqual(0, failures.Count,
$"{failures.Count} packets failed to serialize:" +
$"{Environment.NewLine}{string.Join(Environment.NewLine, failures)}");
}

[TestMethod]
Expand All @@ -107,11 +96,15 @@ public void CollidingPacketNamesAreStillDistinctTypes()

// Two seeds per packet: one exercises the populated path, the other shifts every
// value so an off-by-one in separator or optional-run handling cannot hide behind
// a coincidence.
// a coincidence. Colliding simple names are disambiguated by namespace so each
// snapshot line maps to exactly one type.
private static IEnumerable<string> Corpus()
{
foreach (var type in PacketCorpus.PacketTypes)
{
var id = KnownSimpleNameCollisions.Contains(type.Name)
? $"{type.Namespace}.{type.Name}"
: type.Name;
foreach (var seed in new[] { 1, 7 })
{
string result;
Expand All @@ -124,7 +117,7 @@ private static IEnumerable<string> Corpus()
result = $"<<THREW {ex.GetType().Name}>>";
}

yield return $"{type.Name}#{seed} => {result}";
yield return $"{id}#{seed} => {result}";
}
}
}
Expand Down
Loading