Merge pull request #5669 from IvanSavenko/haven_fix

Fix crash on evaluating creature terrain limiter during initialization (Haven mod fix)

lib/CCreatureSet.cpp

@@ -820,32 +820,28 @@ ImagePath CStackInstance::bonusToGraphics(const std::shared_ptr<Bonus> & bonus)
 
 CArmedInstance * CStackInstance::getArmy()
 {
-	if (armyInstanceID.hasValue())
-		return dynamic_cast<CArmedInstance*>(cb->gameState().getObjInstance(armyInstanceID));
-	return nullptr;
+	return armyInstance;
 }
 
 const CArmedInstance * CStackInstance::getArmy() const
 {
-	if (armyInstanceID.hasValue())
-		return dynamic_cast<const CArmedInstance*>(cb->getObjInstance(armyInstanceID));
-	return nullptr;
+	return armyInstance;
 }
 
-void CStackInstance::setArmy(const CArmedInstance * ArmyObj)
+void CStackInstance::setArmy(CArmedInstance * ArmyObj)
 {
 	auto oldArmy = getArmy();
 
 	if(oldArmy)
 	{
 		detachFrom(*oldArmy);
-		armyInstanceID = {};
+		armyInstance = nullptr;
 	}
 
 	if(ArmyObj)
 	{
 		attachTo(const_cast<CArmedInstance&>(*ArmyObj));
-		armyInstanceID = ArmyObj->id;
+		armyInstance = ArmyObj;
 	}
 }
 
@@ -907,12 +903,13 @@ TerrainId CStackInstance::getNativeTerrain() const
 
 	return getFactionID().toEntity(LIBRARY)->getNativeTerrain();
 }
+
 TerrainId CStackInstance::getCurrentTerrain() const
 {
+	assert(getArmy() != nullptr);
 	return getArmy()->getCurrentTerrain();
 }
 
-
 CreatureID CStackInstance::getCreatureID() const
 {
 	if(getType())

lib/CCreatureSet.h

@@ -76,7 +76,7 @@ class DLL_LINKAGE CStackInstance : public CBonusSystemNode, public CStackBasicDe
 	BonusValueCache nativeTerrain;
 	BonusValueCache initiative;
 
-	ObjectInstanceID armyInstanceID; //stack must be part of some army, army must be part of some object
+	CArmedInstance * armyInstance = nullptr; //stack must be part of some army, army must be part of some object
 
 	IGameCallback * getCallback() const final { return cb; }
 
@@ -92,7 +92,7 @@ public:
 
 	CArmedInstance * getArmy();
 	const CArmedInstance * getArmy() const; //stack must be part of some army, army must be part of some object
-	void setArmy(const CArmedInstance *ArmyObj);
+	void setArmy(CArmedInstance *ArmyObj);
 
 	TExpType getTotalExperience() const;
 	TExpType getAverageExperience() const;
@@ -104,15 +104,19 @@ public:
 		h & static_cast<CStackBasicDescriptor&>(*this);
 		h & static_cast<CArtifactSet&>(*this);
 
+		if (h.hasFeature(Handler::Version::STACK_INSTANCE_ARMY_FIX))
+		{
+			// no-op
+		}
 		if (h.hasFeature(Handler::Version::NO_RAW_POINTERS_IN_SERIALIZER))
 		{
-			h & armyInstanceID;
+			ObjectInstanceID dummyID;
+			h & dummyID;
 		}
 		else
 		{
 			std::shared_ptr<CGObjectInstance> army;
 			h & army;
-			armyInstanceID = army->id;
 		}
 
 		h & totalExperience;

lib/mapObjects/CArmedInstance.cpp

@@ -175,7 +175,7 @@ void CArmedInstance::attachUnitsToArmy()
 	assert(getArmy() != nullptr);
 
 	for(const auto & elem : stacks)
-		elem.second->attachTo(*getArmy());
+		elem.second->setArmy(getArmy());
 }
 
 const IBonusBearer* CArmedInstance::getBonusBearer() const

lib/mapObjects/CGHeroInstance.cpp

@@ -1610,6 +1610,11 @@ void CGHeroInstance::levelUp(const std::vector<SecondarySkill> & skills)
 	nodeHasChanged();
 }
 
+void CGHeroInstance::attachCommanderToArmy()
+{
+	commander->setArmy(this);
+}
+
 void CGHeroInstance::levelUpAutomatically(vstd::RNG & rand)
 {
 	while(gainsLevel())

lib/mapObjects/CGHeroInstance.h

@@ -352,6 +352,7 @@ protected:
 
 private:
 	void levelUpAutomatically(vstd::RNG & rand);
+	void attachCommanderToArmy();
 
 public:
 	std::string getHeroTypeName() const;
@@ -396,6 +397,9 @@ public:
 
 		h & commander;
 		h & visitedObjects;
+
+		if(!h.saving && h.loadingGamestate)
+			attachCommanderToArmy();
 	}
 };
 

lib/serializer/ESerializationVersion.h

@@ -37,8 +37,9 @@ enum class ESerializationVersion : int32_t
 	MAP_HEADER_DISPOSED_HEROES, // map header contains disposed heroes list
 	NO_RAW_POINTERS_IN_SERIALIZER, // large rework that removed all non-owning pointers from serializer
 	STACK_INSTANCE_EXPERIENCE_FIX, // stack experience is stored as total, not as average
+	STACK_INSTANCE_ARMY_FIX, // remove serialization of army that owns stack instance
 	
-	CURRENT = STACK_INSTANCE_EXPERIENCE_FIX,
+	CURRENT = STACK_INSTANCE_ARMY_FIX,
 };
 
 static_assert(ESerializationVersion::MINIMAL <= ESerializationVersion::CURRENT, "Invalid serialization version definition!");