From 70d47718ec461e68b1be6a47212af5757c3b925b Mon Sep 17 00:00:00 2001 From: Bentley Davis <10065854+NonPolynomialTim@users.noreply.github.com> Date: Sat, 1 Aug 2026 13:37:43 -0400 Subject: [PATCH] fix(coop): harnessEquip/harnessSetArmor enforce the real craft-capacity 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 --- src/Basescape/CraftWeaponsState.cpp | 124 +++++++++++----------------- src/Basescape/CraftWeaponsState.h | 4 + src/Basescape/SoldierArmorState.cpp | 7 ++ 3 files changed, 57 insertions(+), 78 deletions(-) diff --git a/src/Basescape/CraftWeaponsState.cpp b/src/Basescape/CraftWeaponsState.cpp index a8e6bd854..c6ddadff4 100644 --- a/src/Basescape/CraftWeaponsState.cpp +++ b/src/Basescape/CraftWeaponsState.cpp @@ -162,54 +162,28 @@ void CraftWeaponsState::btnCancelClick(Action *) } /** - * Equips the weapon on the craft and returns to the previous screen. - * @param action Pointer to an action. + * Capacity gate shared by lstWeaponsClick (UI) and harnessEquip (test), so the + * harness cannot reach an over-capacity state the UI blocks. Returns the STR_ + * error key of the first craft-capacity limit mounting @a refWeapon in this slot + * would violate, or "" if it fits. No state-stack side effects. */ -void CraftWeaponsState::lstWeaponsClick(Action *) +std::string CraftWeaponsState::equipCapacityError(const RuleCraftWeapon* refWeapon) const { CraftWeapon *current = _craft->getWeapons()->at(_weapon); - - const RuleCraftWeapon* refWeapon = _weapons[_lstWeapons->getSelectedRow()]; const RuleCraftWeapon* currWeapon = current ? current->getRules() : nullptr; { int refCapBonus1 = refWeapon ? refWeapon->getBonusStats().soldiers : 0; int currCapBonus1 = currWeapon ? currWeapon->getBonusStats().soldiers : 0; int diff1 = (refCapBonus1 - currCapBonus1); - if (diff1) - { - if ((_craft->getMaxUnitsRaw() - _craft->getSpaceUsed() + diff1) < 0) - { - _game->popState(); - _game->pushState(new ErrorMessageState( - tr("STR_NOT_ENOUGH_CARGO_SPACE"), - _palette, - _game->getMod()->getInterface("craftWeapons")->getElement("errorMessage")->color, - "BACK14.SCR", - _game->getMod()->getInterface("craftWeapons")->getElement("errorPalette")->color) - ); - return; - } - } + if (diff1 && (_craft->getMaxUnitsRaw() - _craft->getSpaceUsed() + diff1) < 0) + return "STR_NOT_ENOUGH_CARGO_SPACE"; } { int refCapBonus2 = refWeapon ? refWeapon->getBonusStats().vehicles : 0; int currCapBonus2 = currWeapon ? currWeapon->getBonusStats().vehicles : 0; int diff2 = (refCapBonus2 - currCapBonus2); - if (diff2) - { - if ((_craft->getMaxVehiclesAndLargeSoldiersRaw() - _craft->getNumVehiclesAndLargeSoldiers() + diff2) < 0) - { - _game->popState(); - _game->pushState(new ErrorMessageState( - tr("STR_NOT_ENOUGH_HWP_CAPACITY"), - _palette, - _game->getMod()->getInterface("craftWeapons")->getElement("errorMessage")->color, - "BACK14.SCR", - _game->getMod()->getInterface("craftWeapons")->getElement("errorPalette")->color) - ); - return; - } - } + if (diff2 && (_craft->getMaxVehiclesAndLargeSoldiersRaw() - _craft->getNumVehiclesAndLargeSoldiers() + diff2) < 0) + return "STR_NOT_ENOUGH_HWP_CAPACITY"; } { int refCapBonus3 = refWeapon ? refWeapon->getBonusStats().maxItems : 0; @@ -228,51 +202,40 @@ void CraftWeaponsState::lstWeaponsClick(Action *) for (auto& itemType : _game->getMod()->getItemsList()) { RuleItem* rule = _game->getMod()->getItem(itemType); - - Unit* isVehicle = rule->getVehicleUnit(); - int cQty = 0; - if (isVehicle) - { - cQty = _craft->getVehicleCount(itemType); - } - else - { - cQty = _craft->getItems()->getItem(rule); - totalItems += cQty; - totalItemStorageSize += cQty * rule->getSize(); - } - } - } - if (diff3) - { - if ((_craft->getMaxItemsRaw() - totalItems + diff3) < 0) - { - _game->popState(); - _game->pushState(new ErrorMessageState( - tr("STR_NOT_ENOUGH_STORAGE_SPACE_1"), - _palette, - _game->getMod()->getInterface("craftWeapons")->getElement("errorMessage")->color, - "BACK14.SCR", - _game->getMod()->getInterface("craftWeapons")->getElement("errorPalette")->color) - ); - return; - } - } - if (diff4_b) - { - if ((_craft->getMaxStorageSpaceRaw() - totalItemStorageSize + diff4) < 0.0) - { - _game->popState(); - _game->pushState(new ErrorMessageState( - tr("STR_NOT_ENOUGH_STORAGE_SPACE_2"), - _palette, - _game->getMod()->getInterface("craftWeapons")->getElement("errorMessage")->color, - "BACK14.SCR", - _game->getMod()->getInterface("craftWeapons")->getElement("errorPalette")->color) - ); - return; + if (rule->getVehicleUnit()) + continue; // vehicles count against HWP capacity, checked above + int cQty = _craft->getItems()->getItem(rule); + totalItems += cQty; + totalItemStorageSize += cQty * rule->getSize(); } } + if (diff3 && (_craft->getMaxItemsRaw() - totalItems + diff3) < 0) + return "STR_NOT_ENOUGH_STORAGE_SPACE_1"; + if (diff4_b && (_craft->getMaxStorageSpaceRaw() - totalItemStorageSize + diff4) < 0.0) + return "STR_NOT_ENOUGH_STORAGE_SPACE_2"; + } + return ""; +} + +/** + * Equips the weapon on the craft and returns to the previous screen. + * @param action Pointer to an action. + */ +void CraftWeaponsState::lstWeaponsClick(Action *) +{ + const RuleCraftWeapon* refWeapon = _weapons[_lstWeapons->getSelectedRow()]; + std::string capErr = equipCapacityError(refWeapon); + if (!capErr.empty()) + { + _game->popState(); + _game->pushState(new ErrorMessageState( + tr(capErr), + _palette, + _game->getMod()->getInterface("craftWeapons")->getElement("errorMessage")->color, + "BACK14.SCR", + _game->getMod()->getInterface("craftWeapons")->getElement("errorPalette")->color) + ); + return; } equipSelectedWeapon(_weapons[_lstWeapons->getSelectedRow()]); @@ -336,6 +299,11 @@ bool CraftWeaponsState::harnessEquip(const std::string& weaponType) std::string t = _weapons[i] ? _weapons[i]->getType() : ""; if (t == weaponType) { + // Enforce the SAME capacity gate lstWeaponsClick applies, so the + // harness cannot mount into an over-capacity craft a real player is + // blocked from. Return false as "click refused". + if (!equipCapacityError(_weapons[i]).empty()) + return false; equipSelectedWeapon(_weapons[i]); return true; } diff --git a/src/Basescape/CraftWeaponsState.h b/src/Basescape/CraftWeaponsState.h index 6c252dcf8..c1b31f325 100644 --- a/src/Basescape/CraftWeaponsState.h +++ b/src/Basescape/CraftWeaponsState.h @@ -50,6 +50,10 @@ class CraftWeaponsState : public State /// SHARED (PRD-J09 GAP-5b): mount @a selRule (0 = dismount) in this slot - /// routes craft_rearm in SHARED, mutates the shared stores locally in SEPARATE. void equipSelectedWeapon(RuleCraftWeapon* selRule); + /// Capacity gate shared by lstWeaponsClick (UI) and harnessEquip (test): + /// STR_ error key of the first craft-capacity limit mounting @a refWeapon in + /// this slot would violate, or "" if it fits. No state-stack side effects. + std::string equipCapacityError(const RuleCraftWeapon* refWeapon) const; public: /// Creates the Craft Weapons state. CraftWeaponsState(Base *base, size_t craft, size_t weapon); diff --git a/src/Basescape/SoldierArmorState.cpp b/src/Basescape/SoldierArmorState.cpp index 2b058906d..5d21fe32f 100644 --- a/src/Basescape/SoldierArmorState.cpp +++ b/src/Basescape/SoldierArmorState.cpp @@ -356,6 +356,13 @@ bool SoldierArmorState::harnessSetArmor(const std::string& armorType) { Armor* next = _game->getMod()->getArmor(armorType, false); if (!next) return false; + // Enforce the SAME craft-space gate lstArmorClick applies, so the + // harness cannot reach an over-capacity state a real player is blocked + // from (STR_NOT_ENOUGH_CRAFT_SPACE). Return false as "click refused". + Soldier* soldier = _base->getSoldiers()->at(_soldier); + Craft* craft = soldier->getCraft(); + if (craft && !craft->validateArmorChange(soldier->getArmor()->getSize(), next->getSize())) + return false; applyArmorSelection(next); return true; }