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

[erwan-joly]

The exceptions

Serializer keyed _packetSerializerDictionary by typeof(T).Name — the simple name. Ten packet class names exist in both a ClientPackets.* and a ServerPackets.* namespace; Initialize let the non-ClientPackets one win, so serializing the loser looked up the winner's compiled delegate and DynamicInvoke threw ArgumentException. (Deserializer never had the problem — it keys by wire header.)

The dictionary is private, so the fix keys it by Type: every packet keeps its own delegate, the ClientPackets-wins eviction logic is deleted, and the failure mode is gone rather than pinned. Serialize and the injected-sub-packet path look up by GetType() directly (also faster than string hashing).

Corpus/snapshot updates

• OnlyTheKnownSimpleNameCollisionsFailToSerialize → EveryPacketSerializes: zero tolerated exceptions.
• The snapshot now pins real wire output for all 398 packets; colliding simple names are disambiguated by namespace in the line ids so each line maps to exactly one type.
• CollidingPacketNamesAreStillDistinctTypes stays, now purely to catch new accidental duplicates.

Also fixes master's red CI

#498 was merged without being up to date with master, and #500/#501 had changed six packets (pstash/pstash_all headers, DmPacket/EventPacket/FbtPacket/RcPacket) after its snapshot was generated — so the snapshot test currently fails on master. The regenerated snapshot here is taken against current master and folds those in. Verified: every changed snapshot line traces to either the collision fix or those two merged PRs; all other 780+ lines are byte-identical.

127/127 tests pass locally.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • Improved packet serialization reliability across client and server packet types.
  • Resolved serialization failures for several packets and handled similarly named packet types correctly.
  • Corrected serialized headers for stash-related packets.

• Tests

  • Expanded serializer coverage to include additional packet types.
  • Updated validation to require successful serialization for every packet and provide clearer type-specific failure details.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 355056c7-b8fd-41a2-9d20-e8292f11d4a8

📥 Commits

Reviewing files that changed from the base of the PR and between 84642e7 and b0f354f.

📒 Files selected for processing (3)

• src/NosCore.Packets/Serializer.cs
• test/NosCore.Packets.Tests/SerializerCorpusTests.ExpressionTreeSerializerOutputIsPinnedForEveryPacket.verified.txt
• test/NosCore.Packets.Tests/SerializerCorpusTests.cs

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

---

Walkthrough

The serializer now uses runtime Type keys to distinguish packets with identical simple names. Corpus tests require successful serialization for all packets and use qualified identifiers for collisions. Verified outputs include new packet entries and updated stash command headers.

Changes

Packet serializer type resolution

Layer / File(s)	Summary
Runtime type lookup src/NosCore.Packets/Serializer.cs	Serializer stores compiled delegates by Type. Registration and serialization resolve delegates from runtime packet types, including injected IPacket values.
Collision-aware corpus generation test/NosCore.Packets.Tests/SerializerCorpusTests.cs	The corpus test records fully qualified failure names, rejects all serialization failures, and qualifies identifiers for colliding packet names.
Serializer output snapshots test/NosCore.Packets.Tests/SerializerCorpusTests.ExpressionTreeSerializerOutputIsPinnedForEveryPacket.verified.txt	Snapshots use qualified packet names where required. Client and server packet outputs are updated, EventPacket, FbtPacket, and client RcPacket entries are added, and stash headers change to pstash_all and pstash.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to b0f35

The serializer now keeps same-named packet types independent while preserving the public API, eliminating the reported collision failure. No actionable merge-blocking risk remains beyond normal checks and review.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 2 files. (1 skipped: 1…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: using Type-based registry keys so packets with colliding names serialize correctly.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 2 files. (1 skipped: 1 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/serializer-type-key

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}