Cleaned up IShipyard interface

AI/Nullkiller/Goals/BuildBoat.cpp

@@ -23,7 +23,7 @@ using namespace Goals;
 
 bool BuildBoat::operator==(const BuildBoat & other) const
 {
-	return shipyard->o->id == other.shipyard->o->id;
+	return shipyard == other.shipyard;
 }
 //
 //TSubgoal BuildBoat::decomposeSingle() const
@@ -54,7 +54,7 @@ void BuildBoat::accept(AIGateway * ai)
 		throw cannotFulfillGoalException("Can not afford boat");
 	}
 
-	if(cb->getPlayerRelations(ai->playerID, shipyard->o->tempOwner) == PlayerRelations::ENEMIES)
+	if(cb->getPlayerRelations(ai->playerID, shipyard->getObject()->getOwner()) == PlayerRelations::ENEMIES)
 	{
 		throw cannotFulfillGoalException("Can not build boat in enemy shipyard");
 	}
@@ -65,9 +65,8 @@ void BuildBoat::accept(AIGateway * ai)
 	}
 
 	logAi->trace(
-		"Building boat at shipyard %s located at %s, estimated boat position %s", 
+		"Building boat at shipyard located at %s, estimated boat position %s",
-		shipyard->o->getObjectName(),
+		shipyard->getObject()->visitablePos().toString(),
-		shipyard->o->visitablePos().toString(),
 		shipyard->bestLocation().toString());
 
 	cb->buildBoat(shipyard);

AI/Nullkiller/Pathfinding/Actions/BoatActions.cpp

