diff --git a/client/Client.h b/client/Client.h index 1f731b7bc..d2295f11b 100644 --- a/client/Client.h +++ b/client/Client.h @@ -187,6 +187,7 @@ public: void changeSpells(const CGHeroInstance * hero, bool give, const std::set & spells) override {}; bool removeObject(const CGObjectInstance * obj) override {return false;}; + void createObject(const int3 & visitablePosition, Obj type, int32_t subtype ) override {}; void setOwner(const CGObjectInstance * obj, PlayerColor owner) override {}; void changePrimSkill(const CGHeroInstance * hero, PrimarySkill::PrimarySkill which, si64 val, bool abs = false) override {}; void changeSecSkill(const CGHeroInstance * hero, SecondarySkill which, int val, bool abs = false) override {}; diff --git a/client/NetPacksClient.cpp b/client/NetPacksClient.cpp index bbed731e2..be792582b 100644 --- a/client/NetPacksClient.cpp +++ b/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); diff --git a/lib/IGameCallback.h b/lib/IGameCallback.h index 3eb4c6c9a..747d71d72 100644 --- a/lib/IGameCallback.h +++ b/lib/IGameCallback.h @@ -90,6 +90,7 @@ public: virtual void changeSpells(const CGHeroInstance * hero, bool give, const std::set &spells)=0; virtual bool removeObject(const CGObjectInstance * obj)=0; + virtual void createObject(const int3 & visitablePosition, Obj type, int32_t subtype = 0) = 0; virtual void setOwner(const CGObjectInstance * objid, PlayerColor owner)=0; virtual void changePrimSkill(const CGHeroInstance * hero, PrimarySkill::PrimarySkill which, si64 val, bool abs=false)=0; virtual void changeSecSkill(const CGHeroInstance * hero, SecondarySkill which, int val, bool abs=false)=0; diff --git a/lib/NetPacks.h b/lib/NetPacks.h index 38e71e425..831db49dc 100644 --- a/lib/NetPacks.h +++ b/lib/NetPacks.h @@ -384,7 +384,9 @@ struct DLL_LINKAGE ChangeObjPos : public CPackForClient { void applyGs(CGameState * gs); + /// Object to move ObjectInstanceID objid; + /// New position of visitable tile of an object int3 nPos; virtual void visitTyped(ICPackVisitor & visitor) override; @@ -745,11 +747,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 +762,7 @@ struct DLL_LINKAGE NewObject : public CPackForClient { h & ID; h & subID; - h & pos; + h & targetPos; } }; diff --git a/lib/NetPacksLib.cpp b/lib/NetPacksLib.cpp index 5be4c529d..019beb076 100644 --- a/lib/NetPacksLib.cpp +++ b/lib/NetPacksLib.cpp @@ -1019,7 +1019,7 @@ void ChangeObjPos::applyGs(CGameState *gs) return; } gs->map->removeBlockVisTiles(obj); - obj->pos = nPos; + obj->pos = nPos + obj->getVisitableOffset(); gs->map->addBlockVisTiles(obj); } @@ -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(); + logGlobal->error("Attempt to create object outside map at %s!", targetPos.toString()); + return; + } - [[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(); - } + 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(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(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(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(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) diff --git a/lib/mapObjectConstructors/CommonConstructors.cpp b/lib/mapObjectConstructors/CommonConstructors.cpp index cef36448e..4a21d8825 100644 --- a/lib/mapObjectConstructors/CommonConstructors.cpp +++ b/lib/mapObjectConstructors/CommonConstructors.cpp @@ -165,15 +165,6 @@ std::string BoatInstanceConstructor::getBoatAnimationName() const return actualAnimation; } -void BoatInstanceConstructor::afterLoadFinalization() -{ - if (layer == EPathfindingLayer::SAIL) - { - if (getTemplates(TerrainId(ETerrainId::WATER)).empty()) - logMod->warn("Boat of type %s has no templates suitable for water!", getJsonKey()); - } -} - void MarketInstanceConstructor::initTypeData(const JsonNode & input) { for(auto & element : input["modes"].Vector()) diff --git a/lib/mapObjectConstructors/CommonConstructors.h b/lib/mapObjectConstructors/CommonConstructors.h index 7144596e6..4a58d699e 100644 --- a/lib/mapObjectConstructors/CommonConstructors.h +++ b/lib/mapObjectConstructors/CommonConstructors.h @@ -97,7 +97,6 @@ protected: public: void initializeObject(CGBoat * object) const override; - void afterLoadFinalization() override; /// Returns boat preview animation, for use in Shipyards std::string getBoatAnimationName() const; diff --git a/lib/mapObjects/CGHeroInstance.cpp b/lib/mapObjects/CGHeroInstance.cpp index 849501222..4ab847bf5 100644 --- a/lib/mapObjects/CGHeroInstance.cpp +++ b/lib/mapObjects/CGHeroInstance.cpp @@ -451,12 +451,7 @@ void CGHeroInstance::onHeroVisit(const CGHeroInstance * h) const { smp.val = maxMovePoints(false); //Create a new boat for hero - NewObject no; - no.ID = Obj::BOAT; - no.subID = getBoatType().getNum(); - - cb->sendAndApply(&no); - + cb->createObject(boatPos, Obj::BOAT, getBoatType().getNum()); boatId = cb->getTopObj(boatPos)->id; } else @@ -958,11 +953,15 @@ BoatId CGHeroInstance::getBoatType() const void CGHeroInstance::getOutOffsets(std::vector &offsets) const { - // FIXME: Offsets need to be fixed once we get rid of convertPosition - // Check issue 515 for details - offsets = - { - int3(-1,1,0), int3(-1,-1,0), int3(-2,0,0), int3(0,0,0), int3(0,1,0), int3(-2,1,0), int3(0,-1,0), int3(-2,-1,0) + offsets = { + {0, -1, 0}, + {+1, -1, 0}, + {+1, 0, 0}, + {+1, +1, 0}, + {0, +1, 0}, + {-1, +1, 0}, + {-1, 0, 0}, + {-1, -1, 0}, }; } diff --git a/lib/mapObjects/CGTownInstance.cpp b/lib/mapObjects/CGTownInstance.cpp index bab2be44e..53e15193d 100644 --- a/lib/mapObjects/CGTownInstance.cpp +++ b/lib/mapObjects/CGTownInstance.cpp @@ -585,7 +585,7 @@ bool CGTownInstance::passableFor(PlayerColor color) const void CGTownInstance::getOutOffsets( std::vector &offsets ) const { - offsets = {int3(-1,2,0), int3(-3,2,0)}; + offsets = {int3(-1,2,0), int3(+1,2,0)}; } CGTownInstance::EGeneratorState CGTownInstance::shipyardStatus() const diff --git a/lib/mapObjects/IObjectInterface.cpp b/lib/mapObjects/IObjectInterface.cpp index c8292ebc3..967470804 100644 --- a/lib/mapObjects/IObjectInterface.cpp +++ b/lib/mapObjects/IObjectInterface.cpp @@ -92,10 +92,13 @@ int3 IBoatGenerator::bestLocation() const for (auto & offset : offsets) { - if(const TerrainTile *tile = getObject()->cb->getTile(getObject()->getPosition() + offset, false)) //tile is in the map + int3 targetTile = getObject()->visitablePos() + offset; + const TerrainTile *tile = getObject()->cb->getTile(targetTile, false); + + if(tile) //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 getObject()->getPosition() + offset; + return targetTile; } } return int3 (-1,-1,-1); diff --git a/lib/mapObjects/MiscObjects.cpp b/lib/mapObjects/MiscObjects.cpp index 403c29158..0ea519a87 100644 --- a/lib/mapObjects/MiscObjects.cpp +++ b/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; @@ -1371,9 +1355,18 @@ void CGShipyard::getOutOffsets( std::vector &offsets ) const // A x S x B // C E G F D offsets = { - int3(-3,0,0), int3(1,0,0), //AB - int3(-3,1,0), int3(1,1,0), int3(-2,1,0), int3(0,1,0), int3(-1,1,0), //CDEFG - int3(-3,-1,0), int3(1,-1,0), int3(-2,-1,0), int3(0,-1,0), int3(-1,-1,0) //HIJKL + {-2, 0, 0}, // A + {+2, 0, 0}, // B + {-2, 1, 0}, // C + {+2, 1, 0}, // D + {-1, 1, 0}, // E + {+1, 1, 0}, // F + {0, 1, 0}, // G + {-2, -1, 0}, // H + {+2, -1, 0}, // I + {-1, -1, 0}, // G + {+1, -1, 0}, // K + {0, -1, 0}, // L }; } @@ -1384,11 +1377,10 @@ const IObjectInterface * CGShipyard::getObject() const void CGShipyard::onHeroVisit( const CGHeroInstance * h ) const { - if(!cb->gameState()->getPlayerRelations(tempOwner, h->tempOwner)) + if(cb->gameState()->getPlayerRelations(tempOwner, h->tempOwner) == PlayerRelations::ENEMIES) cb->setOwner(this, h->tempOwner); - auto s = shipyardStatus(); - if(s != IBoatGenerator::GOOD) + if(shipyardStatus() != IBoatGenerator::GOOD) { InfoWindow iw; iw.type = EInfoWindowMode::AUTO; @@ -1409,8 +1401,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 diff --git a/lib/mapObjects/MiscObjects.h b/lib/mapObjects/MiscObjects.h index 972f52c1f..4dbf8df09 100644 --- a/lib/mapObjects/MiscObjects.h +++ b/lib/mapObjects/MiscObjects.h @@ -359,7 +359,6 @@ public: std::array flagAnimations; CGBoat(); - static int3 translatePos(const int3 &pos, bool reverse = false); template void serialize(Handler &h, const int version) { diff --git a/lib/spells/AdventureSpellMechanics.cpp b/lib/spells/AdventureSpellMechanics.cpp index 5c11f6126..ccc69c79e 100644 --- a/lib/spells/AdventureSpellMechanics.cpp +++ b/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; diff --git a/server/CGameHandler.cpp b/server/CGameHandler.cpp index 1e358da1d..f8a5a4af5 100644 --- a/server/CGameHandler.cpp +++ b/server/CGameHandler.cpp @@ -4429,11 +4429,7 @@ bool CGameHandler::hireHero(const CGObjectInstance *obj, ui8 hid, PlayerColor pl if (getTile(hr.tile)->isWater()) { //Create a new boat for hero - NewObject no; - no.ID = Obj::BOAT; - no.subID = nh->getBoatType().getNum(); - no.pos = hr.tile + int3(1,0,0); - sendAndApply(&no); + createObject(obj->visitablePos(), Obj::BOAT, nh->getBoatType().getNum()); hr.boatId = getTopObj(hr.tile)->id; } @@ -5653,16 +5649,8 @@ bool CGameHandler::buildBoat(ObjectInstanceID objid, PlayerColor playerID) return false; } - //take boat cost giveResources(playerID, -boatCost); - - //create boat - NewObject no; - no.ID = Obj::BOAT; - no.subID = obj->getBoatType().getNum(); - no.pos = tile + int3(1,0,0); - sendAndApply(&no); - + createObject(tile, Obj::BOAT, obj->getBoatType().getNum()); return true; } @@ -5803,12 +5791,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)!", h->diggingStatus()); - //create a hole - NewObject no; - no.ID = Obj::HOLE; - no.pos = h->visitablePos(); - no.subID = 0; - sendAndApply(&no); + createObject(h->visitablePos(), Obj::HOLE, 0 ); //take MPs SetMovePoints smp; @@ -6875,7 +6858,9 @@ void CGameHandler::spawnWanderingMonsters(CreatureID creatureID) { auto count = cre->getRandomAmount(std::rand); - auto monsterId = putNewObject(Obj::MONSTER, creatureID, *tile); + createObject(*tile, Obj::MONSTER, creatureID); + auto monsterId = getTopObj(*tile)->id; + setObjProperty(monsterId, ObjProperty::MONSTER_COUNT, count); setObjProperty(monsterId, ObjProperty::MONSTER_POWER, (si64)1000*count); } @@ -7404,12 +7389,11 @@ scripting::Pool * CGameHandler::getContextPool() const } #endif -const ObjectInstanceID CGameHandler::putNewObject(Obj ID, int subID, int3 pos) +void CGameHandler::createObject(const int3 & visitablePosition, Obj type, int32_t subtype) { NewObject no; - no.ID = ID; //creature - no.subID= subID; - no.pos = pos; + no.ID = type; + no.subID= subtype; + no.targetPos = visitablePosition; sendAndApply(&no); - return no.id; //id field will be filled during applying on gs } diff --git a/server/CGameHandler.h b/server/CGameHandler.h index 14d4c0298..24e97067d 100644 --- a/server/CGameHandler.h +++ b/server/CGameHandler.h @@ -153,6 +153,7 @@ public: //do sth void changeSpells(const CGHeroInstance * hero, bool give, const std::set &spells) override; bool removeObject(const CGObjectInstance * obj) override; + void createObject(const int3 & visitablePosition, Obj type, int32_t subtype ) override; void setOwner(const CGObjectInstance * obj, PlayerColor owner) override; void changePrimSkill(const CGHeroInstance * hero, PrimarySkill::PrimarySkill which, si64 val, bool abs=false) override; void changeSecSkill(const CGHeroInstance * hero, SecondarySkill which, int val, bool abs=false) override; @@ -276,7 +277,6 @@ public: void engageIntoBattle( PlayerColor player ); bool dig(const CGHeroInstance *h); void moveArmy(const CArmedInstance *src, const CArmedInstance *dst, bool allowMerging); - const ObjectInstanceID putNewObject(Obj ID, int subID, int3 pos); template void serialize(Handler &h, const int version) { diff --git a/test/mock/mock_IGameCallback.h b/test/mock/mock_IGameCallback.h index b56a6aa23..f6ed9e856 100644 --- a/test/mock/mock_IGameCallback.h +++ b/test/mock/mock_IGameCallback.h @@ -41,6 +41,7 @@ public: void changeSpells(const CGHeroInstance * hero, bool give, const std::set &spells) override {} bool removeObject(const CGObjectInstance * obj) override {return false;} + void createObject(const int3 & visitablePosition, Obj type, int32_t subtype = 0) override {}; void setOwner(const CGObjectInstance * objid, PlayerColor owner) override {} void changePrimSkill(const CGHeroInstance * hero, PrimarySkill::PrimarySkill which, si64 val, bool abs=false) override {} void changeSecSkill(const CGHeroInstance * hero, SecondarySkill which, int val, bool abs=false) override {}