docs(design): soldier weapon/shield loadout — type vs instance split#540
Conversation
…535) Consolidates #535's directive into a committed design doc: Weapon/Shield become concrete, interned TYPE classes (shared, one per type), while each soldier references its active loadout by id through new parallel _sim_soldier_weapon_id/_sim_soldier_shield_id arrays, mirroring the existing SoA per-soldier state on Unit. Lays out a four-phase plan (type registry -> shield hold-state -> rendering -> weapon-switching gameplay) and how it relates to #516's orders-queue design and #530's formation geometry. No code changes -- design only. Phase issues: #536, #537, #538, #539.
dem-extra1
added
the
work in progress
🎬 Gameplay demo🚫 No gameplay clip for this PR — docs-only design note (docs/soldier-loadout-design.md) -- no code changes, nothing to record. The author marked this change as not demonstrable by a recorded battle ( |
dem-extra1
removed
the
work in progress
This comment has been minimized.
This comment has been minimized.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
- Byte-count claim was mis-attributed to CLAUDE.md; replace with a derivation from Unit.gd's actual _sim_* array declarations (4 PackedVector2Array @ 8B + 3 PackedFloat32Array @ 4B = 44B), explicitly noting it's not a CLAUDE.md citation. - Fix Archers/Cavalry SHIELD_NONE contradiction -- only Archers carry SHIELD_NONE; Cavalry carry SHIELD_ROUND per the registry table. - Drop the fictional Terrain class from the Weapon.effective_reach sketch; reference the real mechanism (PathField.active.speed_at(), Unit.gd:884) instead. - Re-verified every other cited symbol against the code while in there: corrected _spawn_unit's owner (Battle.gd, not Unit.gd), grounded the shield-weight gradient in SoldierCombat.profile_for()'s real per-type values, fixed the archer weapon table entry (0.6m reach_m is the melee sidearm, not the bow -- ranged attacks use a separate RANGED_RANGE constant), clarified Shield.covers() is an illustrative simplification of today's continuous facing_gate()/land_chance() defence math, corrected the nonexistent Combat.gd autoload reference, and updated PR #534's now-current (non-draft) state.
This comment has been minimized.
This comment has been minimized.
|
Working on this --- paws off until I'm done. |
|
Claude finished @dem-extra1's task in 37s —— View job Review complete
What changed since the last reviewThe only new commit on this branch since the prior confirming review ( produces no output — neither of this PR's two files changed. The merge only brought in unrelated content from VerdictAll three findings from the original review were fixed in Ready for merge. |
|
PR is fully clean and ready to merge: latest review (run 28566294851) has zero findings and says "Ready for merge"; all CI checks green including Coverage and codecov/patch; all inline review threads resolved; branch is synced with main (merge commit 946d006). Releasing my claim. |
2 participants
Summary
Consolidates #535's directive ("model soldier weapons/shields as concrete
classed objects") into a committed design doc, resolving the tension against
the existing SoA per-soldier arrays on
Unit:Weapon/Shieldbecome real GDScript classes with concrete fields/methods,as shared, interned TYPE definitions (one instance per type, like a
Resource) — not one object per soldier.
arrays (
_sim_soldier_weapon_id,_sim_soldier_shield_id), mirroring_sim_soldier_pos/_sim_soldier_hp/etc. Per-soldier state that genuinelyvaries (shield hold angle/bracing) lives in its own parallel array, never on
the shared type object.
(
_default_loadout()inscripts/Battle.gd): spear, gladius, bow, spatha,scutum, round shield, no-shield. No new unit types invented.
identical; (2) shield hold-angle state, coordinated with Formations change combat multipliers + the formation field but not soldier GEOMETRY (square isn't square; shielded stances aren't tighter) #530's formation
geometry (PR feat(formations): restructure block geometry per formation, not just combat multipliers (#530) #534, currently in draft); (3) rendering consumes weapon/
shield/hold state; (4) weapon-switching gameplay via Design: unify all unit commands under one polymorphic Order + an orders-queue on Unit #516's future
SwitchWeaponOrder, blocked on Design: unify all unit commands under one polymorphic Order + an orders-queue on Unit #516's orders-queue design landing further.Phase issues filed and linked as sub-issues of #535: #536, #537, #538, #539.
Closes nothing yet — #535 stays open as the umbrella tracking issue until all
phases land.
Docs only, no code changes.
demos/demo.535.jsonopts out of the gameplayclip (nothing to record).
Test plan
.gd/scene/asset changes —tools/check.sh validate/testnotapplicable to this diff.
tools/check.sh chars— doc is underdocs/, not*.qmd/*.R, so outof scope for the chars check, but written in the same plain-ASCII style
as
docs/combat-model.md.scripts/Battle.gd's_default_loadout()andUnit.gd's_sim_soldier_*arrays foraccuracy.
coordination point rather than blocking on it.