@@ -32,9 +32,9 @@ namespace AIPathfinding
 
 	Goals::TSubgoal BuildBoatAction::decompose(const CGHeroInstance * hero) const
 	{
-		if(cb->getPlayerRelations(ai->playerID, shipyard->o->tempOwner) == PlayerRelations::ENEMIES)
+		if(cb->getPlayerRelations(ai->playerID, shipyard->getObject()->getOwner()) == PlayerRelations::ENEMIES)
 		{
-			return Goals::sptr(Goals::CaptureObject(shipyard->o));
+			return Goals::sptr(Goals::CaptureObject(targetObject()));
 		}
 		
 		return Goals::sptr(Goals::Invalid());
@@ -44,7 +44,7 @@ namespace AIPathfinding
 	{
 		auto hero = source->actor->hero;
 
-		if(cb->getPlayerRelations(hero->tempOwner, shipyard->o->tempOwner) == PlayerRelations::ENEMIES)
+		if(cb->getPlayerRelations(hero->tempOwner, shipyard->getObject()->getOwner()) == PlayerRelations::ENEMIES)
 		{
 #if NKAI_TRACE_LEVEL > 1
 			logAi->trace("Can not build a boat. Shipyard is enemy.");
@@ -70,7 +70,7 @@ namespace AIPathfinding
 
 	const CGObjectInstance * BuildBoatAction::targetObject() const
 	{
-		return shipyard->o;
+		return dynamic_cast<const CGObjectInstance*>(shipyard);
 	}
 
 	const ChainActor * BuildBoatAction::getActor(const ChainActor * sourceActor) const
@@ -101,7 +101,7 @@ namespace AIPathfinding
 
 	std::string BuildBoatAction::toString() const
 	{
-		return "Build Boat at " + shipyard->o->getObjectName();
+		return "Build Boat at " + shipyard->getObject()->visitablePos().toString();
 	}
 
 	bool SummonBoatAction::canAct(const AIPathNode * source) const

AI/VCAI/Goals/BuildBoat.cpp

@@ -22,14 +22,14 @@ using namespace Goals;
 
 bool BuildBoat::operator==(const BuildBoat & other) const
 {
-	return shipyard->o->id == other.shipyard->o->id;
+	return shipyard == other.shipyard;
 }
 
 TSubgoal BuildBoat::whatToDoToAchieve()
 {
-	if(cb->getPlayerRelations(ai->playerID, shipyard->o->tempOwner) == PlayerRelations::ENEMIES)
+	if(cb->getPlayerRelations(ai->playerID, shipyard->getObject()->getOwner()) == PlayerRelations::ENEMIES)
 	{
-		return fh->chooseSolution(ai->ah->howToVisitObj(shipyard->o));
+		return fh->chooseSolution(ai->ah->howToVisitObj(dynamic_cast<const CGObjectInstance*>(shipyard)));
 	}
 
 	if(shipyard->shipyardStatus() != IShipyard::GOOD)
@@ -53,7 +53,7 @@ void BuildBoat::accept(VCAI * ai)
 		throw cannotFulfillGoalException("Can not afford boat");
 	}
 
-	if(cb->getPlayerRelations(ai->playerID, shipyard->o->tempOwner) == PlayerRelations::ENEMIES)
+	if(cb->getPlayerRelations(ai->playerID, shipyard->getObject()->getOwner()) == PlayerRelations::ENEMIES)
 	{
 		throw cannotFulfillGoalException("Can not build boat in enemy shipyard");
 	}
@@ -64,9 +64,8 @@ void BuildBoat::accept(VCAI * ai)
 	}
 
 	logAi->trace(
-		"Building boat at shipyard %s located at %s, estimated boat position %s", 
+		"Building boat at shipyard located at %s, estimated boat position %s",
-		shipyard->o->getObjectName(),
+		shipyard->getObject()->visitablePos().toString(),
-		shipyard->o->visitablePos().toString(),
 		shipyard->bestLocation().toString());
 
 	cb->buildBoat(shipyard);
@@ -81,5 +80,5 @@ std::string BuildBoat::name() const
 
 std::string BuildBoat::completeMessage() const
 {
-	return "Boat have been built at " + shipyard->o->visitablePos().toString();
+	return "Boat have been built at " + shipyard->getObject()->visitablePos().toString();
 }

CCallback.cpp

@@ -301,7 +301,7 @@ void CCallback::sendMessage(const std::string &mess, const CGObjectInstance * cu
 void CCallback::buildBoat( const IShipyard *obj )
 {
 	BuildBoat bb;
-	bb.objid = obj->o->id;
+	bb.objid = dynamic_cast<const CGObjectInstance*>(obj)->id;
 	sendRequest(&bb);
 }
 

client/NetPacksClient.cpp

@@ -906,7 +906,7 @@ void ApplyClientNetPackVisitor::visitOpenWindow(OpenWindow & pack)
 	case EOpenWindowMode::SHIPYARD_WINDOW:
 		{
 			const IShipyard *sy = IShipyard::castFrom(cl.getObj(ObjectInstanceID(pack.id1)));
-			callInterfaceIfPresent(cl, sy->o->tempOwner, &IGameEventsReceiver::showShipyardDialog, sy);
+			callInterfaceIfPresent(cl, sy->getObject()->getOwner(), &IGameEventsReceiver::showShipyardDialog, sy);
 		}
 		break;
 	case EOpenWindowMode::THIEVES_GUILD:

lib/mapObjects/CGHeroInstance.cpp

@@ -225,7 +225,6 @@ int CGHeroInstance::maxMovePointsCached(bool onLand, const TurnInfo * ti) const
 }
 
 CGHeroInstance::CGHeroInstance():
-	IBoatGenerator(this),
 	tacticFormationEnabled(false),
 	inTownGarrison(false),
 	moveDir(4),
@@ -973,6 +972,11 @@ void CGHeroInstance::getOutOffsets(std::vector<int3> &offsets) const
 	};
 }
 
+const IObjectInterface * CGHeroInstance::getObject() const
+{
+	return this;
+}
+
 int32_t CGHeroInstance::getSpellCost(const spells::Spell * sp) const
 {
 	return sp->getCost(getSpellSchoolLevel(sp));

lib/mapObjects/CGHeroInstance.h

@@ -133,6 +133,7 @@ public:
 
 	BoatId getBoatType() const override; //0 - evil (if a ship can be evil...?), 1 - good, 2 - neutral
 	void getOutOffsets(std::vector<int3> &offsets) const override; //offsets to obj pos when we boat can be placed
+	const IObjectInterface * getObject() const override;
 
 	//////////////////////////////////////////////////////////////////////////
 

lib/mapObjects/CGTownInstance.cpp

@@ -224,8 +224,6 @@ bool CGTownInstance::hasCapitol() const
 }
 
 CGTownInstance::CGTownInstance():
