From f38a16cab125905fdec801e92bd192852f2479db Mon Sep 17 00:00:00 2001 From: erwan-joly Date: Fri, 24 Apr 2026 15:31:34 +1200 Subject: [PATCH 1/7] feat: Issues sub-tab detecting Missing / WrongStructure / WrongTag packets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 doesn't hide wrong-tag cases. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/NosCore.DeveloperTools/Forms/MainForm.cs | 64 ++++++++--- .../Models/PacketValidationIssue.cs | 33 ++++++ src/NosCore.DeveloperTools/Program.cs | 3 +- .../Services/PacketValidationService.cs | 101 ++++++++++++++++++ 4 files changed, 186 insertions(+), 15 deletions(-) create mode 100644 src/NosCore.DeveloperTools/Models/PacketValidationIssue.cs create mode 100644 src/NosCore.DeveloperTools/Services/PacketValidationService.cs diff --git a/src/NosCore.DeveloperTools/Forms/MainForm.cs b/src/NosCore.DeveloperTools/Forms/MainForm.cs index 2c33711..820fee7 100644 --- a/src/NosCore.DeveloperTools/Forms/MainForm.cs +++ b/src/NosCore.DeveloperTools/Forms/MainForm.cs @@ -13,15 +13,19 @@ public sealed class MainForm : Form private readonly ProcessService _processService; private readonly IInjectionService _injection; private readonly PacketLogService _log; + private readonly PacketValidationService _validation; private AppSettings _settings; private readonly ToolStripStatusLabel _statusLabel = new() { Text = "No process attached." }; private readonly ListBox _logListBox = new(); + private readonly ListBox _issuesListBox = new(); // High packet rates mean per-packet BeginInvoke + ListBox.Add would saturate the UI thread. // Capture threads enqueue here; a UI-thread timer drains the queue in one BeginUpdate/EndUpdate batch. private readonly ConcurrentQueue _pendingPackets = new(); + private readonly ConcurrentQueue _pendingIssues = new(); private readonly System.Windows.Forms.Timer _flushTimer = new() { Interval = 50 }; private const int LogCap = 5000; + private const int IssuesCap = 5000; private readonly CheckBox _captureSendBox = new() { Text = "Capture Send", AutoSize = true }; private readonly CheckBox _captureRecvBox = new() { Text = "Capture Recv", AutoSize = true }; private readonly Button _clearButton = new() { Text = "Clear", AutoSize = true }; @@ -34,12 +38,14 @@ public MainForm( SettingsService settingsService, ProcessService processService, IInjectionService injection, - PacketLogService log) + PacketLogService log, + PacketValidationService validation) { _settingsService = settingsService; _processService = processService; _injection = injection; _log = log; + _validation = validation; _settings = _settingsService.Load(); Text = "NosCore.DeveloperTools"; @@ -137,6 +143,12 @@ private TabPage BuildPacketsTab() _logListBox.KeyDown += OnLogKeyDown; _logListBox.ContextMenuStrip = BuildLogContextMenu(); + _issuesListBox.Dock = DockStyle.Fill; + _issuesListBox.IntegralHeight = false; + _issuesListBox.Font = new Font(FontFamily.GenericMonospace, 9F); + _issuesListBox.HorizontalScrollbar = true; + _issuesListBox.SelectionMode = SelectionMode.MultiExtended; + var toolbar = new FlowLayoutPanel { Dock = DockStyle.Top, @@ -157,7 +169,12 @@ private TabPage BuildPacketsTab() _settings.PacketFilters.CaptureReceive = _captureRecvBox.Checked; Persist(); }; - _clearButton.Click += (_, _) => _log.Clear(); + _clearButton.Click += (_, _) => + { + _log.Clear(); + while (_pendingIssues.TryDequeue(out _)) { } + _issuesListBox.Items.Clear(); + }; _filtersButton.Click += (_, _) => OpenFilters(); toolbar.Controls.Add(_captureSendBox); @@ -168,7 +185,15 @@ private TabPage BuildPacketsTab() var injectBar = BuildInjectBar(); - page.Controls.Add(_logListBox); + var subTabs = new TabControl { Dock = DockStyle.Fill }; + var logPage = new TabPage("Log"); + logPage.Controls.Add(_logListBox); + var issuesPage = new TabPage("Issues"); + issuesPage.Controls.Add(_issuesListBox); + subTabs.TabPages.Add(logPage); + subTabs.TabPages.Add(issuesPage); + + page.Controls.Add(subTabs); page.Controls.Add(injectBar); page.Controls.Add(toolbar); return page; @@ -831,6 +856,11 @@ private void WireEvents() // Drop filtered packets at intake so they never reach the log at all. if (!ShouldCapture(args.Packet)) return; _log.Add(args.Packet); + var issue = _validation.Validate(args.Packet); + if (issue is not null) + { + _pendingIssues.Enqueue(issue); + } }; _injection.StatusChanged += (_, msg) => BeginInvoke(() => _statusLabel.Text = msg); @@ -850,31 +880,37 @@ private void WireEvents() private void FlushPendingPackets() { - if (_pendingPackets.IsEmpty) return; + FlushQueueInto(_pendingPackets, _logListBox, LogCap); + FlushQueueInto(_pendingIssues, _issuesListBox, IssuesCap); + } + + private static void FlushQueueInto(ConcurrentQueue source, ListBox target, int cap) + { + if (source.IsEmpty) return; // Snapshot the queue to a local array so the batch add runs in a single - // BeginUpdate/EndUpdate block even if more packets arrive during the drain. - var buffer = new List(); - while (_pendingPackets.TryDequeue(out var p)) + // BeginUpdate/EndUpdate block even if more items arrive during the drain. + var buffer = new List(); + while (source.TryDequeue(out var item)) { - buffer.Add(p); + buffer.Add(item!); } if (buffer.Count == 0) return; - _logListBox.BeginUpdate(); + target.BeginUpdate(); try { - _logListBox.Items.AddRange(buffer.ToArray()); - var excess = _logListBox.Items.Count - LogCap; + target.Items.AddRange(buffer.Cast().ToArray()); + var excess = target.Items.Count - cap; for (var i = 0; i < excess; i++) { - _logListBox.Items.RemoveAt(0); + target.Items.RemoveAt(0); } - _logListBox.TopIndex = Math.Max(0, _logListBox.Items.Count - 1); + target.TopIndex = Math.Max(0, target.Items.Count - 1); } finally { - _logListBox.EndUpdate(); + target.EndUpdate(); } } diff --git a/src/NosCore.DeveloperTools/Models/PacketValidationIssue.cs b/src/NosCore.DeveloperTools/Models/PacketValidationIssue.cs new file mode 100644 index 0000000..b1ed2e2 --- /dev/null +++ b/src/NosCore.DeveloperTools/Models/PacketValidationIssue.cs @@ -0,0 +1,33 @@ +namespace NosCore.DeveloperTools.Models; + +public enum ValidationCategory +{ + Missing, + WrongStructure, + WrongTag, +} + +public sealed record PacketValidationIssue( + DateTime Timestamp, + ValidationCategory Category, + LoggedPacket Packet, + string Detail) +{ + private readonly string _display = Format(Timestamp, Category, Packet, Detail); + + public override string ToString() => _display; + + private static string Format(DateTime ts, ValidationCategory cat, LoggedPacket p, string detail) + { + var tag = cat switch + { + ValidationCategory.Missing => "[Missing]", + ValidationCategory.WrongStructure => "[Wrong structure]", + ValidationCategory.WrongTag => "[Wrong tag]", + _ => $"[{cat}]", + }; + var conn = p.Connection == PacketConnection.Login ? "Login" : "World"; + var src = p.Direction == PacketDirection.Send ? "Client" : "Server"; + return $"[{ts:HH:mm:ss.fff}] {tag} [{conn}] [{src}] {p.Raw} — {detail}"; + } +} diff --git a/src/NosCore.DeveloperTools/Program.cs b/src/NosCore.DeveloperTools/Program.cs index 650e2fd..f6fad70 100644 --- a/src/NosCore.DeveloperTools/Program.cs +++ b/src/NosCore.DeveloperTools/Program.cs @@ -29,7 +29,8 @@ private static void Main() var processService = new ProcessService(); using var injection = new RemoteAttachmentService(); var log = new PacketLogService(); - using var mainForm = new MainForm(settingsService, processService, injection, log); + var validation = new PacketValidationService(); + using var mainForm = new MainForm(settingsService, processService, injection, log, validation); DiagnosticLog.Info("MainForm constructed, Application.Run()"); Application.Run(mainForm); DiagnosticLog.Info("Application.Run() returned normally"); diff --git a/src/NosCore.DeveloperTools/Services/PacketValidationService.cs b/src/NosCore.DeveloperTools/Services/PacketValidationService.cs new file mode 100644 index 0000000..a2cabf8 --- /dev/null +++ b/src/NosCore.DeveloperTools/Services/PacketValidationService.cs @@ -0,0 +1,101 @@ +using System.Reflection; +using NosCore.DeveloperTools.Models; +using NosCore.Packets; +using NosCore.Packets.Attributes; + +namespace NosCore.DeveloperTools.Services; + +/// +/// Runs every captured packet through NosCore.Packets's deserializer +/// against the direction it was tagged with, and reports anomalies: +/// +/// Missing — header isn't defined anywhere in NosCore.Packets. +/// WrongStructure — header is defined for this direction but the wire format doesn't match the schema (deserializer throws). +/// WrongTag — header is defined but only in the opposite-direction namespace (a client header appeared tagged as [Server], or vice versa). +/// +/// Two separate instances are built so the +/// existing "server loses to client on duplicate header" logic inside +/// doesn't mask wrong-tag cases. +/// +public sealed class PacketValidationService +{ + private readonly Deserializer _clientDeserializer; + private readonly Deserializer _serverDeserializer; + private readonly HashSet _clientHeaders; + private readonly HashSet _serverHeaders; + + public PacketValidationService() + { + var all = typeof(PacketBase).Assembly + .GetTypes() + .Where(t => t is { IsAbstract: false, IsClass: true }) + .Where(t => t.GetCustomAttribute() != null) + .ToList(); + + var clientTypes = all.Where(t => t.Namespace?.Contains("ClientPackets") == true).ToList(); + var serverTypes = all.Where(t => t.Namespace?.Contains("ServerPackets") == true).ToList(); + + _clientDeserializer = new Deserializer(clientTypes); + _serverDeserializer = new Deserializer(serverTypes); + + _clientHeaders = new HashSet(clientTypes.SelectMany(HeadersOf), StringComparer.Ordinal); + _serverHeaders = new HashSet(serverTypes.SelectMany(HeadersOf), StringComparer.Ordinal); + } + + public PacketValidationIssue? Validate(LoggedPacket packet) + { + var header = ExtractHeader(packet.Raw); + if (string.IsNullOrEmpty(header)) return null; + + var fromClient = packet.Direction == PacketDirection.Send; + var expected = fromClient ? _clientHeaders : _serverHeaders; + var opposite = fromClient ? _serverHeaders : _clientHeaders; + var deserializer = fromClient ? _clientDeserializer : _serverDeserializer; + + if (expected.Contains(header)) + { + try + { + _ = deserializer.Deserialize(packet.Raw); + return null; + } + catch (Exception ex) + { + var detail = ex.InnerException?.Message ?? ex.Message; + return new PacketValidationIssue(DateTime.Now, ValidationCategory.WrongStructure, packet, detail); + } + } + + if (opposite.Contains(header)) + { + var expectedTag = fromClient ? "[Client]" : "[Server]"; + var actualSide = fromClient ? "server" : "client"; + return new PacketValidationIssue(DateTime.Now, ValidationCategory.WrongTag, packet, + $"Header '{header}' is a known {actualSide} packet but was captured as {expectedTag}."); + } + + return new PacketValidationIssue(DateTime.Now, ValidationCategory.Missing, packet, + $"Header '{header}' is not defined in NosCore.Packets."); + } + + private static IEnumerable HeadersOf(Type t) + { + var primary = t.GetCustomAttribute()?.Identification; + if (primary != null) yield return primary; + foreach (var alias in t.GetCustomAttributes()) + { + yield return alias.Identification; + } + } + + // Mirror of Deserializer's own header-extraction logic: the leading token + // is the keepalive id when it parses as a ushort (client→server packets), + // otherwise it's the header directly (server→client packets). + private static string ExtractHeader(string raw) + { + var tokens = raw.Split(' ', StringSplitOptions.RemoveEmptyEntries); + if (tokens.Length == 0) return string.Empty; + if (ushort.TryParse(tokens[0], out _) && tokens.Length >= 2) return tokens[1]; + return tokens[0]; + } +} From e92e5212c2249c6816f1be74cdf53f0c830dbaa5 Mon Sep 17 00:00:00 2001 From: erwan-joly Date: Fri, 24 Apr 2026 15:40:06 +1200 Subject: [PATCH 2/7] 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) --- src/NosCore.DeveloperTools/Services/PacketValidationService.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/NosCore.DeveloperTools/Services/PacketValidationService.cs b/src/NosCore.DeveloperTools/Services/PacketValidationService.cs index a2cabf8..8355b60 100644 --- a/src/NosCore.DeveloperTools/Services/PacketValidationService.cs +++ b/src/NosCore.DeveloperTools/Services/PacketValidationService.cs @@ -29,6 +29,7 @@ public PacketValidationService() var all = typeof(PacketBase).Assembly .GetTypes() .Where(t => t is { IsAbstract: false, IsClass: true }) + .Where(t => typeof(PacketBase).IsAssignableFrom(t)) .Where(t => t.GetCustomAttribute() != null) .ToList(); From 2fd3442aaabdfcdd493ba7b57fd7d9a26ccc302d Mon Sep 17 00:00:00 2001 From: erwan-joly Date: Fri, 24 Apr 2026 15:46:50 +1200 Subject: [PATCH 3/7] feat: Copy / Copy with tags / Select all on the Issues listbox MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- src/NosCore.DeveloperTools/Forms/MainForm.cs | 44 +++++++++++--------- 1 file changed, 25 insertions(+), 19 deletions(-) diff --git a/src/NosCore.DeveloperTools/Forms/MainForm.cs b/src/NosCore.DeveloperTools/Forms/MainForm.cs index 820fee7..5d87129 100644 --- a/src/NosCore.DeveloperTools/Forms/MainForm.cs +++ b/src/NosCore.DeveloperTools/Forms/MainForm.cs @@ -140,14 +140,16 @@ private TabPage BuildPacketsTab() _logListBox.Font = new Font(FontFamily.GenericMonospace, 9F); _logListBox.HorizontalScrollbar = true; _logListBox.SelectionMode = SelectionMode.MultiExtended; - _logListBox.KeyDown += OnLogKeyDown; - _logListBox.ContextMenuStrip = BuildLogContextMenu(); + _logListBox.KeyDown += OnListKeyDown; + _logListBox.ContextMenuStrip = BuildListContextMenu(_logListBox); _issuesListBox.Dock = DockStyle.Fill; _issuesListBox.IntegralHeight = false; _issuesListBox.Font = new Font(FontFamily.GenericMonospace, 9F); _issuesListBox.HorizontalScrollbar = true; _issuesListBox.SelectionMode = SelectionMode.MultiExtended; + _issuesListBox.KeyDown += OnListKeyDown; + _issuesListBox.ContextMenuStrip = BuildListContextMenu(_issuesListBox); var toolbar = new FlowLayoutPanel { @@ -914,65 +916,69 @@ private static void FlushQueueInto(ConcurrentQueue source, ListBox target, } } - private ContextMenuStrip BuildLogContextMenu() + private ContextMenuStrip BuildListContextMenu(ListBox list) { var menu = new ContextMenuStrip(); var copy = new ToolStripMenuItem("Copy") { ShortcutKeyDisplayString = "Ctrl+C" }; - copy.Click += (_, _) => CopySelected(withTags: false); + copy.Click += (_, _) => CopySelected(list, withTags: false); menu.Items.Add(copy); var copyTags = new ToolStripMenuItem("Copy with tags"); - copyTags.Click += (_, _) => CopySelected(withTags: true); + copyTags.Click += (_, _) => CopySelected(list, withTags: true); menu.Items.Add(copyTags); menu.Items.Add(new ToolStripSeparator()); var selectAll = new ToolStripMenuItem("Select all") { ShortcutKeyDisplayString = "Ctrl+A" }; - selectAll.Click += (_, _) => SelectAllLog(); + selectAll.Click += (_, _) => SelectAll(list); menu.Items.Add(selectAll); return menu; } - private void OnLogKeyDown(object? sender, KeyEventArgs e) + private void OnListKeyDown(object? sender, KeyEventArgs e) { + if (sender is not ListBox list) return; if (e.Control && e.KeyCode == Keys.A) { - SelectAllLog(); + SelectAll(list); e.SuppressKeyPress = true; e.Handled = true; } else if (e.Control && e.KeyCode == Keys.C) { - CopySelected(withTags: false); + CopySelected(list, withTags: false); e.SuppressKeyPress = true; e.Handled = true; } } - private void SelectAllLog() + private static void SelectAll(ListBox list) { - _logListBox.BeginUpdate(); + list.BeginUpdate(); try { - for (var i = 0; i < _logListBox.Items.Count; i++) + for (var i = 0; i < list.Items.Count; i++) { - _logListBox.SetSelected(i, true); + list.SetSelected(i, true); } } finally { - _logListBox.EndUpdate(); + list.EndUpdate(); } } - private void CopySelected(bool withTags) + private static void CopySelected(ListBox list, bool withTags) { - if (_logListBox.SelectedItems.Count == 0) return; - var lines = _logListBox.SelectedItems - .Cast() - .Select(p => withTags ? p.ToString() : p.Raw); + if (list.SelectedItems.Count == 0) return; + var lines = list.SelectedItems.Cast().Select(item => item switch + { + LoggedPacket p => withTags ? p.ToString() : p.Raw, + PacketValidationIssue i => withTags ? i.ToString() : i.Packet.Raw, + _ => item?.ToString() ?? string.Empty, + }); var text = string.Join(Environment.NewLine, lines); if (!string.IsNullOrEmpty(text)) { From aba853677c654a94fbd85debae8ac2c6aecefb2d Mon Sep 17 00:00:00 2001 From: erwan-joly Date: Fri, 24 Apr 2026 15:51:34 +1200 Subject: [PATCH 4/7] feat: colored stripe on Log rows that have a validation issue MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- src/NosCore.DeveloperTools/Forms/MainForm.cs | 44 ++++++++++++++++++- .../Models/LoggedPacket.cs | 3 ++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/src/NosCore.DeveloperTools/Forms/MainForm.cs b/src/NosCore.DeveloperTools/Forms/MainForm.cs index 5d87129..e823841 100644 --- a/src/NosCore.DeveloperTools/Forms/MainForm.cs +++ b/src/NosCore.DeveloperTools/Forms/MainForm.cs @@ -140,6 +140,13 @@ private TabPage BuildPacketsTab() _logListBox.Font = new Font(FontFamily.GenericMonospace, 9F); _logListBox.HorizontalScrollbar = true; _logListBox.SelectionMode = SelectionMode.MultiExtended; + _logListBox.DrawMode = DrawMode.OwnerDrawFixed; + _logListBox.ItemHeight = _logListBox.Font.Height + 2; + // OwnerDraw disables ListBox's auto-measuring of item width, so pick a large + // static extent covering anything we realistically capture (raw packets rarely + // exceed ~500 chars × ~7 px/char). + _logListBox.HorizontalExtent = 4000; + _logListBox.DrawItem += LogListBox_DrawItem; _logListBox.KeyDown += OnListKeyDown; _logListBox.ContextMenuStrip = BuildListContextMenu(_logListBox); @@ -857,12 +864,14 @@ private void WireEvents() { // Drop filtered packets at intake so they never reach the log at all. if (!ShouldCapture(args.Packet)) return; - _log.Add(args.Packet); + // Stamp Issue before enqueueing so the Log flush sees the flag on first draw. var issue = _validation.Validate(args.Packet); if (issue is not null) { + args.Packet.Issue = issue.Category; _pendingIssues.Enqueue(issue); } + _log.Add(args.Packet); }; _injection.StatusChanged += (_, msg) => BeginInvoke(() => _statusLabel.Text = msg); @@ -954,6 +963,39 @@ private void OnListKeyDown(object? sender, KeyEventArgs e) } } + private void LogListBox_DrawItem(object? sender, DrawItemEventArgs e) + { + if (e.Index < 0 || e.Index >= _logListBox.Items.Count) return; + e.DrawBackground(); + + const int stripeWidth = 4; + var packet = _logListBox.Items[e.Index] as LoggedPacket; + if (packet?.Issue is { } category) + { + var color = category switch + { + ValidationCategory.Missing => Color.Gold, + ValidationCategory.WrongStructure => Color.IndianRed, + ValidationCategory.WrongTag => Color.DarkOrange, + _ => Color.Transparent, + }; + using var brush = new SolidBrush(color); + e.Graphics.FillRectangle(brush, e.Bounds.Left, e.Bounds.Top, stripeWidth, e.Bounds.Height); + } + + var fg = (e.State & DrawItemState.Selected) != 0 ? SystemColors.HighlightText : SystemColors.WindowText; + var textBounds = new Rectangle( + e.Bounds.Left + stripeWidth + 2, e.Bounds.Top, + e.Bounds.Width - stripeWidth - 2, e.Bounds.Height); + TextRenderer.DrawText( + e.Graphics, + packet?.ToString() ?? _logListBox.Items[e.Index]?.ToString() ?? string.Empty, + e.Font ?? _logListBox.Font, + textBounds, fg, + TextFormatFlags.Left | TextFormatFlags.VerticalCenter | TextFormatFlags.NoPadding); + e.DrawFocusRectangle(); + } + private static void SelectAll(ListBox list) { list.BeginUpdate(); diff --git a/src/NosCore.DeveloperTools/Models/LoggedPacket.cs b/src/NosCore.DeveloperTools/Models/LoggedPacket.cs index 8f4ab62..f2b6870 100644 --- a/src/NosCore.DeveloperTools/Models/LoggedPacket.cs +++ b/src/NosCore.DeveloperTools/Models/LoggedPacket.cs @@ -21,6 +21,9 @@ public sealed record LoggedPacket( { private readonly string _display = FormatDisplay(Timestamp, Connection, Direction, Raw); + /// Set by PacketValidationService when validation flags this packet. Drives the Log tab's issue indicator. + public ValidationCategory? Issue { get; set; } + public override string ToString() => _display; private static string FormatDisplay(DateTime ts, PacketConnection conn, PacketDirection dir, string raw) From e1e71dd0f0f779e1713a1dd11f7174aa86e4d397 Mon Sep 17 00:00:00 2001 From: erwan-joly Date: Fri, 24 Apr 2026 16:01:06 +1200 Subject: [PATCH 5/7] feat: failed-headers summary on Issues; drop plain Copy on Issues menu MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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) --- src/NosCore.DeveloperTools/Forms/MainForm.cs | 96 ++++++++++++++++---- 1 file changed, 80 insertions(+), 16 deletions(-) diff --git a/src/NosCore.DeveloperTools/Forms/MainForm.cs b/src/NosCore.DeveloperTools/Forms/MainForm.cs index e823841..820bbb6 100644 --- a/src/NosCore.DeveloperTools/Forms/MainForm.cs +++ b/src/NosCore.DeveloperTools/Forms/MainForm.cs @@ -19,6 +19,14 @@ public sealed class MainForm : Form private readonly ToolStripStatusLabel _statusLabel = new() { Text = "No process attached." }; private readonly ListBox _logListBox = new(); private readonly ListBox _issuesListBox = new(); + private readonly TextBox _failedHeadersBox = new() + { + Dock = DockStyle.Top, + ReadOnly = true, + Multiline = false, + PlaceholderText = "Packet headers that have failed validation (cleared with the log).", + }; + private readonly SortedSet _failedHeaders = new(StringComparer.Ordinal); // High packet rates mean per-packet BeginInvoke + ListBox.Add would saturate the UI thread. // Capture threads enqueue here; a UI-thread timer drains the queue in one BeginUpdate/EndUpdate batch. private readonly ConcurrentQueue _pendingPackets = new(); @@ -156,7 +164,7 @@ private TabPage BuildPacketsTab() _issuesListBox.HorizontalScrollbar = true; _issuesListBox.SelectionMode = SelectionMode.MultiExtended; _issuesListBox.KeyDown += OnListKeyDown; - _issuesListBox.ContextMenuStrip = BuildListContextMenu(_issuesListBox); + _issuesListBox.ContextMenuStrip = BuildListContextMenu(_issuesListBox, includeRawCopy: false); var toolbar = new FlowLayoutPanel { @@ -178,12 +186,7 @@ private TabPage BuildPacketsTab() _settings.PacketFilters.CaptureReceive = _captureRecvBox.Checked; Persist(); }; - _clearButton.Click += (_, _) => - { - _log.Clear(); - while (_pendingIssues.TryDequeue(out _)) { } - _issuesListBox.Items.Clear(); - }; + _clearButton.Click += (_, _) => _log.Clear(); _filtersButton.Click += (_, _) => OpenFilters(); toolbar.Controls.Add(_captureSendBox); @@ -198,7 +201,9 @@ private TabPage BuildPacketsTab() var logPage = new TabPage("Log"); logPage.Controls.Add(_logListBox); var issuesPage = new TabPage("Issues"); + // Fill must be added before the docked Top control so it occupies the remaining space. issuesPage.Controls.Add(_issuesListBox); + issuesPage.Controls.Add(_failedHeadersBox); subTabs.TabPages.Add(logPage); subTabs.TabPages.Add(issuesPage); @@ -857,7 +862,11 @@ private void WireEvents() _log.Cleared += (_, _) => BeginInvoke(() => { while (_pendingPackets.TryDequeue(out _)) { } + while (_pendingIssues.TryDequeue(out _)) { } _logListBox.Items.Clear(); + _issuesListBox.Items.Clear(); + _failedHeaders.Clear(); + _failedHeadersBox.Text = string.Empty; }); _injection.PacketCaptured += (_, args) => @@ -892,7 +901,50 @@ private void WireEvents() private void FlushPendingPackets() { FlushQueueInto(_pendingPackets, _logListBox, LogCap); - FlushQueueInto(_pendingIssues, _issuesListBox, IssuesCap); + FlushIssues(); + } + + private void FlushIssues() + { + if (_pendingIssues.IsEmpty) return; + + var buffer = new List(); + while (_pendingIssues.TryDequeue(out var issue)) + { + buffer.Add(issue); + } + if (buffer.Count == 0) return; + + var headersDirty = false; + foreach (var issue in buffer) + { + var h = issue.Packet.Header; + if (!string.IsNullOrEmpty(h) && _failedHeaders.Add(h)) + { + headersDirty = true; + } + } + + _issuesListBox.BeginUpdate(); + try + { + _issuesListBox.Items.AddRange(buffer.Cast().ToArray()); + var excess = _issuesListBox.Items.Count - IssuesCap; + for (var i = 0; i < excess; i++) + { + _issuesListBox.Items.RemoveAt(0); + } + _issuesListBox.TopIndex = Math.Max(0, _issuesListBox.Items.Count - 1); + } + finally + { + _issuesListBox.EndUpdate(); + } + + if (headersDirty) + { + _failedHeadersBox.Text = string.Join(", ", _failedHeaders); + } } private static void FlushQueueInto(ConcurrentQueue source, ListBox target, int cap) @@ -925,17 +977,27 @@ private static void FlushQueueInto(ConcurrentQueue source, ListBox target, } } - private ContextMenuStrip BuildListContextMenu(ListBox list) + private ContextMenuStrip BuildListContextMenu(ListBox list, bool includeRawCopy = true) { var menu = new ContextMenuStrip(); - var copy = new ToolStripMenuItem("Copy") { ShortcutKeyDisplayString = "Ctrl+C" }; - copy.Click += (_, _) => CopySelected(list, withTags: false); - menu.Items.Add(copy); + if (includeRawCopy) + { + var copy = new ToolStripMenuItem("Copy") { ShortcutKeyDisplayString = "Ctrl+C" }; + copy.Click += (_, _) => CopySelected(list, withTags: false); + menu.Items.Add(copy); - var copyTags = new ToolStripMenuItem("Copy with tags"); - copyTags.Click += (_, _) => CopySelected(list, withTags: true); - menu.Items.Add(copyTags); + var copyTags = new ToolStripMenuItem("Copy with tags"); + copyTags.Click += (_, _) => CopySelected(list, withTags: true); + menu.Items.Add(copyTags); + } + else + { + // For the Issues listbox, raw-only copy is useless — the tags carry the category + detail. + var copyTags = new ToolStripMenuItem("Copy") { ShortcutKeyDisplayString = "Ctrl+C" }; + copyTags.Click += (_, _) => CopySelected(list, withTags: true); + menu.Items.Add(copyTags); + } menu.Items.Add(new ToolStripSeparator()); @@ -957,7 +1019,9 @@ private void OnListKeyDown(object? sender, KeyEventArgs e) } else if (e.Control && e.KeyCode == Keys.C) { - CopySelected(list, withTags: false); + // Issues listbox: always copy with tags (the category + detail is the whole point). + var withTags = list == _issuesListBox; + CopySelected(list, withTags); e.SuppressKeyPress = true; e.Handled = true; } From 6641eff1d4c8c7a2021dbdbb85a9a9a41748f544 Mon Sep 17 00:00:00 2001 From: erwan-joly Date: Fri, 24 Apr 2026 16:07:30 +1200 Subject: [PATCH 6/7] fix: register sub-packet types so 'key not present in dictionary' goes away MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .../Services/PacketValidationService.cs | 30 ++++++++++++++----- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/src/NosCore.DeveloperTools/Services/PacketValidationService.cs b/src/NosCore.DeveloperTools/Services/PacketValidationService.cs index 8355b60..18f14bf 100644 --- a/src/NosCore.DeveloperTools/Services/PacketValidationService.cs +++ b/src/NosCore.DeveloperTools/Services/PacketValidationService.cs @@ -26,21 +26,37 @@ public sealed class PacketValidationService public PacketValidationService() { - var all = typeof(PacketBase).Assembly + var allPacketBase = typeof(PacketBase).Assembly .GetTypes() .Where(t => t is { IsAbstract: false, IsClass: true }) .Where(t => typeof(PacketBase).IsAssignableFrom(t)) + .ToList(); + + // Header-bearing types are the top-level packets, split by direction. + var clientHeadered = allPacketBase + .Where(t => t.Namespace?.Contains("ClientPackets") == true) + .Where(t => t.GetCustomAttribute() != null) + .ToList(); + var serverHeadered = allPacketBase + .Where(t => t.Namespace?.Contains("ServerPackets") == true) .Where(t => t.GetCustomAttribute() != null) .ToList(); - var clientTypes = all.Where(t => t.Namespace?.Contains("ClientPackets") == true).ToList(); - var serverTypes = all.Where(t => t.Namespace?.Contains("ServerPackets") == true).ToList(); + // Sub-packet types have no [PacketHeader] — the Deserializer keys them by + // typeof(T).Name. They're not direction-specific, so feed them to both + // deserializers; otherwise DeserializeValue crashes on 'The given key + // was not present in the dictionary.' + var subpackets = allPacketBase + .Where(t => t.GetCustomAttribute() == null) + .ToList(); - _clientDeserializer = new Deserializer(clientTypes); - _serverDeserializer = new Deserializer(serverTypes); + _clientDeserializer = new Deserializer(clientHeadered.Concat(subpackets)); + _serverDeserializer = new Deserializer(serverHeadered.Concat(subpackets)); - _clientHeaders = new HashSet(clientTypes.SelectMany(HeadersOf), StringComparer.Ordinal); - _serverHeaders = new HashSet(serverTypes.SelectMany(HeadersOf), StringComparer.Ordinal); + // Only headered types drive the direction-match check; sub-packets never + // appear as top-level headers on the wire. + _clientHeaders = new HashSet(clientHeadered.SelectMany(HeadersOf), StringComparer.Ordinal); + _serverHeaders = new HashSet(serverHeadered.SelectMany(HeadersOf), StringComparer.Ordinal); } public PacketValidationIssue? Validate(LoggedPacket packet) From 250cada26cffca711585d5a42b3fd315b9b4ee91 Mon Sep 17 00:00:00 2001 From: erwan-joly Date: Fri, 24 Apr 2026 19:30:27 +1200 Subject: [PATCH 7/7] fix: skip the three pre-auth login-handshake lines in validator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every world-server connection opens with three bare lines from the client before the encrypted packet stream: — single numeric token GF — 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) --- .../Services/PacketValidationService.cs | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/NosCore.DeveloperTools/Services/PacketValidationService.cs b/src/NosCore.DeveloperTools/Services/PacketValidationService.cs index 18f14bf..803edbf 100644 --- a/src/NosCore.DeveloperTools/Services/PacketValidationService.cs +++ b/src/NosCore.DeveloperTools/Services/PacketValidationService.cs @@ -61,6 +61,11 @@ public PacketValidationService() public PacketValidationIssue? Validate(LoggedPacket packet) { + // Skip the pre-auth handshake lines the NosTale client sends before the + // encrypted packet stream begins — they don't use the normal header + // protocol and are correctly absent from NosCore.Packets. + if (IsLoginHandshake(packet)) return null; + var header = ExtractHeader(packet.Raw); if (string.IsNullOrEmpty(header)) return null; @@ -105,6 +110,23 @@ private static IEnumerable HeadersOf(Type t) } } + // The client sends three bare handshake lines at the start of every + // connection before the encrypted packet stream kicks in: + // "" — single numeric token + // " GF " — username / platform / region + // "thisisgfmode" — GF-mode marker + // None of them are header-protocol packets; flagging them as Missing is noise. + private static bool IsLoginHandshake(LoggedPacket packet) + { + if (packet.Direction != PacketDirection.Send) return false; + var tokens = packet.Raw.Split(' ', StringSplitOptions.RemoveEmptyEntries); + if (tokens.Length == 0) return false; + if (tokens.Length == 1 && ulong.TryParse(tokens[0], out _)) return true; + if (tokens.Length == 1 && tokens[0] == "thisisgfmode") return true; + if (tokens.Length >= 2 && tokens[1] == "GF") return true; + return false; + } + // Mirror of Deserializer's own header-extraction logic: the leading token // is the keepalive id when it parses as a ushort (client→server packets), // otherwise it's the header directly (server→client packets).