Skip to content

perf: batch packet-log UI; rename Send/Recv tags to Client/Server#5

Merged
erwan-joly merged 2 commits intomasterfrom
feat/packet-log-perf-and-tags
Apr 24, 2026
Merged

perf: batch packet-log UI; rename Send/Recv tags to Client/Server#5
erwan-joly merged 2 commits intomasterfrom
feat/packet-log-perf-and-tags

Conversation

@erwan-joly
Copy link
Copy Markdown
Contributor

@erwan-joly erwan-joly commented Apr 24, 2026

Summary

  • Perf — batched log drain. Captured packets used to hit the UI thread via one BeginInvoke + one ListBox.Items.Add + one TopIndex update per packet. At high packet rates that saturated the message loop — the log got visibly laggy and Select All could stall the UI for seconds. Now the capture path just enqueues onto a ConcurrentQueue, and a 50 ms UI-thread Timer drains the queue into a single BeginUpdate / AddRange / EndUpdate block.
  • Perf — Select All. 5000-item SetSelected loop used to repaint per call. Wrapped in BeginUpdate / EndUpdate.
  • Perf — display string caching. LoggedPacket.ToString() is called on every repaint per visible row. Formatted string is now computed once in the record's field initializer.
  • UX — rename tags. [Send] / [Recv] were ambiguous (send by whom?). Replaced with [Client] (originated on the game client, i.e. client→server) and [Server] (originated on the server, i.e. server→client).

Test plan

  • Attach to a live client, confirm packet log keeps up during a zone change or mass-mob spawn without the UI locking.
  • Select All on a full 5000-row log — should be instant.
  • Copy (Ctrl+C) and Copy with tags still work; tags now read [World] [Client] / [World] [Server].
  • Clear button drains both the queue and the listbox.

Summary by CodeRabbit

Release Notes

  • New Features

    • Added packet validation with a dedicated Issues tab displaying validation results
    • Enhanced log visualization with color-coded packet categories
  • Improvements

    • Optimized UI performance with batch updates
    • Unified copy and selection functionality across all views

…rver

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>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 24, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 837eb1c0-5ea6-4e10-a730-1a9be02281c8

📥 Commits

Reviewing files that changed from the base of the PR and between 35d7e26 and f38e058.

📒 Files selected for processing (5)
  • src/NosCore.DeveloperTools/Forms/MainForm.cs
  • src/NosCore.DeveloperTools/Models/LoggedPacket.cs
  • src/NosCore.DeveloperTools/Models/PacketValidationIssue.cs
  • src/NosCore.DeveloperTools/Program.cs
  • src/NosCore.DeveloperTools/Services/PacketValidationService.cs

📝 Walkthrough

Walkthrough

This PR introduces packet validation functionality to the developer tools. A new PacketValidationService validates captured packets through reflection-based type analysis and deserialization attempts. The MainForm displays validation issues in a dedicated UI tab, processes packets via a thread-safe queue with timer-based updates, applies owner-drawn rendering, and unifies context menus across both log and issues listboxes.

Changes

Cohort / File(s) Summary
Packet Models
src/NosCore.DeveloperTools/Models/LoggedPacket.cs, src/NosCore.DeveloperTools/Models/PacketValidationIssue.cs
Added Issue property to LoggedPacket; changed display format from Send/Recv to Client/Server; cached string representation. New ValidationCategory enum and PacketValidationIssue record with precomputed formatted display and direction/connection mappings.
Validation Service
src/NosCore.DeveloperTools/Services/PacketValidationService.cs
New service that reflects over PacketBase subclasses to build direction-specific header type collections and instantiates dual Deserializer objects for client/server validation. Returns PacketValidationIssue with Missing, WrongStructure, or WrongTag categories based on deserialization outcomes.
UI & Integration
src/NosCore.DeveloperTools/Forms/MainForm.cs, src/NosCore.DeveloperTools/Program.cs
MainForm refactored to validate packets during intake, queue results in ConcurrentQueue, and flush periodically via timer to UI thread with batch updates. Splits Packets tab into sub-tabs ("Log" and "Issues"), applies owner-drawn rendering with color stripes, unifies context menus and clipboard copy across listboxes. Program.cs wires PacketValidationService dependency into MainForm constructor.

Sequence Diagram

sequenceDiagram
    participant UI as MainForm UI
    participant PLS as PacketLogService
    participant Val as PacketValidationService
    participant Queue as ConcurrentQueue
    participant Timer as Update Timer

    PLS->>UI: PacketLogged event
    activate UI
    UI->>Val: Validate(packet)
    activate Val
    Val->>Val: Reflect on PacketBase types
    Val->>Val: Extract header token
    Val->>Val: Attempt deserialization
    Val-->>UI: PacketValidationIssue or null
    deactivate Val
    UI->>Queue: Enqueue packet & issue
    deactivate UI
    
    Timer->>Queue: Periodic flush
    activate Queue
    Queue->>UI: Batch add to log listbox
    Queue->>UI: Batch add to issues listbox
    Queue->>UI: Update failed headers
    deactivate Queue
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~55 minutes

Poem

🐰 With whiskers twitching, I've built a fine sight,
Packets now validated—wrong tags set right!
Queues dance in thread-safe steps, timer flushes with care,
Issues tab sparkles with colors so fair!
A bunny's delight—validation takes flight! ✨

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/packet-log-perf-and-tags

Comment @coderabbitai help to get the list of available commands and usage tips.

#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>
@erwan-joly erwan-joly merged commit 24e20f8 into master Apr 24, 2026
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant