fix hazard removal while iterating

OpenApoc · Mar 22, 2019 · 01b08d7 · 01b08d7
1 parent 7d52415
commit 01b08d7
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 24 deletions.
diff --git a/game/state/battle/battle.cpp b/game/state/battle/battle.cpp
@@ -1326,7 +1326,7 @@ sp<BattleHazard> Battle::placeHazard(GameState &state, StateRef<Organisation> ow
 					LogWarning(
 					    "Ensure we are not putting out a fire that is attached to a feature!");
 				}
-				existingHazard->die(state, false);
+				existingHazard->dieAndRemove(state, false);
 			}
 		}
 		map->addObjectToMap(hazard);
@@ -1671,8 +1671,10 @@ void Battle::update(GameState &state, unsigned int ticks)
 	Trace::start("Battle::update::hazards->update");
 	for (auto it = this->hazards.begin(); it != this->hazards.end();)
 	{
-		auto d = *it++;
-		d->update(state, ticks);
+		if ((*it)->update(state, ticks))
+			it = hazards.erase(it);
+		else
+			++it;
 	}
 	Trace::end("Battle::update::hazards->update");
 	Trace::start("Battle::update::explosions->update");
@@ -1844,11 +1846,10 @@ void Battle::updateTBEnd(GameState &state)
 	Trace::start("Battle::updateTBEnd::hazards->update");
 	for (auto it = this->hazards.begin(); it != this->hazards.end();)
 	{
-		auto d = *it++;
-		if (d->ownerOrganisation == currentActiveOrganisation)
-		{
-			d->updateTB(state);
-		}
+		if ((*it)->ownerOrganisation == currentActiveOrganisation && (*it)->updateTB(state))
+			it = hazards.erase(it);
+		else
+			++it;
 	}
 	Trace::end("Battle::updateTBEnd::hazards->update");
 	Trace::start("Battle::updateTBEnd::items->update");

diff --git a/game/state/battle/battlehazard.cpp b/game/state/battle/battlehazard.cpp
@@ -37,8 +37,6 @@ BattleHazard::BattleHazard(GameState &state, StateRef<DamageType> damageType, bo
 void BattleHazard::die(GameState &state, bool violently)
 {
 	auto this_shared = shared_from_this();
-	state.current_battle->hazards.erase(this_shared);
-
 	tileObject->removeFromMap();
 	tileObject.reset();
 
@@ -54,6 +52,11 @@ void BattleHazard::die(GameState &state, bool violently)
 		                                  dtSmoke->hazardType->getLifetime(state), power, 6);
 	}
 }
+void BattleHazard::dieAndRemove(GameState &state, bool violently)
+{
+	die(state, violently);
+	state.current_battle->hazards.erase(shared_from_this());
+}
 
 bool BattleHazard::expand(GameState &state, const TileMap &map, const Vec3<int> &to, unsigned ttl,
                           bool fireSmoke)
@@ -215,7 +218,7 @@ bool BattleHazard::expand(GameState &state, const TileMap &map, const Vec3<int>
 		{
 			if (replaceWeaker)
 			{
-				existingHazard->die(state, false);
+				existingHazard->dieAndRemove(state, false);
 			}
 			state.current_battle->placeHazard(
 			    state, ownerOrganisation, ownerUnit, spreadDamageType, {to.x, to.y, to.z},
@@ -426,7 +429,7 @@ void BattleHazard::applyEffect(GameState &state)
 	}
 }
 
-void BattleHazard::updateInner(GameState &state, unsigned int ticks)
+bool BattleHazard::updateInner(GameState &state, unsigned int ticks)
 {
 	nextUpdateTicksAccumulated += ticks;
 	while (nextUpdateTicksAccumulated >= TICKS_PER_HAZARD_UPDATE)
@@ -458,8 +461,8 @@ void BattleHazard::updateInner(GameState &state, unsigned int ticks)
 			}
 			if (age >= 130)
 			{
-				die(state);
-				return;
+				die(state, true);
+				return true;
 			}
 		}
 		else
@@ -476,14 +479,15 @@ void BattleHazard::updateInner(GameState &state, unsigned int ticks)
 			}
 			if (age >= lifetime)
 			{
-				die(state);
-				return;
+				die(state, true);
+				return true;
 			}
 		}
 	}
+	return false;
 }
 
-void BattleHazard::update(GameState &state, unsigned int ticks)
+bool BattleHazard::update(GameState &state, unsigned int ticks)
 {
 	bool realTime = state.current_battle->mode == Battle::Mode::RealTime;
 
@@ -509,11 +513,12 @@ void BattleHazard::update(GameState &state, unsigned int ticks)
 
 	if (realTime)
 	{
-		updateInner(state, ticks);
+		return updateInner(state, ticks);
 	}
+	return false;
 }
 
-void BattleHazard::updateTB(GameState &state) { updateInner(state, TICKS_PER_TURN); }
+bool BattleHazard::updateTB(GameState &state) { return updateInner(state, TICKS_PER_TURN); }
 
 void BattleHazard::updateTileVisionBlock(GameState &state)
 {

diff --git a/game/state/battle/battlehazard.h b/game/state/battle/battlehazard.h
@@ -50,12 +50,14 @@ class BattleHazard : public std::enable_shared_from_this<BattleHazard>
 	            bool fireSmoke = false);
 	void grow(GameState &state);
 	void applyEffect(GameState &state);
-	void die(GameState &state, bool violently = true);
+	void die(GameState &state, bool violently);
+	void dieAndRemove(GameState &state, bool violently);
 	void updateTileVisionBlock(GameState &state);
 
-	void update(GameState &state, unsigned int ticks);
-	void updateInner(GameState &state, unsigned int ticks);
-	void updateTB(GameState &state);
+	// these return true if this hazard has died and needs to be deleted
+	bool update(GameState &state, unsigned int ticks);
+	bool updateInner(GameState &state, unsigned int ticks);
+	bool updateTB(GameState &state);
 
 	BattleHazard() = default;
 	BattleHazard(GameState &state, StateRef<DamageType> damageType, bool delayVisibility = true);
@@ -129,4 +131,4 @@ Smoke cannot extinguish fire that is burning a scenery, but can extinguish fire
 ground.
 Walls are apparently immune to fire.
 
-*/
+*/