Review & cleanup hiding / displaying map objects

lib/gameState/CGameState.cpp

@@ -583,7 +583,6 @@ void CGameState::removeHeroPlaceholders()
 	// remove any hero placeholders that remain on map after (potential) campaign heroes placement
 	for(auto obj : map->getObjects<CGHeroPlaceholder>())
 	{
-		map->removeBlockVisTiles(obj, true);
 		map->eraseObject(obj->id);
 	}
 }
@@ -951,11 +950,8 @@ void CGameState::placeHeroesInTowns()
 				// assume that this hero should be visiting the town (H3M format quirk) and move hero to correct position
 				if (heroOnTownBlockableTile)
 				{
-					map->removeBlockVisTiles(h);
 					int3 correctedPos = h->convertFromVisitablePos(t->visitablePos());
-					h->setAnchorPos(correctedPos);
-					map->addBlockVisTiles(h);
-
+					map->moveObject(h->id, correctedPos);
 					assert(t->visitableAt(h->visitablePos()));
 				}
 			}

lib/mapObjects/CGObjectInstance.cpp

@@ -125,7 +125,7 @@ void CGObjectInstance::setType(MapObjectID newID, MapObjectSubID newSubID)
 	auto &tile = cb->gameState()->getMap().getTile(position);
 
 	//recalculate blockvis tiles - new appearance might have different blockmap than before
-	cb->gameState()->getMap().removeBlockVisTiles(this, true);
+	cb->gameState()->getMap().hideObject(this);
 	auto handler = LIBRARY->objtypeh->getHandlerFor(newID, newSubID);
 
 	if(!handler->getTemplates(tile.getTerrainID()).empty())
@@ -155,7 +155,7 @@ void CGObjectInstance::setType(MapObjectID newID, MapObjectSubID newSubID)
 	this->ID = Obj(newID);
 	this->subID = newSubID;
 
-	cb->gameState()->getMap().addBlockVisTiles(this);
+	cb->gameState()->getMap().showObject(this);
 }
 
 void CGObjectInstance::pickRandomObject(vstd::RNG & rand)

lib/mapping/CMap.cpp

@@ -184,7 +184,7 @@ CMap::CMap(IGameCallback * cb)
 
 CMap::~CMap() = default;
 
-void CMap::removeBlockVisTiles(CGObjectInstance * obj, bool total)
+void CMap::hideObject(CGObjectInstance * obj)
 {
 	const int zVal = obj->anchorPos().z;
 	for(int fx = 0; fx < obj->getWidth(); ++fx)
@@ -196,17 +196,14 @@ void CMap::removeBlockVisTiles(CGObjectInstance * obj, bool total)
 			if(xVal>=0 && xVal < width && yVal>=0 && yVal < height)
 			{
 				TerrainTile & curt = terrain[zVal][xVal][yVal];
-				if(total || obj->visitableAt(int3(xVal, yVal, zVal)))
-					curt.visitableObjects -= obj->id;
-
-				if(total || obj->blockingAt(int3(xVal, yVal, zVal)))
-					curt.blockingObjects -= obj->id;
+				curt.visitableObjects -= obj->id;
+				curt.blockingObjects -= obj->id;
 			}
 		}
 	}
 }
 
-void CMap::addBlockVisTiles(CGObjectInstance * obj)
+void CMap::showObject(CGObjectInstance * obj)
 {
 	const int zVal = obj->anchorPos().z;
 	for(int fx = 0; fx < obj->getWidth(); ++fx)
@@ -219,10 +216,16 @@ void CMap::addBlockVisTiles(CGObjectInstance * obj)
 			{
 				TerrainTile & curt = terrain[zVal][xVal][yVal];
 				if(obj->visitableAt(int3(xVal, yVal, zVal)))
+				{
+					assert(!vstd::contains(curt.visitableObjects, obj->id));
 					curt.visitableObjects.push_back(obj->id);
+				}
 
 				if(obj->blockingAt(int3(xVal, yVal, zVal)))
+				{
+					assert(!vstd::contains(curt.blockingObjects, obj->id));
 					curt.blockingObjects.push_back(obj->id);
+				}
 			}
 		}
 	}
