Merge pull request #4770 from IvanSavenko/prison_fix

Fix for loading hero types / identities

AI/Nullkiller/Goals/CaptureObject.h

@@ -31,7 +31,7 @@ namespace Goals
 		{
 			objid = obj->id.getNum();
 			tile = obj->visitablePos();
-			name = obj->typeName;
+			name = obj->getTypeName();
 		}
 
 		bool operator==(const CaptureObject & other) const override;

AI/Nullkiller/Goals/ExecuteHeroChain.cpp

@@ -30,7 +30,7 @@ ExecuteHeroChain::ExecuteHeroChain(const AIPath & path, const CGObjectInstance *
 #if NKAI_TRACE_LEVEL >= 1
 		targetName = obj->getObjectName() + tile.toString();
 #else
-		targetName = obj->typeName + tile.toString();
+		targetName = obj->getTypeName() + tile.toString();
 #endif
 	}
 	else

AI/Nullkiller/Pathfinding/Actors.cpp

@@ -440,7 +440,7 @@ int DwellingActor::getInitialTurn(bool waitForGrowth, int dayOfWeek)
 
 std::string DwellingActor::toString() const
 {
-	return dwelling->typeName + dwelling->visitablePos().toString();
+	return dwelling->getTypeName() + dwelling->visitablePos().toString();
 }
 
 CCreatureSet * DwellingActor::getDwellingCreatures(const CGDwelling * dwelling, bool waitForGrowth)

client/PlayerLocalState.cpp

@@ -347,7 +347,7 @@ void PlayerLocalState::deserialize(const JsonNode & source)
 {
 	// this method must be called after player state has been initialized
 	assert(currentSelection != nullptr);
-	assert(!ownedTowns.empty() || wanderingHeroes.empty());
+	assert(!ownedTowns.empty() || !wanderingHeroes.empty());
 
 	auto oldHeroes = wanderingHeroes;
 	auto oldTowns = ownedTowns;

lib/gameState/CGameStateCampaign.cpp

@@ -370,7 +370,7 @@ void CGameStateCampaign::replaceHeroesPlaceholders()
 			heroToPlace->tempOwner = heroPlaceholder->tempOwner;
 		heroToPlace->setAnchorPos(heroPlaceholder->anchorPos());
 		heroToPlace->setHeroType(heroToPlace->getHeroTypeID());
-		heroToPlace->appearance = VLC->objtypeh->getHandlerFor(Obj::HERO, heroToPlace->getHeroTypeID())->getTemplates().front();
+		heroToPlace->appearance = heroToPlace->getObjectHandler()->getTemplates().front();
 
 		gameState->map->removeBlockVisTiles(heroPlaceholder, true);
 		gameState->map->objects[heroPlaceholder->id.getNum()] = nullptr;

lib/mapObjectConstructors/AObjectTypeHandler.cpp

@@ -133,8 +133,6 @@ void AObjectTypeHandler::preInitObject(CGObjectInstance * obj) const
 {
 	obj->ID = Obj(type);
 	obj->subID = subtype;
-	obj->typeName = typeName;
-	obj->subTypeName = getJsonKey();
 	obj->blockVisit = blockVisit;
 	obj->removable = removable;
 }

lib/mapObjects/CGHeroInstance.cpp

@@ -339,12 +339,20 @@ void CGHeroInstance::initHero(vstd::RNG & rand, const HeroTypeID & SUBID)
 	initHero(rand);
 }
 
