New objects are now created and initialized on server and sent to client

client/Client.h

@@ -161,7 +161,7 @@ public:
 
 	void changeSpells(const CGHeroInstance * hero, bool give, const std::set<SpellID> & spells) override {};
 	bool removeObject(const CGObjectInstance * obj, const PlayerColor & initiator) override {return false;};
-	void createObject(const int3 & visitablePosition, const PlayerColor & initiator, MapObjectID type, MapObjectSubID subtype) override {};
+	void createBoat(const int3 & visitablePosition, BoatId type, PlayerColor initiator) override {};
 	void setOwner(const CGObjectInstance * obj, PlayerColor owner) override {};
 	void giveExperience(const CGHeroInstance * hero, TExpType val) override {};
 	void changePrimSkill(const CGHeroInstance * hero, PrimarySkill which, si64 val, bool abs = false) override {};

client/NetPacksClient.cpp

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

lib/IGameCallback.h

@@ -92,7 +92,7 @@ public:
 
 	virtual void changeSpells(const CGHeroInstance * hero, bool give, const std::set<SpellID> &spells)=0;
 	virtual bool removeObject(const CGObjectInstance * obj, const PlayerColor & initiator) = 0;
-	virtual void createObject(const int3 & visitablePosition, const PlayerColor & initiator, MapObjectID type, MapObjectSubID subtype) = 0;
+	virtual void createBoat(const int3 & visitablePosition, BoatId type, PlayerColor initiator) = 0;
 	virtual void setOwner(const CGObjectInstance * objid, PlayerColor owner)=0;
 	virtual void giveExperience(const CGHeroInstance * hero, TExpType val) =0;
 	virtual void changePrimSkill(const CGHeroInstance * hero, PrimarySkill which, si64 val, bool abs=false)=0;

lib/mapObjects/CGHeroInstance.cpp

@@ -532,7 +532,7 @@ void CGHeroInstance::onHeroVisit(const CGHeroInstance * h) const
 				if (!boat)
 				{
 					//Create a new boat for hero
-					cb->createObject(boatPos, h->getOwner(), Obj::BOAT, getBoatType().getNum());
+					cb->createBoat(boatPos, getBoatType(), h->getOwner());
 					boatId = cb->getTopObj(boatPos)->id;
 				}
 			}
@@ -614,11 +614,6 @@ void CGHeroInstance::pickRandomObject(vstd::RNG & rand)
 	this->subID = oldSubID;
 }
 