@@ -523,7 +526,7 @@ void CMap::addNewObject(std::shared_ptr<CGObjectInstance> obj)
 		objects[obj->id.getNum()] = obj;
 
 	instanceNames[obj->instanceName] = obj;
-	addBlockVisTiles(obj.get());
+	showObject(obj.get());
 
 	//TODO: how about defeated heroes recruited again?
 
@@ -533,16 +536,16 @@ void CMap::addNewObject(std::shared_ptr<CGObjectInstance> obj)
 void CMap::moveObject(ObjectInstanceID target, const int3 & dst)
 {
 	auto obj = objects.at(target).get();
-	removeBlockVisTiles(obj);
+	hideObject(obj);
 	obj->setAnchorPos(dst);
-	addBlockVisTiles(obj);
+	showObject(obj);
 }
 
 std::shared_ptr<CGObjectInstance> CMap::removeObject(ObjectInstanceID oldObject)
 {
 	auto obj = objects.at(oldObject);
 
-	removeBlockVisTiles(obj.get());
+	hideObject(obj.get());
 	instanceNames.erase(obj->instanceName);
 	obj->afterRemoveFromMap(this);
 
@@ -585,11 +588,11 @@ std::shared_ptr<CGObjectInstance> CMap::replaceObject(ObjectInstanceID oldObject
 
 	newObject->id = oldObjectID;
 
-	removeBlockVisTiles(oldObject.get(), true);
+	hideObject(oldObject.get());
 	instanceNames.erase(oldObject->instanceName);
 
 	objects.at(oldObjectID.getNum()) = newObject;
-	addBlockVisTiles(newObject.get());
+	showObject(newObject.get());
 	instanceNames[newObject->instanceName] = newObject;
 
 	oldObject->afterRemoveFromMap(this);
@@ -604,7 +607,7 @@ std::shared_ptr<CGObjectInstance> CMap::eraseObject(ObjectInstanceID oldObjectID
 
 	instanceNames.erase(oldObject->instanceName);
 	objects.at(oldObjectID) = nullptr;
-	removeBlockVisTiles(oldObject.get(), true);
+	hideObject(oldObject.get());
 	oldObject->afterRemoveFromMap(this);
 
 	return oldObject;

lib/mapping/CMap.h

@@ -93,8 +93,6 @@ public:
 	bool checkForVisitableDir(const int3 & src, const TerrainTile * pom, const int3 & dst) const;
 	int3 guardingCreaturePosition (int3 pos) const;
 
-	void addBlockVisTiles(CGObjectInstance * obj);
-	void removeBlockVisTiles(CGObjectInstance * obj, bool total = false);
 	void calculateGuardingGreaturePositions();
 
 	CArtifactInstance * createScroll(const SpellID & spellId);
@@ -115,6 +113,9 @@ public:
 	void addNewObject(std::shared_ptr<CGObjectInstance> obj);
 	void moveObject(ObjectInstanceID target, const int3 & dst);
 
+	void hideObject(CGObjectInstance * obj);
+	void showObject(CGObjectInstance * obj);
+
 	/// Remove objects and shifts object indicies.
 	/// Only for use in map editor / RMG
 	std::shared_ptr<CGObjectInstance> removeObject(ObjectInstanceID oldObject);

lib/networkPacks/NetPacksLib.cpp

@@ -1044,9 +1044,7 @@ void ChangeObjPos::applyGs(CGameState *gs)
 		logNetwork->error("Wrong ChangeObjPos: object %d doesn't exist!", objid.getNum());
 		return;
 	}
-	gs->getMap().removeBlockVisTiles(obj);
-	obj->setAnchorPos(nPos + obj->getVisitableOffset());
-	gs->getMap().addBlockVisTiles(obj);
+	gs->getMap().moveObject(objid, nPos + obj->getVisitableOffset());
 }
 
 void ChangeObjectVisitors::applyGs(CGameState *gs)
@@ -1175,8 +1173,6 @@ void RemoveObject::applyGs(CGameState *gs)
 {
 	CGObjectInstance *obj = gs->getObjInstance(objectID);
 	logGlobal->debug("removing object id=%d; address=%x; name=%s", objectID, (intptr_t)obj, obj->getObjectName());
-	//unblock tiles
-	gs->getMap().removeBlockVisTiles(obj);
 
 	if (initiator.isValidPlayer())
 		gs->getPlayerState(initiator)->destroyedObjects.insert(objectID);
@@ -1320,7 +1316,7 @@ void TryMoveHero::applyGs(CGameState *gs)
 		auto * boat = dynamic_cast<CGBoat *>(topObject);
 		assert(boat);
 
-		gs->getMap().removeBlockVisTiles(boat); //hero blockvis mask will be used, we don't need to duplicate it with boat
+		gs->getMap().hideObject(boat); //hero blockvis mask will be used, we don't need to duplicate it with boat
 		h->setBoat(boat);
 	}
 	else if(result == DISEMBARK) //hero leaves boat to destination tile
@@ -1328,17 +1324,17 @@ void TryMoveHero::applyGs(CGameState *gs)
 		auto * b = h->getBoat();
 		b->direction = h->moveDir;
 		b->pos = start;
-		gs->getMap().addBlockVisTiles(b);
+		gs->getMap().showObject(b);
 		h->setBoat(nullptr);
 	}
 
 	if(start!=end && (result == SUCCESS || result == TELEPORTATION || result == EMBARK || result == DISEMBARK))
 	{
-		gs->getMap().removeBlockVisTiles(h);
-		h->pos = end;
+		gs->getMap().hideObject(h);
+		h->setAnchorPos(end);
 		if(auto * b = h->getBoat())
-			b->pos = end;
-		gs->getMap().addBlockVisTiles(h);
+			b->setAnchorPos(end);
+		gs->getMap().showObject(h);
 	}
 
 	auto & fogOfWarMap = gs->getPlayerTeam(h->getOwner())->fogOfWarMap;
