NewObject pack now receives visitable position instead of h3m position

client/NetPacksClient.cpp

@@ -961,7 +961,7 @@ void ApplyClientNetPackVisitor::visitNewObject(NewObject & pack)
 {
 	cl.invalidatePaths();
 
-	const CGObjectInstance *obj = cl.getObj(pack.id);
+	const CGObjectInstance *obj = cl.getObj(pack.createdObjectID);
 	if(CGI->mh)
 		CGI->mh->onObjectFadeIn(obj);
 

lib/NetPacks.h

@@ -745,11 +745,14 @@ struct DLL_LINKAGE NewObject : public CPackForClient
 {
 	void applyGs(CGameState * gs);
 
+	/// Object ID to create
 	Obj ID;
+	/// Object secondary ID to create
 	ui32 subID = 0;
-	int3 pos;
+	/// Position of visitable tile of created object
+	int3 targetPos;
 
-	ObjectInstanceID id; //used locally, filled during applyGs
+	ObjectInstanceID createdObjectID; //used locally, filled during applyGs
 
 	virtual void visitTyped(ICPackVisitor & visitor) override;
 
@@ -757,7 +760,7 @@ struct DLL_LINKAGE NewObject : public CPackForClient
 	{
 		h & ID;
 		h & subID;
-		h & pos;
+		h & targetPos;
 	}
 };
 

lib/NetPacksLib.cpp

@@ -1492,57 +1492,52 @@ void NewObject::applyGs(CGameState *gs)
 {
 	TerrainId terrainType = ETerrainId::NONE;
 
-	if(ID == Obj::BOAT && !gs->isInTheMap(pos)) //special handling for bug #3060 - pos outside map but visitablePos is not
+	if (!gs->isInTheMap(targetPos))
 	{
-		CGObjectInstance testObject = CGObjectInstance();
-		testObject.pos = pos;
-		testObject.appearance = VLC->objtypeh->getHandlerFor(ID, subID)->getTemplates(ETerrainId::WATER).front();
-
-		[[maybe_unused]] const int3 previousXAxisTile = CGBoat::translatePos(pos, true);
-		assert(gs->isInTheMap(previousXAxisTile) && (testObject.visitablePos() == previousXAxisTile));
-	}
-	else
-	{
-		const TerrainTile & t = gs->map->getTile(pos);
-		terrainType = t.terType->getId();
+		logGlobal->error("Attempt to create object outside map at %s!", targetPos.toString());
+		return;
 	}
 
+	const TerrainTile & t = gs->map->getTile(targetPos);
+	terrainType = t.terType->getId();
+
 	auto handler = VLC->objtypeh->getHandlerFor(ID, subID);
 	CGObjectInstance * o = handler->create();
 	handler->configureObject(o, gs->getRandomGenerator());
 	
-	switch(ID)
+	if (ID == Obj::MONSTER) //probably more options will be needed
 	{
-	case Obj::BOAT:
-		terrainType = ETerrainId::WATER; //TODO: either boat should only spawn on water, or all water objects should be handled this way
-		break;
-			
-	case Obj::MONSTER: //probably more options will be needed
-		{
-			//CStackInstance hlp;
-			auto * cre = dynamic_cast<CGCreature *>(o);
-			//cre->slots[0] = hlp;
-			assert(cre);
-			cre->notGrowingTeam = cre->neverFlees = false;
-			cre->character = 2;
-			cre->gainedArtifact = ArtifactID::NONE;
-			cre->identifier = -1;
-			cre->addToSlot(SlotID(0), new CStackInstance(CreatureID(subID), -1)); //add placeholder stack
-		}
-		break;
+		//CStackInstance hlp;
+		auto * cre = dynamic_cast<CGCreature *>(o);
+		//cre->slots[0] = hlp;
+		assert(cre);
+		cre->notGrowingTeam = cre->neverFlees = false;
+		cre->character = 2;
+		cre->gainedArtifact = ArtifactID::NONE;
+		cre->identifier = -1;
+		cre->addToSlot(SlotID(0), new CStackInstance(CreatureID(subID), -1)); //add placeholder stack
 	}
+
+	assert(!handler->getTemplates(terrainType).empty());
+
+	if (!handler->getTemplates(terrainType).empty())
+		o->appearance = handler->getTemplates(terrainType).front();
+	else
+		o->appearance = handler->getTemplates().front();
+
+	o->id = ObjectInstanceID(static_cast<si32>(gs->map->objects.size()));
 	o->ID = ID;
 	o->subID = subID;
-	o->pos = pos;
-	o->appearance = handler->getTemplates(terrainType).front();
-	id = o->id = ObjectInstanceID(static_cast<si32>(gs->map->objects.size()));
+	o->pos = targetPos - o->getVisitableOffset();
 
 	gs->map->objects.emplace_back(o);
 	gs->map->addBlockVisTiles(o);
 	o->initObj(gs->getRandomGenerator());
 	gs->map->calculateGuardingGreaturePositions();
 
-	logGlobal->debug("Added object id=%d; address=%x; name=%s", id, (intptr_t)o, o->getObjectName());
+	createdObjectID = o->id;
+
+	logGlobal->debug("Added object id=%d; address=%x; name=%s", o->id, (intptr_t)o, o->getObjectName());
 }
 
 void NewArtifact::applyGs(CGameState *gs)

lib/mapObjects/CGHeroInstance.cpp

@@ -454,6 +454,7 @@ void CGHeroInstance::onHeroVisit(const CGHeroInstance * h) const
 				NewObject no;
 				no.ID = Obj::BOAT;
 				no.subID = getBoatType().getNum();
+				no.targetPos = boatPos;
 
 				cb->sendAndApply(&no);
 

lib/mapObjects/MiscObjects.cpp

@@ -1294,22 +1294,6 @@ CGBoat::CGBoat()
 	layer = EPathfindingLayer::EEPathfindingLayer::SAIL;
 }
 