-void CGHeroInstance::initObj(vstd::RNG & rand)
-{
-
-}
-
 void CGHeroInstance::recreateSecondarySkillsBonuses()
 {
 	auto secondarySkillsBonuses = getBonuses(Selector::sourceType()(BonusSource::SECONDARY_SKILL));

lib/mapObjects/CGHeroInstance.h

@@ -293,7 +293,6 @@ public:
 	void boatDeserializationFix();
 	void deserializationFix();
 
-	void initObj(vstd::RNG & rand) override;
 	void pickRandomObject(vstd::RNG & rand) override;
 	void onHeroVisit(const CGHeroInstance * h) const override;
 	std::string getObjectName() const override;

lib/networkPacks/NetPacksLib.cpp

@@ -1421,7 +1421,6 @@ void HeroRecruited::applyGs(CGameState * gs) const
 
 	h->setOwner(player);
 	h->pos = tile;
-	h->initObj(gs->getRandomGenerator());
 
 	if(h->id == ObjectInstanceID())
 	{
@@ -1475,59 +1474,13 @@ void GiveHero::applyGs(CGameState * gs) const
 
 void NewObject::applyGs(CGameState *gs)
 {
-	TerrainId terrainType = ETerrainId::NONE;
+	newObject->id = ObjectInstanceID(static_cast<si32>(gs->map->objects.size()));
 
-	if (!gs->isInTheMap(targetPos))
+	gs->map->objects.emplace_back(newObject);
-	{
+	gs->map->addBlockVisTiles(newObject);
-		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(gs->callback, nullptr);
-	handler->configureObject(o, gs->getRandomGenerator());
-	assert(o->ID == this->ID);
-	
-	if (ID == 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(subID.getNum(), -1)); //add placeholder stack
-	}
-
-	assert(!handler->getTemplates(terrainType).empty());
-	if (handler->getTemplates().empty())
-	{
-		logGlobal->error("Attempt to create object (%d %d) with no templates!", ID, subID.getNum());
-		return;
-	}
-
-	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->pos = targetPos + o->getVisitableOffset();
-
-	gs->map->objects.emplace_back(o);
-	gs->map->addBlockVisTiles(o);
-	o->initObj(gs->getRandomGenerator());
 	gs->map->calculateGuardingGreaturePositions();
 
-	createdObjectID = o->id;
+	logGlobal->debug("Added object id=%d; address=%x; name=%s", newObject->id, (intptr_t)newObject, newObject->getObjectName());
-
-	logGlobal->debug("Added object id=%d; address=%x; name=%s", o->id, (intptr_t)o, o->getObjectName());
 }
 
 void NewArtifact::applyGs(CGameState *gs)

lib/networkPacks/PacksForClient.h

@@ -787,23 +787,15 @@ struct DLL_LINKAGE NewObject : public CPackForClient
 	void applyGs(CGameState * gs);
 
 	/// Object ID to create
-	MapObjectID ID;
+	CGObjectInstance * newObject;
-	/// Object secondary ID to create
-	MapObjectSubID subID;
-	/// Position of visitable tile of created object
-	int3 targetPos;
 	/// Which player initiated creation of this object
 	PlayerColor initiator;
 
-	ObjectInstanceID createdObjectID; //used locally, filled during applyGs
-
 	void visitTyped(ICPackVisitor & visitor) override;
 
 	template <typename Handler> void serialize(Handler & h)
 	{
-		h & ID;
+		h & newObject;
-		subID.serializeIdentifier(h, ID);
-		h & targetPos;
 		h & initiator;
 	}
 };

lib/spells/AdventureSpellMechanics.cpp

@@ -238,12 +238,7 @@ ESpellCastResult SummonBoatMechanics::applyAdventureEffects(SpellCastEnvironment
 	}
 	else //create boat
 	{
-		NewObject no;
+		env->createBoat(summonPos, BoatId::NECROPOLIS, parameters.caster->getCasterOwner());
-		no.ID = Obj::BOAT;
-		no.subID = BoatId::NECROPOLIS;
-		no.targetPos = summonPos;
-		no.initiator = parameters.caster->getCasterOwner();
-		env->apply(&no);
 	}
 	return ESpellCastResult::OK;
 }

lib/spells/ISpellMechanics.h

@@ -58,6 +58,7 @@ public:
 	virtual const CMap * getMap() const = 0;
 	virtual const CGameInfoCallback * getCb() const = 0;
 
+	virtual void createBoat(const int3 & visitablePosition, BoatId type, PlayerColor initiator) = 0;
 	virtual bool moveHero(ObjectInstanceID hid, int3 dst, EMovementMode mode) = 0;	//TODO: remove
 
 	virtual void genericQuery(Query * request, PlayerColor color, std::function<void(std::optional<int32_t>)> callback) = 0;//TODO: type safety on query, use generic query packet when implemented

server/CGameHandler.cpp

@@ -50,9 +50,12 @@
 
 #include "../lib/mapping/CMap.h"
 #include "../lib/mapping/CMapService.h"
+#include "../lib/mapObjects/CGCreature.h"
 #include "../lib/mapObjects/CGMarket.h"
 #include "../lib/mapObjects/CGTownInstance.h"
 #include "../lib/mapObjects/MiscObjects.h"
+#include "../lib/mapObjectConstructors/AObjectTypeHandler.h"
+#include "../lib/mapObjectConstructors/CObjectClassesHandler.h"
 #include "../lib/modding/ModIncompatibility.h"
 #include "../lib/networkPacks/StackLocation.h"
 #include "../lib/pathfinder/CPathfinder.h"