-	IShipyard(this),
-	IMarket(),
 	town(nullptr),
 	builded(0),
 	destroyed(0),
@@ -590,6 +588,19 @@ void CGTownInstance::getOutOffsets( std::vector<int3> &offsets ) const
 	offsets = {int3(-1,2,0), int3(-3,2,0)};
 }
 
+CGTownInstance::EGeneratorState CGTownInstance::shipyardStatus() const
+{
+	if (!hasBuilt(BuildingID::SHIPYARD))
+		return EGeneratorState::UNKNOWN;
+
+	return IShipyard::shipyardStatus();
+}
+
+const IObjectInterface * CGTownInstance::getObject() const
+{
+	return this;
+}
+
 void CGTownInstance::mergeGarrisonOnSiege() const
 {
 	auto getWeakestStackSlot = [&](ui64 powerLimit)

lib/mapObjects/CGTownInstance.h

@@ -46,6 +46,8 @@ class DLL_LINKAGE CGTownInstance : public CGDwelling, public IShipyard, public I
 {
 	std::string name; // name of town
 public:
+	using CGDwelling::getPosition;
+
 	enum EFortLevel {NONE = 0, FORT = 1, CITADEL = 2, CASTLE = 3};
 
 	CTownAndVisitingHero townAndVis;
@@ -71,7 +73,6 @@ public:
 	template <typename Handler> void serialize(Handler &h, const int version)
 	{
 		h & static_cast<CGDwelling&>(*this);
-		h & static_cast<IShipyard&>(*this);
 		h & name;
 		h & builded;
 		h & destroyed;
@@ -134,6 +135,8 @@ public:
 	int getSightRadius() const override; //returns sight distance
 	BoatId getBoatType() const override; //0 - evil (if a ship can be evil...?), 1 - good, 2 - neutral
 	void getOutOffsets(std::vector<int3> &offsets) const override; //offsets to obj pos when we boat can be placed. Parameter will be cleared
+	EGeneratorState shipyardStatus() const override;
+	const IObjectInterface * getObject() const override;
 	int getMarketEfficiency() const override; //=market count
 	bool allowsTrade(EMarketMode::EMarketMode mode) const override;
 	std::vector<int> availableItemsIds(EMarketMode::EMarketMode mode) const override;

lib/mapObjects/IObjectInterface.cpp

@@ -92,10 +92,10 @@ int3 IBoatGenerator::bestLocation() const
 
 	for (auto & offset : offsets)
 	{
-		if(const TerrainTile *tile = IObjectInterface::cb->getTile(o->pos + offset, false)) //tile is in the map
+		if(const TerrainTile *tile = getObject()->cb->getTile(getObject()->getPosition() + offset, false)) //tile is in the map
 		{
 			if(tile->terType->isWater()  &&  (!tile->blocked || tile->blockingObjects.front()->ID == Obj::BOAT)) //and is water and is not blocked or is blocked by boat
-				return o->pos + offset;
+				return getObject()->getPosition() + offset;
 		}
 	}
 	return int3 (-1,-1,-1);
@@ -107,24 +107,14 @@ IBoatGenerator::EGeneratorState IBoatGenerator::shipyardStatus() const
 	const TerrainTile *t = IObjectInterface::cb->getTile(tile);
 	if(!t)
 		return TILE_BLOCKED; //no available water
-	else if(t->blockingObjects.empty())
+
+	if(t->blockingObjects.empty())
 		return GOOD; //OK
-	else if(t->blockingObjects.front()->ID == Obj::BOAT)
+
+	if(t->blockingObjects.front()->ID == Obj::BOAT)
 		return BOAT_ALREADY_BUILT; //blocked with boat
-	else
-		return TILE_BLOCKED; //blocked
-}
 
-BoatId IBoatGenerator::getBoatType() const
+	return TILE_BLOCKED; //blocked
-{
-	//We make good ships by default
-	return EBoatId::BOAT_GOOD;
-}
-
-
-IBoatGenerator::IBoatGenerator(const CGObjectInstance *O)
-: o(O)
-{
 }
 
 void IBoatGenerator::getProblemText(MetaString &out, const CGHeroInstance *visitor) const
@@ -144,7 +134,7 @@ void IBoatGenerator::getProblemText(MetaString &out, const CGHeroInstance *visit
 			out.addTxt(MetaString::ADVOB_TXT, 189);
 		break;
 	case NO_WATER:
-		logGlobal->error("Shipyard without water! %s \t %d", o->pos.toString(), o->id.getNum());
+		logGlobal->error("Shipyard without water at tile %s! ", getObject()->getPosition().toString());
 		return;
 	}
 }
@@ -155,34 +145,9 @@ void IShipyard::getBoatCost(TResources & cost) const
 	cost[EGameResID::GOLD] = 1000;
 }
 