+TObjectTypeHandler CGHeroInstance::getObjectHandler() const
+{
+	if (ID == Obj::HERO)
+		return VLC->objtypeh->getHandlerFor(ID, getHeroClass()->getIndex());
+	else // prison or random hero
+		return VLC->objtypeh->getHandlerFor(ID, 0);
+}
+
 void CGHeroInstance::initHero(vstd::RNG & rand)
 {
 	assert(validTypes(true));
 	
 	if (ID == Obj::HERO)
-		appearance = VLC->objtypeh->getHandlerFor(Obj::HERO, getHeroClass()->getIndex())->getTemplates().front();
+		appearance = getObjectHandler()->getTemplates().front();
 
 	if(!vstd::contains(spells, SpellID::PRESET))
 	{

lib/mapObjects/CGHeroInstance.h

@@ -307,6 +307,8 @@ public:
 	std::string getHoverText(PlayerColor player) const override;
 	std::string getMovementPointsTextIfOwner(PlayerColor player) const;
 
+	TObjectTypeHandler getObjectHandler() const override;
+
 	void afterAddToMap(CMap * map) override;
 	void afterRemoveFromMap(CMap * map) override;
 

lib/mapObjects/CGObjectInstance.cpp

@@ -192,6 +192,16 @@ TObjectTypeHandler CGObjectInstance::getObjectHandler() const
 	return VLC->objtypeh->getHandlerFor(ID, subID);
 }
 
+std::string CGObjectInstance::getTypeName() const
+{
+	return getObjectHandler()->getTypeName();
+}
+
+std::string CGObjectInstance::getSubtypeName() const
+{
+	return getObjectHandler()->getSubTypeName();
+}
+
 void CGObjectInstance::setPropertyDer( ObjProperty what, ObjPropertyID identifier )
 {}
 
@@ -350,8 +360,11 @@ void CGObjectInstance::serializeJson(JsonSerializeFormat & handler)
 	//only save here, loading is handled by map loader
 	if(handler.saving)
 	{
-		handler.serializeString("type", typeName);
-		handler.serializeString("subtype", subTypeName);
+		std::string ourTypeName = getTypeName();
+		std::string ourSubtypeName = getSubtypeName();
+
+		handler.serializeString("type", ourTypeName);
+		handler.serializeString("subtype", ourSubtypeName);
 
 		handler.serializeInt("x", pos.x);
 		handler.serializeInt("y", pos.y);

lib/mapObjects/CGObjectInstance.h

@@ -43,8 +43,6 @@ public:
 	int3 pos;
 
 	std::string instanceName;
-	std::string typeName;
-	std::string subTypeName;
 
 	CGObjectInstance(IGameCallback *cb);
 	~CGObjectInstance() override;
@@ -52,6 +50,9 @@ public:
 	MapObjectID getObjGroupIndex() const override;
 	MapObjectSubID getObjTypeIndex() const override;
 
+	std::string getTypeName() const;
+	std::string getSubtypeName() const;
+
 	/// "center" tile from which the sight distance is calculated
 	int3 getSightCenter() const;
 	/// If true hero can visit this object only from neighbouring tiles and can't stand on this object
@@ -100,7 +101,7 @@ public:
 	std::optional<AudioPath> getVisitSound(vstd::RNG & rng) const;
 	std::optional<AudioPath> getRemovalSound(vstd::RNG & rng) const;
 
-	TObjectTypeHandler getObjectHandler() const;
+	virtual TObjectTypeHandler getObjectHandler() const;
 
 	/** VIRTUAL METHODS **/
 
@@ -142,8 +143,12 @@ public:
 	template <typename Handler> void serialize(Handler &h)
 	{
 		h & instanceName;
-		h & typeName;
-		h & subTypeName;
+		if (h.version < Handler::Version::REMOVE_OBJECT_TYPENAME)
+		{
+			std::string unused;
+			h & unused;
+			h & unused;
+		}
 		h & pos;
 		h & ID;
 		subID.serializeIdentifier(h, ID);

lib/mapping/CMap.cpp

@@ -608,7 +608,7 @@ void CMap::setUniqueInstanceName(CGObjectInstance * obj)
 	auto uid = uidCounter++;
 
 	boost::format fmt("%s_%d");
-	fmt % obj->typeName % uid;
+	fmt % obj->getTypeName() % uid;
 	obj->instanceName = fmt.str();
 }
 

lib/rmg/modificators/ObstaclePlacer.cpp

@@ -153,7 +153,7 @@ void ObstaclePlacer::postProcess(const rmg::Object & object)
 	riverManager = zone.getModificator<RiverPlacer>();
 	if(riverManager)
 	{
-		const auto objTypeName = object.instances().front()->object().typeName;
+		const auto objTypeName = object.instances().front()->object().getTypeName();
 		if(objTypeName == "mountain")
 			riverManager->riverSource().unite(object.getArea());
 		else if(objTypeName == "lake")

lib/serializer/ESerializationVersion.h

@@ -64,6 +64,7 @@ enum class ESerializationVersion : int32_t
 	SPELL_RESEARCH, // 865 - spell research
 	LOCAL_PLAYER_STATE_DATA, // 866 - player state contains arbitrary client-side data
 	REMOVE_TOWN_PTR, // 867 - removed pointer to CTown from CGTownInstance
+	REMOVE_OBJECT_TYPENAME, // 868 - remove typename from CGObjectInstance
 
-	CURRENT = REMOVE_TOWN_PTR
+	CURRENT = REMOVE_OBJECT_TYPENAME
 };

mapeditor/inspector/inspector.cpp

@@ -470,8 +470,6 @@ void Inspector::updateProperties()
 	addProperty("ID", obj->ID.getNum());
 	addProperty("SubID", obj->subID);
 	addProperty("InstanceName", obj->instanceName);
-	addProperty("TypeName", obj->typeName);
-	addProperty("SubTypeName", obj->subTypeName);
 	
 	if(obj->ID != Obj::HERO_PLACEHOLDER && !dynamic_cast<CGHeroInstance*>(obj))
 	{

mapeditor/mapcontroller.cpp

@@ -112,13 +112,6 @@ void MapController::repairMap(CMap * map) const
 	allImpactedObjects.insert(allImpactedObjects.end(), map->predefinedHeroes.begin(), map->predefinedHeroes.end());
 	for(auto obj : allImpactedObjects)
 	{
-		//setup proper names (hero name will be fixed later
-		if(obj->ID != Obj::HERO && obj->ID != Obj::PRISON && (obj->typeName.empty() || obj->subTypeName.empty()))
-		{
-			auto handler = VLC->objtypeh->getHandlerFor(obj->ID, obj->subID);
-			obj->typeName = handler->getTypeName();
-			obj->subTypeName = handler->getSubTypeName();
-		}
 		//fix flags
 		if(obj->getOwner() == PlayerColor::UNFLAGGABLE)
 		{
@@ -142,18 +135,7 @@ void MapController::repairMap(CMap * map) const
 
 			auto const & type = VLC->heroh->objects[nih->subID];
 			assert(type->heroClass);
-			//TODO: find a way to get proper type name
-			if(obj->ID == Obj::HERO)
-			{
-				nih->typeName = "hero";
-				nih->subTypeName = type->heroClass->getJsonKey();
-			}
-			if(obj->ID == Obj::PRISON)
-			{
-				nih->typeName = "prison";
-				nih->subTypeName = "prison";
-				nih->subID = 0;
-			}
+
 			if(nih->ID == Obj::HERO) //not prison
 				nih->appearance = VLC->objtypeh->getHandlerFor(Obj::HERO, type->heroClass->getIndex())->getTemplates().front();
 			//fix spellbook
@@ -568,8 +550,6 @@ bool MapController::canPlaceObject(int level, CGObjectInstance * newObj, QString
 	
 	if(newObj->ID == Obj::GRAIL && objCounter >= 1) //special case for grail
 	{
-		auto typeName = QString::fromStdString(newObj->typeName);
-		auto subTypeName = QString::fromStdString(newObj->subTypeName);
 		error = QObject::tr("There can only be one grail object on the map.");
 		return false; //maplimit reached
 	}

test/map/MapComparer.cpp

@@ -203,8 +203,8 @@ void MapComparer::compareObject(const CGObjectInstance * actual, const CGObjectI
 	EXPECT_EQ(actual->instanceName, expected->instanceName);
 	EXPECT_EQ(typeid(actual).name(), typeid(expected).name());//todo: remove and use just comparison
 
-	std::string actualFullID = boost::str(boost::format("%s(%d)|%s(%d) %d") % actual->typeName % actual->ID % actual->subTypeName % actual->subID % actual->tempOwner);
-	std::string expectedFullID = boost::str(boost::format("%s(%d)|%s(%d) %d") % expected->typeName % expected->ID % expected->subTypeName % expected->subID % expected->tempOwner);
+	std::string actualFullID = boost::str(boost::format("(%d)|(%d) %d") % actual->ID % actual->subID % actual->tempOwner);
+	std::string expectedFullID = boost::str(boost::format("(%d)|(%d) %d") % expected->ID % expected->subID % expected->tempOwner);
 
 	EXPECT_EQ(actualFullID, expectedFullID);