@@ -3684,7 +3687,7 @@ bool CGameHandler::buildBoat(ObjectInstanceID objid, PlayerColor playerID)
 	}
 
 	giveResources(playerID, -boatCost);
-	createObject(tile, playerID, Obj::BOAT, obj->getBoatType().getNum());
+	createBoat(tile, obj->getBoatType(), playerID);
 	return true;
 }
 
@@ -3809,7 +3812,7 @@ bool CGameHandler::dig(const CGHeroInstance *h)
 	if (h->diggingStatus() != EDiggingStatus::CAN_DIG) //checks for terrain and movement
 		COMPLAIN_RETF("Hero cannot dig (error code %d)!", static_cast<int>(h->diggingStatus()));
 
-	createObject(h->visitablePos(), h->getOwner(), Obj::HOLE, 0 );
+	createHole(h->visitablePos(), h->getOwner());
 
 	//take MPs
 	SetMovePoints smp;
@@ -4214,7 +4217,7 @@ void CGameHandler::spawnWanderingMonsters(CreatureID creatureID)
 		{
 			auto count = cre->getRandomAmount(std::rand);
 
-			createObject(*tile, PlayerColor::NEUTRAL, Obj::MONSTER, creatureID);
+			createWanderingMonster(*tile, creatureID);
 			auto monsterId = getTopObj(*tile)->id;
 
 			setObjPropertyValue(monsterId, ObjProperty::MONSTER_COUNT, count);
@@ -4413,13 +4416,71 @@ scripting::Pool * CGameHandler::getGlobalContextPool() const
 //}
 #endif
 
-void CGameHandler::createObject(const int3 & visitablePosition, const PlayerColor & initiator, MapObjectID type, MapObjectSubID subtype)
+
+CGObjectInstance * CGameHandler::createNewObject(const int3 & visitablePosition, MapObjectID objectID, MapObjectSubID subID)
 {
+	TerrainId terrainType = ETerrainId::NONE;
+
+	if (!gs->isInTheMap(visitablePosition))
+		throw std::runtime_error("Attempt to create object outside map at " + visitablePosition.toString());
+
+	const TerrainTile & t = gs->map->getTile(visitablePosition);
+	terrainType = t.terType->getId();
+
+	auto handler = VLC->objtypeh->getHandlerFor(objectID, subID);
+
+	CGObjectInstance * o = handler->create(gs->callback, nullptr);
+	handler->configureObject(o, getRandomGenerator());
+	assert(o->ID == objectID);
+
+	assert(!handler->getTemplates(terrainType).empty());
+	if (handler->getTemplates().empty())
+		throw std::runtime_error("Attempt to create object (" + std::to_string(objectID) + ", " + std::to_string(subID.getNum()) + ") with no templates!");
+
+	if (!handler->getTemplates(terrainType).empty())
+		o->appearance = handler->getTemplates(terrainType).front();
+	else
+		o->appearance = handler->getTemplates().front();
+
+
+	o->pos = visitablePosition + o->getVisitableOffset();
+	return o;
+}
+
+void CGameHandler::createWanderingMonster(const int3 & visitablePosition, CreatureID creature)
+{
+	auto createdObject = createNewObject(visitablePosition, Obj::MONSTER, creature);
+
+	auto * cre = dynamic_cast<CGCreature *>(createdObject);
+	assert(cre);
+	cre->notGrowingTeam = cre->neverFlees = false;
+	cre->character = 2;
+	cre->gainedArtifact = ArtifactID::NONE;
+	cre->identifier = -1;
+	cre->addToSlot(SlotID(0), new CStackInstance(creature, -1)); //add placeholder stack
+
+	newObject(createdObject, PlayerColor::NEUTRAL);
+}
+
+void CGameHandler::createBoat(const int3 & visitablePosition, BoatId type, PlayerColor initiator)
+{
+	auto createdObject = createNewObject(visitablePosition, Obj::BOAT, type);
+	newObject(createdObject, initiator);
+}
+
+void CGameHandler::createHole(const int3 & visitablePosition, PlayerColor initiator)
+{
+	auto createdObject = createNewObject(visitablePosition, Obj::HOLE, 0);
+	newObject(createdObject, initiator);
+}
+
+void CGameHandler::newObject(CGObjectInstance * object, PlayerColor initiator)
+{
+	object->initObj(gs->getRandomGenerator());
+
 	NewObject no;
-	no.ID = type;
+	no.newObject = object;
-	no.subID = subtype;
 	no.initiator = initiator;
-	no.targetPos = visitablePosition;
 	sendAndApply(&no);
 }
 

server/CGameHandler.h

@@ -92,6 +92,14 @@ public:
 	bool isAllowedExchange(ObjectInstanceID id1, ObjectInstanceID id2);
 	void giveSpells(const CGTownInstance *t, const CGHeroInstance *h);
 
+	// Helpers to create new object of specified type
+
+	CGObjectInstance * createNewObject(const int3 & visitablePosition, MapObjectID objectID, MapObjectSubID subID);
+	void createWanderingMonster(const int3 & visitablePosition, CreatureID creature);
+	void createBoat(const int3 & visitablePosition, BoatId type, PlayerColor initiator) override;
+	void createHole(const int3 & visitablePosition, PlayerColor initiator);
+	void newObject(CGObjectInstance * object, PlayerColor initiator);
+
 	explicit CGameHandler(CVCMIServer * lobby);
 	~CGameHandler();
 
@@ -100,7 +108,6 @@ public:
 	//do sth
 	void changeSpells(const CGHeroInstance * hero, bool give, const std::set<SpellID> &spells) override;
 	bool removeObject(const CGObjectInstance * obj, const PlayerColor & initiator) override;
-	void createObject(const int3 & visitablePosition, const PlayerColor & initiator, MapObjectID type, MapObjectSubID subtype) override;
 	void setOwner(const CGObjectInstance * obj, PlayerColor owner) override;
 	void giveExperience(const CGHeroInstance * hero, TExpType val) override;
 	void changePrimSkill(const CGHeroInstance * hero, PrimarySkill which, si64 val, bool abs=false) override;

server/ServerSpellCastEnvironment.cpp

@@ -94,6 +94,11 @@ bool ServerSpellCastEnvironment::moveHero(ObjectInstanceID hid, int3 dst, EMovem
 	return gh->moveHero(hid, dst, mode, false);
 }
 
+void ServerSpellCastEnvironment::createBoat(const int3 & visitablePosition, BoatId type, PlayerColor initiator)
+{
+	return gh->createBoat(visitablePosition, type, initiator);
+}
+
 void ServerSpellCastEnvironment::genericQuery(Query * request, PlayerColor color, std::function<void(std::optional<int32_t>)> callback)
 {
 	auto query = std::make_shared<CGenericQuery>(gh, color, callback);

server/ServerSpellCastEnvironment.h

@@ -37,6 +37,7 @@ public:
 	const CMap * getMap() const override;
 	const CGameInfoCallback * getCb() const override;
 	bool moveHero(ObjectInstanceID hid, int3 dst, EMovementMode mode) override;
+	void createBoat(const int3 & visitablePosition, BoatId type, PlayerColor initiator) override;
 	void genericQuery(Query * request, PlayerColor color, std::function<void(std::optional<int32_t>)> callback) override;
 private:
 	CGameHandler * gh;

server/processors/HeroPoolProcessor.cpp

@@ -227,8 +227,7 @@ bool HeroPoolProcessor::hireHero(const ObjectInstanceID & objectID, const HeroTy
 	if(gameHandler->getTile(targetPos)->isWater() && !recruitedHero->boat)
 	{
 		//Create a new boat for hero
-		gameHandler->createObject(targetPos, player, Obj::BOAT, recruitedHero->getBoatType().getNum());
+		gameHandler->createBoat(targetPos, recruitedHero->getBoatType(), player);
-
 		hr.boatId = gameHandler->getTopObj(targetPos)->id;
 	}