-IShipyard::IShipyard(const CGObjectInstance *O)
-	: IBoatGenerator(O)
-{
-}
-
-IShipyard * IShipyard::castFrom( CGObjectInstance *obj )
-{
-	if(!obj)
-		return nullptr;
-
-	if(obj->ID == Obj::TOWN)
-	{
-		return dynamic_cast<CGTownInstance *>(obj);
-	}
-	else if(obj->ID == Obj::SHIPYARD)
-	{
-		return dynamic_cast<CGShipyard *>(obj);
-	}
-	else
-	{
-		return nullptr;
-	}
-}
-
 const IShipyard * IShipyard::castFrom( const CGObjectInstance *obj )
 {
-	return castFrom(const_cast<CGObjectInstance*>(obj));
+	return dynamic_cast<const IShipyard *>(obj);
 }
 
-
 VCMI_LIB_NAMESPACE_END

lib/mapObjects/IObjectInterface.h

@@ -78,39 +78,25 @@ public:
 class DLL_LINKAGE IBoatGenerator
 {
 public:
-	const CGObjectInstance *o;
-
-	IBoatGenerator(const CGObjectInstance *O);
 	virtual ~IBoatGenerator() = default;
 
-	virtual BoatId getBoatType() const; //0 - evil (if a ship can be evil...?), 1 - good, 2 - neutral
+	virtual const IObjectInterface * getObject() const = 0;
-	virtual void getOutOffsets(std::vector<int3> &offsets) const =0; //offsets to obj pos when we boat can be placed
+
+	virtual BoatId getBoatType() const = 0; //0 - evil (if a ship can be evil...?), 1 - good, 2 - neutral
+	virtual void getOutOffsets(std::vector<int3> & offsets) const = 0; //offsets to obj pos when we boat can be placed
 	int3 bestLocation() const; //returns location when the boat should be placed
 
-	enum EGeneratorState {GOOD, BOAT_ALREADY_BUILT, TILE_BLOCKED, NO_WATER};
+	enum EGeneratorState {GOOD, BOAT_ALREADY_BUILT, TILE_BLOCKED, NO_WATER, UNKNOWN};
-	EGeneratorState shipyardStatus() const; //0 - can buid, 1 - there is already a boat at dest tile, 2 - dest tile is blocked, 3 - no water
+	virtual EGeneratorState shipyardStatus() const;
 	void getProblemText(MetaString &out, const CGHeroInstance *visitor = nullptr) const;
-
-	template <typename Handler> void serialize(Handler &h, const int version)
-	{
-		h & o;
-	}
 };
 
 class DLL_LINKAGE IShipyard : public IBoatGenerator
 {
 public:
-	IShipyard(const CGObjectInstance *O);
-
 	virtual void getBoatCost(ResourceSet & cost) const;
 
 	static const IShipyard *castFrom(const CGObjectInstance *obj);
-	static IShipyard *castFrom(CGObjectInstance *obj);
-
-	template <typename Handler> void serialize(Handler &h, const int version)
-	{
-		h & static_cast<IBoatGenerator&>(*this);
-	}
 };
 
 VCMI_LIB_NAMESPACE_END

lib/mapObjects/MiscObjects.cpp

@@ -1915,12 +1915,6 @@ void CGSirens::onHeroVisit( const CGHeroInstance * h ) const
 		}
 	}
 	cb->showInfoDialog(&iw);
-
-}
-
-CGShipyard::CGShipyard()
-	:IShipyard(this)
-{
 }
 
 void CGShipyard::getOutOffsets( std::vector<int3> &offsets ) const
