spikerj/spikersoft-issues
1
0 Fork 0
Code Issues 23 Pull Requests Actions Packages Projects Releases Wiki Activity

EquipItem slot validation off by one — EquipmentSlot↔ItemEquipSlot ordinal cast mismatch #793

New Issue
Closed
opened 2026-07-22 06:59:52 +00:00 by spikerj · 1 comment
spikerj commented 2026-07-22 06:59:52 +00:00
Owner
Copy link
Copy source

Found by the coverage wave (spikersoft-backend PR #454) while testing BaseZone.ProcessEquipItemCommand.

PlayerEntity.EquipItem (SpikerSoft.Common IGameEntity.cs ~line 800) validates with:

if (definition.EquipSlot != (ItemEquipSlot)(int)equipSlot && definition.EquipSlot != ItemEquipSlot.None)
    return (false, null);

But the enums are misaligned by one:

  • EquipmentSlot (IGameCommand.cs): Weapon = 0, Armor = 1, HpPotion = 2, MpPotion = 3, ...
  • ItemEquipSlot (ItemDefinition.cs): None = 0, Weapon = 1, Armor = 2, HpPotion = 3, MpPotion = 4

So the cast maps EquipmentSlot.Weapon (0) → ItemEquipSlot.None (0), and every item equips only through the command slot one BELOW its real one:

  • EquipItemCommand { EquipmentSlot = Weapon } with a weapon (e.g. wooden_staff) → rejected
  • EquipItemCommand { EquipmentSlot = Armor } with that same weapon → accepted (equipment dict then stores the weapon under Armor)

Impact: clients sending the semantically correct slot can never equip anything; whatever works today works by accident of the off-by-one. Fix by mapping explicitly (switch) or aligning the enums — note EquipmentSlot.Armor is documented "Legacy - maps to Torso", so an explicit mapping function is probably the safe fix.

Until fixed, BaseZoneItemCommandTests pins the current off-by-one behavior as characterization tests referencing this issue — they will fail when the mapping is corrected (then flip the expectations).

Found by the coverage wave (spikersoft-backend PR #454) while testing `BaseZone.ProcessEquipItemCommand`. `PlayerEntity.EquipItem` (SpikerSoft.Common `IGameEntity.cs` ~line 800) validates with: ```csharp if (definition.EquipSlot != (ItemEquipSlot)(int)equipSlot && definition.EquipSlot != ItemEquipSlot.None) return (false, null); ``` But the enums are misaligned by one: - `EquipmentSlot` (IGameCommand.cs): `Weapon = 0, Armor = 1, HpPotion = 2, MpPotion = 3, ...` - `ItemEquipSlot` (ItemDefinition.cs): `None = 0, Weapon = 1, Armor = 2, HpPotion = 3, MpPotion = 4` So the cast maps `EquipmentSlot.Weapon (0)` → `ItemEquipSlot.None (0)`, and every item equips only through the command slot one BELOW its real one: - `EquipItemCommand { EquipmentSlot = Weapon }` with a weapon (e.g. `wooden_staff`) → **rejected** - `EquipItemCommand { EquipmentSlot = Armor }` with that same weapon → **accepted** (equipment dict then stores the weapon under `Armor`) Impact: clients sending the semantically correct slot can never equip anything; whatever works today works by accident of the off-by-one. Fix by mapping explicitly (switch) or aligning the enums — note `EquipmentSlot.Armor` is documented "Legacy - maps to Torso", so an explicit mapping function is probably the safe fix. Until fixed, `BaseZoneItemCommandTests` pins the current off-by-one behavior as characterization tests referencing this issue — they will fail when the mapping is corrected (then flip the expectations).
spikerj commented 2026-08-07 13:47:23 +00:00
Author
Owner
Copy link
Copy source

Migrated to spikerj/spikersoft-backend#539 as part of the umbrella-tracker breakup.

Verified 2026-08-07 against the synced default branch.

  • Code: spikersoft-backend@98102023 — still broken. SpikerSoft.Common/Models/GameServer/IGameEntity.cs:810 still does definition.EquipSlot != (ItemEquipSlot)(int)equipSlot, and SpikerSoft.Common/Models/GameServer/ItemDefinition.cs:19-26 still declares ItemEquipSlot { None, Weapon, Armor, HpPotion, MpPotion } — i.e. still offset by one from EquipmentSlot. The ordinal cast therefore still maps EquipmentSlot.Weapon (0) → ItemEquipSlot.None (0).
  • Live: Not directly probeable without an in-game equip. docker service ps spikersoft-gameserver_spikersoft-gameserver → 1/1 Running on SERVER, image redeployed 2026-08-07 12:25 UTC.
  • Remaining: Replace the ordinal cast with an explicit EquipmentSlot → ItemEquipSlot mapping (switch), noting EquipmentSlot.Armor is documented "Legacy - maps to Torso"; then flip the BaseZoneItemCommandTests characterization expectations that currently pin the off-by-one.

Status: still broken — full detail carried over to the new issue.

Closing here. Work now lives in the repo that holds the fix, so fixes #539 in a PR will
auto-close it on merge. The umbrella tracker keeps cross-repo epics only.

— Opus 5 Agent

Migrated to **spikerj/spikersoft-backend#539** as part of the umbrella-tracker breakup. Verified 2026-08-07 against the synced default branch. - **Code:** `spikersoft-backend@98102023` — still broken. `SpikerSoft.Common/Models/GameServer/IGameEntity.cs:810` still does `definition.EquipSlot != (ItemEquipSlot)(int)equipSlot`, and `SpikerSoft.Common/Models/GameServer/ItemDefinition.cs:19-26` still declares `ItemEquipSlot { None, Weapon, Armor, HpPotion, MpPotion }` — i.e. still offset by one from `EquipmentSlot`. The ordinal cast therefore still maps `EquipmentSlot.Weapon (0)` → `ItemEquipSlot.None (0)`. - **Live:** Not directly probeable without an in-game equip. `docker service ps spikersoft-gameserver_spikersoft-gameserver` → 1/1 Running on SERVER, image redeployed 2026-08-07 12:25 UTC. - **Remaining:** Replace the ordinal cast with an explicit `EquipmentSlot → ItemEquipSlot` mapping (switch), noting `EquipmentSlot.Armor` is documented "Legacy - maps to Torso"; then flip the `BaseZoneItemCommandTests` characterization expectations that currently pin the off-by-one. Status: still broken — full detail carried over to the new issue. Closing here. Work now lives in the repo that holds the fix, so `fixes #539` in a PR will auto-close it on merge. The umbrella tracker keeps cross-repo epics only. — Opus 5 Agent
Sign in to join this conversation.
No Branch/Tag Specified
Labels
Clear labels
agentic
Actionable surfaced by the agentic architecture & security review
 bug
Something is not working
 duplicate
This issue or pull request already exists
 enhancement
New feature
 epic
Cross-repo epic — children live in per-repo trackers
 help wanted
Need some help
 invalid
Something is wrong
 question
More information is needed
 sonarqube
Code quality issue surfaced by SonarQube (bug / code smell)
 wontfix
This won't be fixed
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
spikerj (Joseph Spiker)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: spikerj/spikersoft-issues#793