@@ -1403,13 +1399,10 @@ void SetHeroesInTown::applyGs(CGameState *gs)
 		t->setGarrisonedHero(g);
 
 	if(v)
-	{
-		gs->getMap().addBlockVisTiles(v);
-	}
+		gs->getMap().showObject(v);
+
 	if(g)
-	{
-		gs->getMap().removeBlockVisTiles(g);
-	}
+		gs->getMap().hideObject(g);
 }
 
 void HeroRecruited::applyGs(CGameState *gs)
@@ -1424,7 +1417,7 @@ void HeroRecruited::applyGs(CGameState *gs)
 		auto * boat = dynamic_cast<CGBoat *>(obj);
 		if (boat)
 		{
-			gs->getMap().removeBlockVisTiles(boat);
+			gs->getMap().hideObject(boat);
 			h->setBoat(boat);
 		}
 	}
@@ -1453,7 +1446,7 @@ void GiveHero::applyGs(CGameState *gs)
 		auto * boat = dynamic_cast<CGBoat *>(obj);
 		if (boat)
 		{
-			gs->getMap().removeBlockVisTiles(boat);
+			gs->getMap().hideObject(boat);
 			h->setBoat(boat);
 		}
 	}
@@ -1463,7 +1456,7 @@ void GiveHero::applyGs(CGameState *gs)
 	h->attachTo(*gs->getPlayerState(player));
 
 	auto oldVisitablePos = h->visitablePos();
-	gs->getMap().removeBlockVisTiles(h,true);
+	gs->getMap().hideObject(h);
 	h->updateAppearance();
 
 	h->setOwner(player);
@@ -1472,7 +1465,7 @@ void GiveHero::applyGs(CGameState *gs)
 	gs->getMap().heroAddedToMap(h);
 	gs->getPlayerState(h->getOwner())->addOwnedObject(h);
 
-	gs->getMap().addBlockVisTiles(h);
+	gs->getMap().showObject(h);
 	h->setVisitedTown(nullptr, false);
 }