fix: exc_list items parse one per sub-packet, and req_exc is accepted mid-trade (21.1.3)

[erwan-joly]

Summary

• Deserializer: a trailing sub-packet list whose fields are space-separated (ExcListPacket.SubPackets) counted one item per token instead of one per sub-packet. exc_list 21 44 10 53 21 2 87 55 (the serializer corpus' own output) deserialized to six entries: two real offers, then four default Equipment/0/0 fillers. The count is now tokens / (PropertyAmount + 1), rounded up so a partial trailing item still parses as before. Dotted sub-packet lists and lists with an explicit Length don't take this branch.
• req_exc scope: Confirmed/Cancelled are only sent while a trade is open, but the header was InGame only, so NosCore's scope gate dropped them before the handler. It is now InGame | InTrade. While trading, the other request types are already refused by the handler (Requested → trading with someone else, List → OpenExchange fails, shops return early).

Needed by NosCoreIO/NosCore#2427, which currently works around both in the server.

Test plan

• dotnet test test/NosCore.Packets.Tests: 131/131
• New: exc_list with two and one offers yields exactly two and one items with the right fields
• New: req_exc declares InTrade
• Documentation snapshot updated (req_exc → InTrade | InGame)

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Exchange requests are now accepted both during gameplay and while trading.
  • Lists of exchange sub-packets are now parsed more accurately, including lists with multiple items or incomplete final items.
• Documentation
  • Updated packet availability information to reflect that exchange requests are supported during trading.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: NosCoreIO/NosCore.Packets/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: 880efa72-c153-4a47-8e88-22e76c532238

📥 Commits

Reviewing files that changed from the base of the PR and between 18e863e and d5e8114.

📒 Files selected for processing (5)

• documentation/DocumentationTest.PacketsDocumentation.verified.md
• src/NosCore.Packets/ClientPackets/Exchanges/ExchangeRequestPacket.cs
• src/NosCore.Packets/Deserializer.cs
• src/NosCore.Packets/NosCore.Packets.csproj
• test/NosCore.Packets.Tests/DeserializerTest.cs

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

Walkthrough

The req_exc packet is now documented and accepted in InTrade. Final-list item counting in Deserializer.DeserializeList now uses the registered sub-packet schema width when available. Tests cover the exchange request scope and exc_list packets with one or multiple sub-packets.

Changes

Exchange request scope

Layer / File(s)	Summary
Declare and verify exchange request scope src/NosCore.Packets/ClientPackets/Exchanges/ExchangeRequestPacket.cs, documentation/DocumentationTest.PacketsDocumentation.verified.md, test/NosCore.Packets.Tests/DeserializerTest.cs	req_exc now includes Scope.InTrade alongside Scope.InGame. The documentation and test include InTrade.

Exchange list deserialization

Layer / File(s)	Summary
Count final-list sub-packets from schema width src/NosCore.Packets/Deserializer.cs, test/NosCore.Packets.Tests/DeserializerTest.cs, src/NosCore.Packets/NosCore.Packets.csproj	For a final list without an explicit separator, item count uses the matching sub-packet schema width and rounds up. If no creator matches or the first token contains the configured separator, counting uses one token per item. Tests cover exc_list packets with one and two sub-packets. The package version changes from 21.1.2 to 21.1.3.

Priority: ➖ Normal

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

Change: Bug fix

Merge Risk: ⚪ Minimal · up to d5e81

The confirmed list-parsing behavior matches the supported packet format. No concrete merge-blocking failure is established by the available evidence.

Pre-merge checks · 4 · 1

Failed checks (1 warning)

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

Passed checks (4 passed)

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly identifies both primary changes: correcting exc_list sub-packet parsing and accepting req_exc during an active trade. It is concise and specific.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

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

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.}