diff --git a/src/Battlescape/BattlescapeState.cpp b/src/Battlescape/BattlescapeState.cpp index 7bb74ecf9..9b722e5a8 100644 --- a/src/Battlescape/BattlescapeState.cpp +++ b/src/Battlescape/BattlescapeState.cpp @@ -2325,78 +2325,55 @@ void BattlescapeState::coopHealing(int actor_id, int type, int part, std::string if (!unit) return; - // coop (PRD-P1 rule, applied here by PRD-P6): replay on a stack-local action - // carrying the HEALER's own medikit. The classic branch below runs on - // _currentAction, whose weapon is whatever the receiving player happens to be - // holding - in parallel mode that is the other player's business entirely, - // and medikitUse() dereferences it. - if (connectionTCP::parallelTurnActive()) + // Replay every peer medikit use on a stack-local action carrying the HEALER's + // own medikit. The old classic-turn path reused this machine's currentAction + // and set actor/type/time but never weapon; medikitUse() then dereferenced a + // null (or, worse, unrelated local) weapon. Modern packets identify both the + // healer and item, so classic and parallel turns can share the safe path. + BattleUnit* healer = healer_id == -1 ? unit : nullptr; + if (healer_id != -1) { - BattleUnit* healer = unit; - if (healer_id != -1) + for (auto u : *_save->getUnits()) { - for (auto u : *_save->getUnits()) + if (u->getId() == healer_id) { - if (u->getId() == healer_id) - { - healer = u; - break; - } + healer = u; + break; } } + } - BattleAction action = BattlescapeGame::makeReplayAction(healer); - action.type = (BattleActionType)type; - action.Time = time; - action.weapon = BattlescapeGame::coopResolveWeapon(_save, healer, weapon_id, weapon_type, hand); - if (!action.weapon) - { - Log(LOG_INFO) << "coop: medkit replay skipped, healer " << healer->getId() - << " has no '" << weapon_type << "' (id " << weapon_id << ")"; - return; - } - - BattleMediKitAction mode = medkit_state == "stimulant" ? BMA_STIMULANT - : medkit_state == "painkiller" ? BMA_PAINKILLER - : BMA_HEAL; - UnitBodyPart bodyPart = mode == BMA_HEAL ? (UnitBodyPart)part : BODYPART_TORSO; - _battleGame->getTileEngine()->medikitUse(&action, unit, mode, bodyPart); - // both machines now hold the same charge counts, so this reaches the same - // answer on both and the item census stays equal. - _battleGame->getTileEngine()->medikitRemoveIfEmpty(&action); - updateSoldierInfo(); + if (!healer) + { + Log(LOG_WARNING) << "coop: medkit replay skipped, healer " << healer_id << " was not found"; return; } - _save->setSelectedUnit(unit); - _battleGame->getCurrentAction()->actor = unit; - - _battleGame->getCurrentAction()->type = (BattleActionType)type; - - _battleGame->getCurrentAction()->Time = time; - - if (medkit_state == "heal") - { - - _battleGame->getTileEngine()->medikitUse(_battleGame->getCurrentAction(), unit, BMA_HEAL, (UnitBodyPart)part); - - } - - if (medkit_state == "stimulant") - { - - _battleGame->getTileEngine()->medikitUse(_battleGame->getCurrentAction(), unit, BMA_STIMULANT, BODYPART_TORSO); - - } - - if (medkit_state == "painkiller") - { - - _battleGame->getTileEngine()->medikitUse(_battleGame->getCurrentAction(), unit, BMA_PAINKILLER, BODYPART_TORSO); - - } + BattleAction action = BattlescapeGame::makeReplayAction(healer); + action.type = (BattleActionType)type; + action.Time = time; + action.weapon = BattlescapeGame::coopResolveWeapon(_save, healer, weapon_id, weapon_type, hand); + if (!action.weapon || !action.weapon->getRules() + || action.weapon->getRules()->getBattleType() != BT_MEDIKIT) + { + Log(LOG_WARNING) << "coop: medkit replay skipped, healer " << healer->getId() + << " has no '" << weapon_type << "' (id " << weapon_id << ")"; + return; + } - + BattleMediKitAction mode = medkit_state == "stimulant" ? BMA_STIMULANT + : medkit_state == "painkiller" ? BMA_PAINKILLER + : BMA_HEAL; + UnitBodyPart bodyPart = BODYPART_TORSO; + if (mode == BMA_HEAL && part >= 0 && part < BODYPART_MAX) + { + bodyPart = (UnitBodyPart)part; + } + _battleGame->getTileEngine()->medikitUse(&action, unit, mode, bodyPart); + // Both machines now hold the same charge counts, so this reaches the same + // answer on both and the item census stays equal. + _battleGame->getTileEngine()->medikitRemoveIfEmpty(&action); + updateSoldierInfo(); } void BattlescapeState::coopActiveGranade(int actor_id, int type, std::string hand, int fusetimer, int item_id) diff --git a/src/Battlescape/ExplosionBState.cpp b/src/Battlescape/ExplosionBState.cpp index 38ee800a1..a568e5d79 100644 --- a/src/Battlescape/ExplosionBState.cpp +++ b/src/Battlescape/ExplosionBState.cpp @@ -556,7 +556,7 @@ void ExplosionBState::think() // terrain leaked onto the pacing path again. if (_explosionCounter > 0) ++g_coopTerrainPacingConsumes; _coopTaskCompleted = false; - _parent->getCoopMod()->_coopPacingWait = false; + _parent->getCoopMod()->_coopPacingWait = false; _parent->popState(); return; } diff --git a/src/Battlescape/TileEngine.cpp b/src/Battlescape/TileEngine.cpp index 2a5b883b1..07f35bc04 100644 --- a/src/Battlescape/TileEngine.cpp +++ b/src/Battlescape/TileEngine.cpp @@ -33,6 +33,7 @@ #include "../Savegame/HitLog.h" #include "../Engine/RNG.h" #include "../Engine/GraphSubset.h" +#include "../Engine/Logger.h" #include "BattlescapeState.h" #include "../Mod/MapDataSet.h" #include "../Mod/Unit.h" @@ -5526,6 +5527,12 @@ void TileEngine::medikitRemoveIfEmpty(BattleAction *action) bool TileEngine::medikitUse(BattleAction *action, BattleUnit *target, BattleMediKitAction originalMedikitAction, UnitBodyPart bodyPart) { + if (!action || !target || !action->weapon || !action->weapon->getRules()) + { + Log(LOG_WARNING) << "medikitUse: rejected action with missing target, weapon or rules"; + return false; + } + if (_save->isPreview()) { return false; diff --git a/src/CoopMod/OptionsMultiplayer.cpp b/src/CoopMod/OptionsMultiplayer.cpp index b3dddeb60..07fd8f012 100644 --- a/src/CoopMod/OptionsMultiplayer.cpp +++ b/src/CoopMod/OptionsMultiplayer.cpp @@ -26,6 +26,7 @@ #include "../Interface/Window.h" #include "../Mod/Mod.h" #include "../Mod/RuleInterface.h" +#include "../CoopMod/connectionTCP.h" #include "../CoopMod/OptionsMultiplayer.h" #include #include @@ -421,8 +422,34 @@ void OptionsMultiplayerState::lstOptionsClick(Action* action) if (setting->type() == OPTION_BOOL) { bool* b = setting->asBool(); + // Research sync is host-authoritative while connected. The client sees + // the negotiated value but must not silently override it locally. + if (b == &Options::EnableResearchSync + && _game->getCoopMod()->getCoopStatic() + && !_game->getCoopMod()->getServerOwner()) + { + return; + } *b = !*b; settingText = *b ? tr("STR_YES") : tr("STR_NO"); + if (b == &Options::EnableResearchSync && _game->getCoopMod()) + { + // Make an in-session change effective immediately on this peer. The + // handshake already distributes the host's initial value to the client. + _game->getCoopMod()->_enable_research_sync = *b; + if (!*b) + { + _game->getCoopMod()->waitedResearch.clear(); + } + if (_game->getCoopMod()->getCoopStatic() + && _game->getCoopMod()->getServerOwner()) + { + Json::Value root; + root["state"] = "research_sync_option"; + root["enabled"] = *b; + _game->getCoopMod()->sendTCPPacketData(root.toStyledString()); + } + } if (b == &Options::lazyLoadResources && !*b) { Options::reload = true; // reload when turning lazy loading off diff --git a/src/CoopMod/connectionTCP.cpp b/src/CoopMod/connectionTCP.cpp index 345c613db..b8a80518e 100644 --- a/src/CoopMod/connectionTCP.cpp +++ b/src/CoopMod/connectionTCP.cpp @@ -7362,9 +7362,24 @@ void connectionTCP::onTCPMessage(std::string stateString, Json::Value obj) if (stateString == "research") { + // The host decides this once during the COOP_READY handshake. Do not + // retain disabled packets: otherwise switching the option back on could + // unexpectedly apply an old completion. + if (_enable_research_sync) + { + waitedResearch.append(obj); + } - waitedResearch.append(obj); + } + if (stateString == "research_sync_option" && !getServerOwner()) + { + _enable_research_sync = obj.get("enabled", false).asBool(); + Options::EnableResearchSync = _enable_research_sync; + if (!_enable_research_sync) + { + waitedResearch.clear(); + } } if (stateString == "add_coop_item") @@ -12558,11 +12573,17 @@ void connectionTCP::onTCPMessage(std::string stateString, Json::Value obj) connectionTCP::_enable_time_sync = Options::EnableTimeSync; root["enable_time_sync"] = connectionTCP::_enable_time_sync; - // reaction shoot option (PVP) + // Reaction-fire suppression is a PvP-only option. Always reset the + // session value for non-PvP games so a disabled value from an earlier PvP + // session cannot leak into a later co-op campaign. if (getCoopGamemode() == 2 || getCoopGamemode() == 3) { connectionTCP::_enable_reaction_shoot = Options::EnableReactionFirePvp; } + else + { + connectionTCP::_enable_reaction_shoot = true; + } // reaction shoot root["enable_reaction_shoot"] = connectionTCP::_enable_reaction_shoot; @@ -12698,6 +12719,10 @@ void connectionTCP::onTCPMessage(std::string stateString, Json::Value obj) // research option bool enable_research_sync = obj["enable_research_sync"].asBool(); _enable_research_sync = enable_research_sync; + // Keep the multiplayer options page in sync with the host-authoritative + // session value. Previously it continued to display the client's local + // default (YES), even when the host had sent NO. + Options::EnableResearchSync = enable_research_sync; // time option bool enable_time_sync = obj["enable_time_sync"].asBool(); diff --git a/src/Engine/Options.cpp b/src/Engine/Options.cpp index c9616f947..8478195a3 100644 --- a/src/Engine/Options.cpp +++ b/src/Engine/Options.cpp @@ -574,7 +574,7 @@ void createAdvancedOptionsOTHER() // coop _info.push_back(OptionInfo(OPTION_OTHER, "EnableTimeSync", &EnableTimeSync, true, "Enable Time Sync", "STR_GEOSCAPE")); _info.push_back(OptionInfo(OPTION_OTHER, "EnableHostOnlyTimeSpeed", &EnableHostOnlyTimeSpeed, false, "Only Host Can Change Time Speed", "STR_GEOSCAPE")); - _info.push_back(OptionInfo(OPTION_OTHER, "EnableResearchSync", &EnableResearchSync, true, "Enable Research Sync", "STR_BASESCAPE")); + _info.push_back(OptionInfo(OPTION_OTHER, "EnableResearchSync", &EnableResearchSync, true, "Enable Research Sync (Separate)", "STR_BASESCAPE")); _info.push_back(OptionInfo(OPTION_OTHER, "UnbalancedCraftSoldiersLimit", &UnbalancedCraftSoldiersLimit, false, "Unbalanced Craft Soldiers Limit", "STR_BASESCAPE")); _info.push_back(OptionInfo(OPTION_OTHER, "EnableReactionFirePvp", &EnableReactionFirePvp, true, "Enable Reaction Fire in PvP", "STR_BATTLESCAPE")); _info.push_back(OptionInfo(OPTION_OTHER, "EnableOtherPlayerFootsteps", &EnableOtherPlayerFootsteps, true,"Enable Other Player Footstep Sounds", "STR_BATTLESCAPE")); diff --git a/src/Geoscape/GeoscapeState.cpp b/src/Geoscape/GeoscapeState.cpp index 431c13ede..8b3873e1b 100644 --- a/src/Geoscape/GeoscapeState.cpp +++ b/src/Geoscape/GeoscapeState.cpp @@ -1357,7 +1357,9 @@ void GeoscapeState::think() // coop // research - if (_game->getCoopMod()->getCoopStatic() && !_game->getCoopMod()->waitedResearch.empty()) + if (_game->getCoopMod()->getCoopStatic() + && _game->getCoopMod()->_enable_research_sync + && !_game->getCoopMod()->waitedResearch.empty()) { if (_game->getSavedGame()->getSelectedBase()) @@ -1423,6 +1425,13 @@ void GeoscapeState::think() } } } + else if (!_game->getCoopMod()->_enable_research_sync + && !_game->getCoopMod()->waitedResearch.empty()) + { + // Never let completions received before/while research sync was disabled + // become active later if the option changes. + _game->getCoopMod()->waitedResearch.clear(); + } // coop _game->getCoopMod()->setCoopCampaign(true); diff --git a/src/Geoscape/ResearchCompleteState.cpp b/src/Geoscape/ResearchCompleteState.cpp index 752df73e4..a70e1f66c 100644 --- a/src/Geoscape/ResearchCompleteState.cpp +++ b/src/Geoscape/ResearchCompleteState.cpp @@ -94,7 +94,8 @@ ResearchCompleteState::ResearchCompleteState(const RuleResearch* newResearch, co // here. (This also avoids the newResearch->getName() null-deref below when the // host completes an already-seen lookup.) if (_game->getCoopMod()->getCoopStatic() == true && _coop == false - && !_game->getCoopMod()->isSharedCampaign()) + && !_game->getCoopMod()->isSharedCampaign() + && _game->getCoopMod()->_enable_research_sync) { Json::Value root; @@ -117,7 +118,7 @@ ResearchCompleteState::ResearchCompleteState(const RuleResearch* newResearch, co } root["base_lat"] = base->getLatitude(); - root["base_lot"] = base->getLongitude(); + root["base_lon"] = base->getLongitude(); _game->getCoopMod()->sendTCPPacketData(root.toStyledString());