fix(coop): harnessEquip/harnessSetArmor enforce the real craft-capacity gate - #122
Closed
NonPolynomialTim wants to merge 1 commit into
Closed
fix(coop): harnessEquip/harnessSetArmor enforce the real craft-capacity gate#122NonPolynomialTim wants to merge 1 commit into
NonPolynomialTim wants to merge 1 commit into
Conversation
…ty gate The harness* fidelity audit found two methods that bypassed the client-side capacity gate their real button enforces - and which the SHARED host validators do NOT re-check - so a test could reach an over-capacity craft no player can reach (the same class of hazard as the resumeCampaign() lobby bypass): - harnessSetArmor called applyArmorSelection() directly, skipping lstArmorClick's Craft::validateArmorChange (STR_NOT_ENOUGH_CRAFT_SPACE). - harnessEquip called equipSelectedWeapon() directly, skipping lstWeaponsClick's four capacity gates (cargo / HWP / storage_1 / storage_2). Extract lstWeaponsClick's capacity checks into a pure CraftWeaponsState::equipCapacityError() - one source of truth, no state-stack side effects - used by both the UI handler and harnessEquip; add the matching validateArmorChange guard to harnessSetArmor. Both now refuse an over-capacity change exactly like a player. test_shared_equip2 stays green. The deeper half - craftRearmValidate / soldierArmorValidate in SharedEcon.cpp not re-checking craft capacity host-side (so capacity is client-only-enforced in SHARED) - is a real latent bug filed as a separate follow-up. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Collaborator
Author
|
Superseded by #123, which fixed issue #121 (host-side craft-capacity re-validation) and in doing so redid this PR's harness gate fixes with a better, shareable |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #120 (merged). #120 landed the mid-battle-resume fix plus the harness "real button" hardening (start/resume/inline callers). This PR carries the one commit that missed the #120 merge window: the harness craft-capacity gate fixes. Rebased on current
origin/main.What
The
harness*fidelity audit found two methods that bypassed the client-side capacity gate their real button enforces — and which the SHARED host validators do not re-check — so a test could reach an over-capacity craft no player can (the same class of hazard as theresumeCampaign()lobby bypass #120 fixed):harnessSetArmorcalledapplyArmorSelection()directly, skippinglstArmorClick'sCraft::validateArmorChange(STR_NOT_ENOUGH_CRAFT_SPACE).harnessEquipcalledequipSelectedWeapon()directly, skippinglstWeaponsClick's four capacity gates (cargo / HWP / storage_1 / storage_2).Extracted
lstWeaponsClick's capacity checks into a pureCraftWeaponsState::equipCapacityError()— one source of truth, no state-stack side effects — used by both the UI handler andharnessEquip; added the matchingvalidateArmorChangeguard toharnessSetArmor. Both now refuse an over-capacity change exactly like a player.test_shared_equip2stays green.Not in this PR
The deeper engine half — the SHARED host validators (
craftRearmValidate/soldierArmorValidateinSharedEcon.cpp) not re-checking craft capacity, so capacity is client-only-enforced in SHARED — is a real latent bug tracked in #121.🤖 Generated with Claude Code