fix: inventory move leaves source slot uncleared on client

[erwan-joly]

Summary

• Fixes the visible inventory bug where moving an item (mvi / mve) left the source slot rendering the old item. Root cause: the server sent `ivn {slot}.-1.0.0.0` where real NosTale uses VNum = 0 to mean "empty"; -1 is the serializer's null-sub-packet marker that the client silently discards.
• Consumes the NosCore.Packets 17.0.0 bump that tightened `IvnPacket.IvnSubPackets` to `List` (non-nullable) and widened `IvnSubPacket` to 7 fields so equipment matches the trace.
• Picks up collateral renames from the vanosilla audit: `InCharacterSubPacket.Unknown/Morph/Unknown2/Unknown3` → `FairyBooster/FairyMorph/ShowInEffect/Morph`. The previously named `Morph` was actually the fairy's morph — the character's morph now sits at its correct index.
• Picks up the `InItemSubPacket` position swap so `in 9 …` emits `{amount} {isQuest} 0 {owner}` matching the capture.

Why

Trace shows the server reply to `mvi 0 19 1 20` is two `ivn` packets, one zeroed (`ivn 0 19.0.0.0.0.0.0`) and one populated (`ivn 0 20.4998.0.0.0.0.0`). NosCore was producing `ivn 0 19.-1.0.0.0` — different VNum, fewer fields — and the client rendered the move as a duplicate instead of a reassignment.

Test plan

• `dotnet test NosCore.sln` — 924 / 924 pass (includes updated `BiPacketHandlerTests.SlotShouldBeCleared` now asserting VNum = 0 and the unchanged MVI / MVE handler tests).
• Manual: log in, drag an equipped weapon between two empty slots and confirm the source clears on the client after release.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Chores

  • Updated NosCore.Packets dependency to version 17.0.0

• Bug Fixes

  • Corrected packet generation for inventory handling with proper field initialization
  • Fixed character information packet construction with accurate field assignments

• Tests

  • Updated test assertions to verify correct packet field values

[coderabbitai]

Walkthrough

The NosCore.Packets dependency is upgraded from version 16.9.0 to 17.0.0. Corresponding code changes adjust packet construction logic: inventory packet structures change from nullable to non-nullable element lists; redundant field assignments are removed; unknown placeholder fields are replaced with named properties; and test assertions are updated accordingly.

Changes

Cohort / File(s)	Summary
Dependency Update Directory.Packages.props	NosCore.Packets version bumped from 16.9.0 to 17.0.0.
Inventory Packet Structure src/NosCore.GameObject/Ecs/Extensions/CharacterEntityExtension.cs, src/NosCore.GameObject/Ecs/Extensions/ItemInstanceExtension.cs	Packet construction refactored: IvnSubPackets list element type changed from nullable (List<IvnSubPacket?>) to non-nullable (List<IvnSubPacket>); redundant default value assignments (VNum, RareAmount, UpgradeDesign, SecondUpgrade) removed from ItemInstanceExtension.
Character Packet Fields src/NosCore.GameObject/Ecs/Extensions/PlayerBundleExtensions.cs	InCharacterSubPacket generation updated: unknown placeholder fields (Unknown, Unknown2, Unknown3) replaced with named properties (FairyBooster, FairyMorph, ShowInEffect).
Inventory Test Assertion test/NosCore.PacketHandlers.Tests/Inventory/BiPacketHandlerTests.cs	Test expectation updated for cleared inventory slots: VNum == -1 changed to VNum == 0.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main fix: inventory items left in source slots during moves now properly clear on the client side, addressing the root cause of the VNum = -1 vs VNum = 0 protocol issue.
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.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ 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 fix/inventory-move-slot-clear

---

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

[coderabbitai]

🧹 Nitpick comments (1)

src/NosCore.GameObject/Ecs/Extensions/ItemInstanceExtension.cs (1)

22-27: Make cleared-slot VNum explicit instead of implicit default.

At Line 26, only Slot is assigned. Given this PR fixes a protocol-sensitive clear-slot path, explicitly setting VNum = 0 improves safety and intent.

Proposed change

             if (itemInstance == null)
             {
                 return new IvnSubPacket
                 {
-                    Slot = slot
+                    Slot = slot,
+                    VNum = 0
                 };
             }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/NosCore.GameObject/Ecs/Extensions/ItemInstanceExtension.cs` around lines
22 - 27, The returned IvnSubPacket when itemInstance is null omits an explicit
VNum, relying on the implicit default; update the null branch that constructs
the IvnSubPacket (where itemInstance == null) to set VNum = 0 along with Slot to
make the cleared-slot value explicit (reference IvnSubPacket, itemInstance,
slot, and VNum).

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/NosCore.GameObject/Ecs/Extensions/ItemInstanceExtension.cs`:
- Around line 22-27: The returned IvnSubPacket when itemInstance is null omits
an explicit VNum, relying on the implicit default; update the null branch that
constructs the IvnSubPacket (where itemInstance == null) to set VNum = 0 along
with Slot to make the cleared-slot value explicit (reference IvnSubPacket,
itemInstance, slot, and VNum).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: b43dbbdd-e2f5-4f04-becb-8d49b9bc95ee

📥 Commits

Reviewing files that changed from the base of the PR and between 6432b93 and be1b297.

📒 Files selected for processing (5)

• Directory.Packages.props
• src/NosCore.GameObject/Ecs/Extensions/CharacterEntityExtension.cs
• src/NosCore.GameObject/Ecs/Extensions/ItemInstanceExtension.cs
• src/NosCore.GameObject/Ecs/Extensions/PlayerBundleExtensions.cs
• test/NosCore.PacketHandlers.Tests/Inventory/BiPacketHandlerTests.cs