feat: Issues sub-tab — flags missing/wrong-structure/wrong-tag packets#6
Merged
erwan-joly merged 7 commits intofeat/packet-log-perf-and-tagsfrom Apr 24, 2026
Merged
Conversation
…ckets
Every captured packet is run through NosCore.Packets's deserializer
against its expected direction, and anomalies are logged in a new
Issues sub-tab under the Packets tab:
- Missing — header not defined anywhere in NosCore.Packets.
- WrongStructure — header defined for this direction but the
deserializer rejects the wire format.
- WrongTag — header is defined only in the opposite-direction
namespace (client header captured as [Server], or
vice versa).
Uses two separate Deserializer instances so the
"server loses to client on duplicate header" dedup inside
Deserializer.Initialize<T> doesn't hide wrong-tag cases.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
NosCore.Packets has classes marked with [PacketHeader] that don't inherit from PacketBase (e.g. ServerPackets.Event.EventPacket). Passing those to Deserializer crashed startup: ArgumentException: GenericArguments[0], 'EventPacket', on 'Void Initialize[T]()' violates the constraint of type 'T'. Add an IsAssignableFrom(PacketBase) filter before constructing the deserializers. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Same context menu + Ctrl+C / Ctrl+A handling the Log listbox has, now on Issues too. Parameterised so the listbox is passed in rather than hard-coded to _logListBox. Copy picks up Raw text for both item types (LoggedPacket.Raw and PacketValidationIssue.Packet.Raw); Copy with tags uses the full ToString() line — including the [Missing] / [Wrong structure] / [Wrong tag] category and the deserializer detail. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Each LoggedPacket carries a nullable ValidationCategory set when the validator flags it. The Log listbox is owner-drawn and paints a 4px left stripe per row: gold for Missing, indian-red for WrongStructure, dark-orange for WrongTag. Selection highlighting still works — the stripe sits over the normal row background. Lets you see problematic packets at a glance while reading the Log, without having to switch to the Issues tab for every packet. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Issues sub-tab gets a read-only TextBox docked at the top listing every packet header that failed validation (unique, sorted). Cleared with the log. - Context menu on Issues drops the raw "Copy" option — only "Copy with tags" makes sense there since the tags carry the category and deserializer detail. Ctrl+C on the Issues listbox now copies with tags; Log listbox behavior is unchanged. - Clear handling consolidated into the PacketLogService.Cleared event so both the Clear button and any future direct clear call empty the log, issues queue, issues listbox, and failed-headers set. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…s away Deserializer.DeserializeValue looks up sub-packet property types by typeof(T).Name in _packetDeserializerDictionary. Sub-packets don't carry [PacketHeader] — my original type filter excluded them, so every parent packet with a sub-packet field failed with e.g. The given key 'InCharacterSubPacket' was not present in the dictionary. Register all PacketBase-derived types without [PacketHeader] on both client and server deserializer instances (they're direction-neutral). Keep the _clientHeaders / _serverHeaders sets populated from header- bearing types only so the direction-match check still works. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Every world-server connection opens with three bare lines from the client before the encrypted packet stream: <sessionId> — single numeric token <account> GF <n> — username / platform / region thisisgfmode — GF-mode marker None of them use the header protocol so NosCore.Packets legitimately doesn't define them. Flagging them as Missing was pure noise on the Issues tab. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
erwan-joly
added a commit
that referenced
this pull request
Apr 24, 2026
* perf: batch packet log UI updates; rename Send/Recv tags to Client/Server Drain captured packets on a 50 ms UI-thread Timer into a single BeginUpdate/AddRange/EndUpdate block instead of BeginInvoke-ing the UI thread once per packet. At high packet rates this was saturating the message loop; the batched path keeps the ListBox responsive. Cache the formatted display string on LoggedPacket so repaints don't re-format per item. Select All now runs inside BeginUpdate/EndUpdate so 5000-item selection no longer fires a paint per SetSelected call. Rename the direction tag from [Send]/[Recv] to [Client]/[Server] — it was ambiguous whether "Send" meant sent by client or sent by server. [Client] now unambiguously marks packets that originated on the game client (client→server) and [Server] marks packets that originated on the server (server→client). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat: Issues sub-tab — flags missing/wrong-structure/wrong-tag packets (#6) * feat: Issues sub-tab detecting Missing / WrongStructure / WrongTag packets Every captured packet is run through NosCore.Packets's deserializer against its expected direction, and anomalies are logged in a new Issues sub-tab under the Packets tab: - Missing — header not defined anywhere in NosCore.Packets. - WrongStructure — header defined for this direction but the deserializer rejects the wire format. - WrongTag — header is defined only in the opposite-direction namespace (client header captured as [Server], or vice versa). Uses two separate Deserializer instances so the "server loses to client on duplicate header" dedup inside Deserializer.Initialize<T> doesn't hide wrong-tag cases. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: filter packet types to PacketBase-derived only NosCore.Packets has classes marked with [PacketHeader] that don't inherit from PacketBase (e.g. ServerPackets.Event.EventPacket). Passing those to Deserializer crashed startup: ArgumentException: GenericArguments[0], 'EventPacket', on 'Void Initialize[T]()' violates the constraint of type 'T'. Add an IsAssignableFrom(PacketBase) filter before constructing the deserializers. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat: Copy / Copy with tags / Select all on the Issues listbox Same context menu + Ctrl+C / Ctrl+A handling the Log listbox has, now on Issues too. Parameterised so the listbox is passed in rather than hard-coded to _logListBox. Copy picks up Raw text for both item types (LoggedPacket.Raw and PacketValidationIssue.Packet.Raw); Copy with tags uses the full ToString() line — including the [Missing] / [Wrong structure] / [Wrong tag] category and the deserializer detail. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat: colored stripe on Log rows that have a validation issue Each LoggedPacket carries a nullable ValidationCategory set when the validator flags it. The Log listbox is owner-drawn and paints a 4px left stripe per row: gold for Missing, indian-red for WrongStructure, dark-orange for WrongTag. Selection highlighting still works — the stripe sits over the normal row background. Lets you see problematic packets at a glance while reading the Log, without having to switch to the Issues tab for every packet. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat: failed-headers summary on Issues; drop plain Copy on Issues menu - Issues sub-tab gets a read-only TextBox docked at the top listing every packet header that failed validation (unique, sorted). Cleared with the log. - Context menu on Issues drops the raw "Copy" option — only "Copy with tags" makes sense there since the tags carry the category and deserializer detail. Ctrl+C on the Issues listbox now copies with tags; Log listbox behavior is unchanged. - Clear handling consolidated into the PacketLogService.Cleared event so both the Clear button and any future direct clear call empty the log, issues queue, issues listbox, and failed-headers set. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: register sub-packet types so 'key not present in dictionary' goes away Deserializer.DeserializeValue looks up sub-packet property types by typeof(T).Name in _packetDeserializerDictionary. Sub-packets don't carry [PacketHeader] — my original type filter excluded them, so every parent packet with a sub-packet field failed with e.g. The given key 'InCharacterSubPacket' was not present in the dictionary. Register all PacketBase-derived types without [PacketHeader] on both client and server deserializer instances (they're direction-neutral). Keep the _clientHeaders / _serverHeaders sets populated from header- bearing types only so the direction-match check still works. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: skip the three pre-auth login-handshake lines in validator Every world-server connection opens with three bare lines from the client before the encrypted packet stream: <sessionId> — single numeric token <account> GF <n> — username / platform / region thisisgfmode — GF-mode marker None of them use the header protocol so NosCore.Packets legitimately doesn't define them. Flagging them as Missing was pure noise on the Issues tab. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on top of #5 (`feat/packet-log-perf-and-tags`). When that merges, this PR's base will auto-retarget to `master` and the diff will collapse to just the validation feature.
Summary
New Issues sub-tab under the Packets tab. Every captured packet is run through `NosCore.Packets`'s `Deserializer` against its expected direction and flagged when:
Implementation builds two `Deserializer` instances (one for `ClientPackets.` types, one for `ServerPackets.`) so the "server loses to client on duplicate header" dedup inside `Deserializer.Initialize` doesn't mask wrong-tag cases.
Test plan