From 546510cd7019703b0bbf9d851aab0296155ccfdc Mon Sep 17 00:00:00 2001 From: erwan-joly Date: Fri, 24 Apr 2026 20:36:03 +1200 Subject: [PATCH] fix: deserialize list-of-subpackets with pipe/dot SpecialSeparator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Lists like ClinitPacket (`[PacketListIndex(0, SpecialSeparator = "|")]`) carry items space-separated on the wire, with inner fields joined by "|". The serializer produces this correctly. The deserializer had two bugs: 1. Line 325 picked SpecialSeparator as the list-item separator for IPacket lists, then tried to split the first match token by "|" to count items — turning "5130502|99|68|547|Trippybell" into length=5 instead of reading each whitespace-separated item. This inflated length and indexed past the end of the matches array. 2. Line 356's inner field-split loop looked up the next property's SpecialSeparator, falling back to "." when the sub-packet's properties didn't decorate their own separators. For ClinitSubPacket-style packets that fallback never matches the subpacket content, so the whole item was appended once per property — yielding "X|..X|..X|..X|..X|.." that failed to parse. Fix 1: for IPacket lists, ignore SpecialSeparator as list-item separator — it's the inner field separator. ListSeparator (when set) still wins. Fix 2: the inner field-split loop falls back to the outer list's SpecialSeparator when the sub-packet's properties don't specify one. Unlocks deserialisation of clinit / flinit / kdlinit / skyinit / finit / pinit / blinit / rcblist / rcslist / ninv / mlobjlst / tarank and the inner-field handling of any future list of sub-packets with shared separator. Regression test added on ClinitPacket. Primitive-list handling (stp, stp2, bpp, bpm, exclist, nstest server list) left unchanged — confirmed by full suite passing. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/NosCore.Packets/Deserializer.cs | 18 ++++++++++-- src/NosCore.Packets/NosCore.Packets.csproj | 2 +- .../NosCore.Packets.Tests/DeserializerTest.cs | 29 +++++++++++++++++++ 3 files changed, 46 insertions(+), 3 deletions(-) diff --git a/src/NosCore.Packets/Deserializer.cs b/src/NosCore.Packets/Deserializer.cs index 9aa0618..ad96e16 100644 --- a/src/NosCore.Packets/Deserializer.cs +++ b/src/NosCore.Packets/Deserializer.cs @@ -322,7 +322,13 @@ public IPacket Deserialize(string packetContent) var separator = packetIndexAttribute.SpecialSeparator; if (packetIndexAttribute is PacketListIndexAttribute ind) { - separator = ind.ListSeparator ?? (typeof(IPacket).IsAssignableFrom(subType) ? separator : "."); + // For IPacket (sub-packet) lists, SpecialSeparator on the list names + // the *inner field* separator inside each sub-packet — not the list + // item separator. The list items are tokenised by the outer packet's + // whitespace. Only ListSeparator (when explicitly set) defines how + // items are joined. For primitive lists, SpecialSeparator remains the + // inter-item separator, defaulting to ".". + separator = ind.ListSeparator ?? (typeof(IPacket).IsAssignableFrom(subType) ? null : separator ?? "."); } if (isMaxIndex && string.IsNullOrWhiteSpace(separator)) { @@ -359,9 +365,17 @@ public IPacket Deserialize(string packetContent) var packSeperators = subType.GetProperties() .Select(prop => prop.GetCustomAttribute()).Where(s => s != null).ToList(); + // Fall back to the outer list's SpecialSeparator when the next property + // doesn't override it. Sub-packets like ClinitSubPacket don't decorate + // their own properties, so without this fallback the IndexOf looks for + // "." in strings like "5130502|99|68|547|Trippybell" (not present) and + // every loop iteration appends the whole subpacket — producing + // "X|..X|..X|..X|..X|.." that then fails to parse. + var fieldSeparator = packetIndexAttribute.SpecialSeparator ?? "."; for (var index = 0; index < packSeperators.Count; index++) { - var c = index == packSeperators.Count - 1 ? -1 : subpacket.IndexOf(packSeperators[index + 1]!.SpecialSeparator ?? ".", StringComparison.Ordinal); + var isLast = index == packSeperators.Count - 1; + var c = isLast ? -1 : subpacket.IndexOf(packSeperators[index + 1]!.SpecialSeparator ?? fieldSeparator, StringComparison.Ordinal); if (c == -1) { toConvert += subpacket; diff --git a/src/NosCore.Packets/NosCore.Packets.csproj b/src/NosCore.Packets/NosCore.Packets.csproj index 623103d..b2b7c87 100644 --- a/src/NosCore.Packets/NosCore.Packets.csproj +++ b/src/NosCore.Packets/NosCore.Packets.csproj @@ -12,7 +12,7 @@ https://github.com/NosCoreIO/NosCore.Packets.git nostale, noscore, chickenapi, nostale private server source, nostale emulator - 17.0.0 + 17.1.0 false NosCore's Packets (Nostale packets) defined over classes MIT diff --git a/test/NosCore.Packets.Tests/DeserializerTest.cs b/test/NosCore.Packets.Tests/DeserializerTest.cs index 7bf5544..bb3e018 100644 --- a/test/NosCore.Packets.Tests/DeserializerTest.cs +++ b/test/NosCore.Packets.Tests/DeserializerTest.cs @@ -417,6 +417,35 @@ public void DeserializeWithGuid() Assert.AreEqual("458E0876581FA9A6EFE00A28AA8E75F2", packet.Md5String); } + [TestMethod] + public void PacketWithListOfSubPacketsWithoutPerPropertySeparatorsSplitsCorrectly() + { + // ClinitPacket is `[PacketListIndex(0, SpecialSeparator = "|")]` over + // ClinitSubPacket whose five properties don't decorate their own + // SpecialSeparator. Prior to 17.1.0 the list-of-sub-packets path + // fell back to "." when looking for each field boundary, which + // wasn't present, so the full sub-packet was appended once per + // property — yielding "X|..X|..X|..X|..X|.." that then failed to parse. + var deserializer = new Deserializer(new[] + { + typeof(ClinitPacket), + typeof(ClinitSubPacket), + }); + + var packet = (ClinitPacket)deserializer.Deserialize( + "clinit 5130502|99|68|547|Trippybell 7170218|99|94|325|Hamu"); + + Assert.IsNotNull(packet.SubPackets); + Assert.HasCount(2, packet.SubPackets!); + Assert.AreEqual(5130502L, packet.SubPackets[0]!.CharacterId); + Assert.AreEqual((byte)99, packet.SubPackets[0]!.Level); + Assert.AreEqual((byte)68, packet.SubPackets[0]!.HeroLevel); + Assert.AreEqual(547, packet.SubPackets[0]!.Compliment); + Assert.AreEqual("Trippybell", packet.SubPackets[0]!.CharacterName); + Assert.AreEqual(7170218L, packet.SubPackets[1]!.CharacterId); + Assert.AreEqual("Hamu", packet.SubPackets[1]!.CharacterName); + } + [TestMethod] public void PacketWithEmptySpecialSeparatorSplitsEachCharIntoField() {