@@ -1935,6 +1929,11 @@ void CGShipyard::getOutOffsets( std::vector<int3> &offsets ) const
 	};
 }
 
+const IObjectInterface * CGShipyard::getObject() const
+{
+	return this;
+}
+
 void CGShipyard::onHeroVisit( const CGHeroInstance * h ) const
 {
 	if(!cb->gameState()->getPlayerRelations(tempOwner, h->tempOwner))
@@ -1960,6 +1959,11 @@ void CGShipyard::serializeJsonOptions(JsonSerializeFormat& handler)
 	serializeJsonOwner(handler);
 }
 
+BoatId CGShipyard::getBoatType() const
+{
+	return EBoatId::BOAT_GOOD;
+}
+
 void CCartographer::onHeroVisit( const CGHeroInstance * h ) const
 {
 	//if player has not bought map of this subtype yet and underground exist for stalagmite cartographer

lib/mapObjects/MiscObjects.h

@@ -461,14 +461,11 @@ class DLL_LINKAGE CGShipyard : public CGObjectInstance, public IShipyard
 {
 public:
 	void getOutOffsets(std::vector<int3> &offsets) const override; //offsets to obj pos when we boat can be placed
-	CGShipyard();
 	void onHeroVisit(const CGHeroInstance * h) const override;
+	const IObjectInterface * getObject() const override;
+	BoatId getBoatType() const override;
+
 
-	template <typename Handler> void serialize(Handler &h, const int version)
-	{
-		h & static_cast<CGObjectInstance&>(*this);
-		h & static_cast<IShipyard&>(*this);
-	}
 protected:
 	void serializeJsonOptions(JsonSerializeFormat & handler) override;
 };

lib/registerTypes/RegisterTypes.h

@@ -62,7 +62,7 @@ void registerTypesMapObjects1(Serializer &s)
 	s.template registerType<CGObjectInstance, CGBoat>();
 	s.template registerType<CGObjectInstance, CGMagi>();
 	s.template registerType<CGObjectInstance, CGSirens>();
-	s.template registerType<CGObjectInstance, CGShipyard>(); s.template registerType<IShipyard, CGShipyard>();
+	s.template registerType<CGObjectInstance, CGShipyard>();
 	s.template registerType<CGObjectInstance, CGDenOfthieves>();
 	s.template registerType<CGObjectInstance, CGLighthouse>();
 	s.template registerType<CGObjectInstance, CGTerrainPatch>();
@@ -75,9 +75,9 @@ void registerTypesMapObjects1(Serializer &s)
 	s.template registerType<CGObjectInstance, CArmedInstance>(); s.template registerType<CBonusSystemNode, CArmedInstance>(); s.template registerType<CCreatureSet, CArmedInstance>();
 
 	// Armed objects
-	s.template registerType<CArmedInstance, CGHeroInstance>(); s.template registerType<IBoatGenerator, CGHeroInstance>(); s.template registerType<CArtifactSet, CGHeroInstance>();
+	s.template registerType<CArmedInstance, CGHeroInstance>(); s.template registerType<CArtifactSet, CGHeroInstance>();
 	s.template registerType<CArmedInstance, CGDwelling>();
-		s.template registerType<CGDwelling, CGTownInstance>(); s.template registerType<IShipyard, CGTownInstance>();
+		s.template registerType<CGDwelling, CGTownInstance>();
 	s.template registerType<CArmedInstance, CGPandoraBox>();
 		s.template registerType<CGPandoraBox, CGEvent>();
 	s.template registerType<CArmedInstance, CGCreature>();

server/CGameHandler.cpp

@@ -5627,12 +5627,6 @@ bool CGameHandler::buildBoat(ObjectInstanceID objid, PlayerColor playerID)
 		complain("Cannot build boat in this shipyard!");
 		return false;
 	}
-	else if (obj->o->ID == Obj::TOWN
-	        && !static_cast<const CGTownInstance*>(obj)->hasBuilt(BuildingID::SHIPYARD))
-	{
-		complain("Cannot build boat in the town - no shipyard!");
-		return false;
-	}
 
 	TResources boatCost;
 	obj->getBoatCost(boatCost);