-int3 CGBoat::translatePos(const int3& pos, bool reverse /* = false */)
-{
-	//pos - offset we want to place the boat at the map
-	//returned value - actual position as seen by game mechanics
-
-	//If reverse = true, then it's the opposite.
-	if (!reverse)
-	{
-		return pos + int3(1, 0, 0);
-	}
-	else
-	{
-		return pos - int3(1, 0, 0);
-	}
-}
-
 void CGSirens::initObj(CRandomGenerator & rand)
 {
 	blockVisit = true;
@@ -1409,8 +1393,7 @@ void CGShipyard::serializeJsonOptions(JsonSerializeFormat& handler)
 
 BoatId CGShipyard::getBoatType() const
 {
-	// In H3, external shipyard will always create same boat as castle
-	return EBoatId::CASTLE;
+	return createdBoat;
 }
 
 void CCartographer::onHeroVisit( const CGHeroInstance * h ) const

lib/mapObjects/MiscObjects.h

@@ -359,7 +359,6 @@ public:
 	std::array<std::string, PlayerColor::PLAYER_LIMIT_I> flagAnimations;
 
 	CGBoat();
-	static int3 translatePos(const int3 &pos, bool reverse = false);
 
 	template <typename Handler> void serialize(Handler &h, const int version)
 	{

lib/spells/AdventureSpellMechanics.cpp

@@ -201,7 +201,7 @@ ESpellCastResult SummonBoatMechanics::applyAdventureEffects(SpellCastEnvironment
 	{
 		ChangeObjPos cop;
 		cop.objid = nearest->id;
-		cop.nPos = CGBoat::translatePos(summonPos);
+		cop.nPos = summonPos;
 		env->apply(&cop);
 	}
 	else if(schoolLevel < 2) //none or basic level -> cannot create boat :(
@@ -216,7 +216,7 @@ ESpellCastResult SummonBoatMechanics::applyAdventureEffects(SpellCastEnvironment
 		NewObject no;
 		no.ID = Obj::BOAT;
 		no.subID = BoatId(EBoatId::NECROPOLIS);
-		no.pos = CGBoat::translatePos(summonPos);
+		no.targetPos = summonPos;
 		env->apply(&no);
 	}
 	return ESpellCastResult::OK;

server/CGameHandler.cpp

@@ -4432,7 +4432,7 @@ bool CGameHandler::hireHero(const CGObjectInstance *obj, ui8 hid, PlayerColor pl
 		NewObject no;
 		no.ID = Obj::BOAT;
 		no.subID = nh->getBoatType().getNum();
-		no.pos = hr.tile + int3(1,0,0);
+		no.targetPos = obj->visitablePos();
 		sendAndApply(&no);
 
 		hr.boatId = getTopObj(hr.tile)->id;
@@ -5660,7 +5660,7 @@ bool CGameHandler::buildBoat(ObjectInstanceID objid, PlayerColor playerID)
 	NewObject no;
 	no.ID = Obj::BOAT;
 	no.subID = obj->getBoatType().getNum();
-	no.pos = tile + int3(1,0,0);
+	no.targetPos = tile;
 	sendAndApply(&no);
 
 	return true;
@@ -5806,7 +5806,7 @@ bool CGameHandler::dig(const CGHeroInstance *h)
 	//create a hole
 	NewObject no;
 	no.ID = Obj::HOLE;
-	no.pos = h->visitablePos();
+	no.targetPos = h->visitablePos();
 	no.subID = 0;
 	sendAndApply(&no);
 
@@ -7409,7 +7409,7 @@ const ObjectInstanceID CGameHandler::putNewObject(Obj ID, int subID, int3 pos)
 	NewObject no;
 	no.ID = ID; //creature
 	no.subID= subID;
-	no.pos = pos;
+	no.targetPos = pos;
 	sendAndApply(&no);
-	return no.id; //id field will be filled during applying on gs
+	return no.createdObjectID; //id field will be filled during applying on gs
 }