Skip to content

Commit

Permalink
SpawnGroup/Formation: Rework FormationEntry use to copy it to each ne…
Browse files Browse the repository at this point in the history
…w formation

Formation entry was reused among same spawns leading to cross map usage in different instances of same map.
  • Loading branch information
killerwife committed Dec 4, 2024
1 parent f1e009d commit cf9593d
Show file tree
Hide file tree
Showing 7 changed files with 72 additions and 77 deletions.
4 changes: 2 additions & 2 deletions src/game/Chat/Level2.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3375,8 +3375,8 @@ bool ChatHandler::HandleWpShowCommand(char* args)
if (mgenType == WAYPOINT_MOTION_TYPE || mgenType == LINEAR_WP_MOTION_TYPE || mgenType == PATH_MOTION_TYPE)
{
uint32 pathEntry = wpOwner->GetEntry();
if (targetCreature->GetCreatureGroup() && targetCreature->GetCreatureGroup()->GetFormationEntry())
pathEntry = targetCreature->GetCreatureGroup()->GetFormationEntry()->MovementIdOrWander;
if (targetCreature->GetCreatureGroup() && targetCreature->GetCreatureGroup()->GetFormationData())
pathEntry = targetCreature->GetCreatureGroup()->GetFormationData()->GetFormationEntry().MovementIdOrWander;
if (WaypointMovementGenerator<Creature> const* wpMMGen = dynamic_cast<WaypointMovementGenerator<Creature> const*>(wpOwner->GetMotionMaster()->GetCurrent()))
{
wpMMGen->GetPathInformation(wpPathId, wpOrigin);
Expand Down
20 changes: 10 additions & 10 deletions src/game/DBScripts/ScriptMgr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3018,15 +3018,15 @@ bool ScriptAction::ExecuteDbscriptCommand(WorldObject* pSource, WorldObject* pTa
break;
}

FormationEntrySPtr fEntry = std::make_shared<FormationEntry>();
fEntry->GroupId = targetGroup->GetGroupId();
fEntry->Type = static_cast<SpawnGroupFormationType>(m_script->textId[0]);
fEntry->Spread = m_script->x;
fEntry->Options = m_script->textId[1];
fEntry->MovementType = 0;
fEntry->MovementIdOrWander = 0;
fEntry->Comment = "Dynamically created formation!";
fEntry->IsDynamic = true;
FormationEntry fEntry;
fEntry.GroupId = targetGroup->GetGroupId();
fEntry.Type = static_cast<SpawnGroupFormationType>(m_script->textId[0]);
fEntry.Spread = m_script->x;
fEntry.Options = m_script->textId[1];
fEntry.MovementType = 0;
fEntry.MovementIdOrWander = 0;
fEntry.Comment = "Dynamically created formation!";
fEntry.IsDynamic = true;

targetGroup->SetFormationData(fEntry);
break;
Expand Down Expand Up @@ -3082,7 +3082,7 @@ bool ScriptAction::ExecuteDbscriptCommand(WorldObject* pSource, WorldObject* pTa
break;
}

targetGroup->SetFormationData(nullptr);
targetGroup->ClearFormationData();
break;
}

Expand Down
38 changes: 19 additions & 19 deletions src/game/Globals/ObjectMgr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1247,9 +1247,9 @@ void ObjectMgr::LoadSpawnGroups()

entry.Active = false;
entry.EnabledByDefault = true;
entry.formationEntry = nullptr;
entry.FormationEntry = nullptr;
entry.HasChancedSpawns = false;
newContainer->spawnGroupMap.emplace(entry.Id, entry);
newContainer->spawnGroupMap.emplace(entry.Id, std::move(entry));
} while (result->NextRow());
}

Expand All @@ -1260,31 +1260,31 @@ void ObjectMgr::LoadSpawnGroups()
{
Field* fields = result->Fetch();

FormationEntrySPtr fEntry = std::make_shared<FormationEntry>();
fEntry->GroupId = fields[0].GetUInt32();
FormationEntry fEntry;
fEntry.GroupId = fields[0].GetUInt32();
uint32 fType = fields[1].GetUInt32();
fEntry->Spread = fields[2].GetFloat();
fEntry->Options = fields[3].GetUInt32();
fEntry->MovementIdOrWander = fields[4].GetUInt32();
fEntry->MovementType = fields[5].GetUInt32();
fEntry->Comment = fields[6].GetCppString();
fEntry.Spread = fields[2].GetFloat();
fEntry.Options = fields[3].GetUInt32();
fEntry.MovementIdOrWander = fields[4].GetUInt32();
fEntry.MovementType = fields[5].GetUInt32();
fEntry.Comment = fields[6].GetCppString();

auto itr = newContainer->spawnGroupMap.find(fEntry->GroupId);
auto itr = newContainer->spawnGroupMap.find(fEntry.GroupId);
if (itr == newContainer->spawnGroupMap.end())
{
sLog.outErrorDb("LoadSpawnGroups: Invalid group Id:%u found in `spawn_group_formation`. Skipping.", fEntry->GroupId);
sLog.outErrorDb("LoadSpawnGroups: Invalid group Id:%u found in `spawn_group_formation`. Skipping.", fEntry.GroupId);
continue;
}

if (fType >= static_cast<uint32>(SPAWN_GROUP_FORMATION_TYPE_COUNT))
{
sLog.outErrorDb("LoadSpawnGroups: Invalid formation type in `spawn_group_formation` ID:%u. Skipping.", fEntry->GroupId);
sLog.outErrorDb("LoadSpawnGroups: Invalid formation type in `spawn_group_formation` ID:%u. Skipping.", fEntry.GroupId);
continue;
}

if (fEntry->MovementType >= static_cast<uint32>(MAX_DB_MOTION_TYPE))
if (fEntry.MovementType >= static_cast<uint32>(MAX_DB_MOTION_TYPE))
{
sLog.outErrorDb("LoadSpawnGroups: Invalid movement type in `spawn_group_formation` ID:%u. Skipping.", fEntry->GroupId);
sLog.outErrorDb("LoadSpawnGroups: Invalid movement type in `spawn_group_formation` ID:%u. Skipping.", fEntry.GroupId);
continue;
}

Expand All @@ -1299,15 +1299,15 @@ void ObjectMgr::LoadSpawnGroups()
// }
// }

fEntry->Type = static_cast<SpawnGroupFormationType>(fType);
fEntry.Type = static_cast<SpawnGroupFormationType>(fType);

if (fEntry->Spread > 15.0f || fEntry->Spread < -15)
if (fEntry.Spread > 15.0f || fEntry.Spread < -15)
{
sLog.outErrorDb("LoadSpawnGroups: Invalid spread value (%5.2f) should be between (-15..15) in formation ID:%u . Skipping.", fEntry->Spread, fEntry->GroupId);
sLog.outErrorDb("LoadSpawnGroups: Invalid spread value (%5.2f) should be between (-15..15) in formation ID:%u . Skipping.", fEntry.Spread, fEntry.GroupId);
continue;
}

itr->second.formationEntry = std::move(fEntry);
itr->second.FormationEntry = std::make_unique<FormationEntry>(std::move(fEntry));
} while (result->NextRow());
}

Expand Down Expand Up @@ -1390,7 +1390,7 @@ void ObjectMgr::LoadSpawnGroups()
// check and fix correctness of slot id indexation
for (auto& sg : newContainer->spawnGroupMap)
{
if (sg.second.formationEntry == nullptr)
if (sg.second.FormationEntry == nullptr)
continue;

auto& guidMap = sg.second.DbGuids;
Expand Down
71 changes: 33 additions & 38 deletions src/game/Maps/SpawnGroup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -444,8 +444,8 @@ std::string SpawnGroup::to_string() const

CreatureGroup::CreatureGroup(SpawnGroupEntry const& entry, Map& map) : SpawnGroup(entry, map, uint32(TYPEID_UNIT))
{
if (entry.formationEntry)
m_formationData = std::make_shared<FormationData>(this, entry.formationEntry);
if (entry.FormationEntry)
m_formationData = std::make_shared<FormationData>(this, *(entry.FormationEntry.get()));
else
m_formationData = nullptr;
}
Expand Down Expand Up @@ -535,18 +535,16 @@ void CreatureGroup::TriggerLinkingEvent(uint32 event, Unit* target)
}
}

void CreatureGroup::SetFormationData(FormationEntrySPtr fEntry)
void CreatureGroup::SetFormationData(FormationEntry const& fEntry)
{
if (fEntry)
{
m_formationData = std::make_shared<FormationData>(this, fEntry);
m_formationData->Reset();
}
else
{
m_formationData->Disband();
m_formationData = nullptr;
}
m_formationData = std::make_shared<FormationData>(this, fEntry);
m_formationData->Reset();
}

void CreatureGroup::ClearFormationData()
{
m_formationData->Disband();
m_formationData = nullptr;
}

void CreatureGroup::Update()
Expand Down Expand Up @@ -678,8 +676,8 @@ void GameObjectGroup::Despawn(uint32 timeMSToDespawn /*= 0*/, uint32 forcedDespa
// Formation code //
////////////////////

FormationData::FormationData(CreatureGroup* gData, FormationEntrySPtr fEntry) :
m_groupData(gData), m_fEntry(fEntry), m_mirrorState(false), m_followerStopped(false), m_masterDied(false), m_lastWP(0), m_wpPathId(0)
FormationData::FormationData(CreatureGroup* gData, FormationEntry const& fEntry) :
m_groupData(gData), m_formationEntry(fEntry), m_mirrorState(false), m_followerStopped(false), m_masterDied(false), m_lastWP(0), m_wpPathId(0)
{
for (auto const& sData : m_groupData->GetGroupEntry().DbGuids)
{
Expand All @@ -690,15 +688,15 @@ FormationData::FormationData(CreatureGroup* gData, FormationEntrySPtr fEntry) :
m_realMasterDBGuid = sData.DbGuid;
}

m_masterMotionType = static_cast<MovementGeneratorType>(fEntry->MovementType);
m_masterMotionType = static_cast<MovementGeneratorType>(m_formationEntry.MovementType);

// provided slot id should be ordered with no gap!
m_slotGuid = m_slotsMap.size();

// set current value from their defaults
m_currentFormationShape = m_fEntry->Type;
m_currentSpread = m_fEntry->Spread;
m_currentOptions = m_fEntry->Options;
m_currentFormationShape = m_formationEntry.Type;
m_currentSpread = m_formationEntry.Spread;
m_currentOptions = m_formationEntry.Options;
}

FormationData::~FormationData()
Expand Down Expand Up @@ -796,12 +794,12 @@ void FormationData::ClearMoveGen()
Unit* slotUnit = slot->GetOwner();
if (slotUnit && slotUnit->IsAlive())
{
if (m_fEntry->IsDynamic && slot->IsFormationMaster())
if (m_formationEntry.IsDynamic && slot->IsFormationMaster())
continue;
if (slot->IsFormationMaster())
{
// do not change leader movement in dynamic state, script have to handle that
if (m_fEntry->IsDynamic)
if (m_formationEntry.IsDynamic)
continue;

m_lastWP = slotUnit->GetMotionMaster()->getLastReachedWaypoint();
Expand Down Expand Up @@ -829,14 +827,14 @@ void FormationData::SetMasterMovement()
newMaster->GetMotionMaster()->Clear(true, true);
if (m_masterMotionType == WAYPOINT_MOTION_TYPE)
{
newMaster->GetMotionMaster()->MoveWaypoint(m_fEntry->MovementIdOrWander, PATH_FROM_WAYPOINT_PATH);
newMaster->GetMotionMaster()->MoveWaypoint(m_formationEntry.MovementIdOrWander, PATH_FROM_WAYPOINT_PATH);
newMaster->GetMotionMaster()->SetNextWaypoint(m_lastWP + 1);
m_wpPathId = 0;
m_lastWP = 0;
}
else if (m_masterMotionType == LINEAR_WP_MOTION_TYPE)
{
newMaster->GetMotionMaster()->MoveLinearWP(m_fEntry->MovementIdOrWander, PATH_FROM_WAYPOINT_PATH);
newMaster->GetMotionMaster()->MoveLinearWP(m_formationEntry.MovementIdOrWander, PATH_FROM_WAYPOINT_PATH);
newMaster->GetMotionMaster()->SetNextWaypoint(m_lastWP + 1);
m_wpPathId = 0;
m_lastWP = 0;
Expand Down Expand Up @@ -908,7 +906,7 @@ bool FormationData::TrySetNewMaster(Unit* masterCandidat /*= nullptr*/)
FixSlotsPositions();

// will start master movement only if its not dynamic formation
if (!m_fEntry->IsDynamic)
if (!m_formationEntry.IsDynamic)
SetMasterMovement();

SetFollowersMaster();
Expand All @@ -923,7 +921,7 @@ void FormationData::Reset()
m_mirrorState = false;
m_masterDied = false;

SwitchFormation(m_fEntry->Type);
SwitchFormation(m_formationEntry.Type);

FormationSlotMap::iterator slotItr = m_slotsMap.end();
for (FormationSlotMap::iterator itr = m_slotsMap.begin(); itr != m_slotsMap.end();)
Expand Down Expand Up @@ -1334,9 +1332,6 @@ FormationSlotDataSPtr FormationData::SetFormationSlot(Creature* creature, SpawnG

auto const& gEntry = m_groupData->GetGroupEntry();

if (m_fEntry == nullptr)
return nullptr;

uint32 dbGuid = creature->GetDbGuid();

// check if existing slot
Expand All @@ -1363,7 +1358,7 @@ FormationSlotDataSPtr FormationData::SetFormationSlot(Creature* creature, SpawnG
SetMasterMovement();

// reset formation shape as it will restart from node 0 in respawn case
SwitchFormation(m_fEntry->Type);
SwitchFormation(m_formationEntry.Type);
}

if (GetMaster())
Expand Down Expand Up @@ -1391,13 +1386,13 @@ std::string FormationData::to_string() const
std::string fType = FormationType[static_cast<uint32>(m_currentFormationShape)];
std::string fMoveType = GetMoveTypeStr(m_masterMotionType);
std::string fOptions = (HaveOption(SPAWN_GROUP_FORMATION_OPTION_KEEP_CONPACT)) ? ", keepCompact" : "no options";
result << "Formation group id: " << m_fEntry->GroupId << "\n";
result << "Shape: " << fType << "\n";
result << "Spread: " << m_currentSpread << "\n";
result << "MovementType: " << fMoveType << "\n";
result << "MovementId: " << m_fEntry->MovementIdOrWander << "\n";
result << "Options: " << fOptions << "\n";
result << "Comment: " << m_fEntry->Comment << "\n";
result << "Formation group id: " << m_formationEntry.GroupId << "\n";
result << "Shape: " << fType << "\n";
result << "Spread: " << m_currentSpread << "\n";
result << "MovementType: " << fMoveType << "\n";
result << "MovementId: " << m_formationEntry.MovementIdOrWander << "\n";
result << "Options: " << fOptions << "\n";
result << "Comment: " << m_formationEntry.Comment << "\n";

for (auto slot : m_slotsMap)
{
Expand All @@ -1416,8 +1411,8 @@ std::string FormationData::to_string() const
// Change movement data so it can resume it if leader change
void FormationData::SetMovementInfo(MovementGeneratorType moveType, uint32 wanderOrPahtId)
{
m_fEntry->MovementIdOrWander = wanderOrPahtId;
m_fEntry->MovementType = moveType;
m_formationEntry.MovementIdOrWander = wanderOrPahtId;
m_formationEntry.MovementType = moveType;
m_masterMotionType = moveType;
m_lastWP = 0;
auto master = GetMaster();
Expand Down
10 changes: 5 additions & 5 deletions src/game/Maps/SpawnGroup.h
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,9 @@ class CreatureGroup : public SpawnGroup
void RemoveObject(WorldObject* wo) override;
void TriggerLinkingEvent(uint32 event, Unit* target);

void SetFormationData(FormationEntrySPtr fEntry);
void SetFormationData(FormationEntry const& fEntry);
void ClearFormationData();
FormationData* GetFormationData() { return m_formationData.get(); }
FormationEntrySPtr GetFormationEntry() const { return m_entry.formationEntry; }

virtual void Update() override;

Expand Down Expand Up @@ -189,7 +189,7 @@ class FormationSlotData
class FormationData
{
public:
FormationData(CreatureGroup* gData, FormationEntrySPtr fEntry);
FormationData(CreatureGroup* gData, FormationEntry const& fEntry);
FormationData() = delete;
~FormationData();

Expand Down Expand Up @@ -218,7 +218,7 @@ class FormationData
FormationSlotDataSPtr SetFormationSlot(Creature* creature, SpawnGroupFormationSlotType slotType = SPAWN_GROUP_FORMATION_SLOT_TYPE_STATIC);
std::string to_string() const;

FormationEntrySPtr GetFormationEntry() const { return m_fEntry; }
FormationEntry const& GetFormationEntry() const { return m_formationEntry; }
void SetMovementInfo(MovementGeneratorType moveType, uint32 pahtId);

void ResetLastWP() { m_lastWP = 0; }
Expand All @@ -242,7 +242,7 @@ class FormationData
bool HaveOption(SpawGroupFormationOptions const& option) const { return (static_cast<uint32>(m_currentOptions) & option) != 0; }

CreatureGroup* m_groupData;
FormationEntrySPtr m_fEntry;
FormationEntry m_formationEntry;
SpawnGroupFormationType m_currentFormationShape;
FormationSlotMap m_slotsMap;
MovementGeneratorType m_masterMotionType;
Expand Down
2 changes: 1 addition & 1 deletion src/game/Maps/SpawnGroupDefines.h
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ struct SpawnGroupEntry
std::vector<uint32> LinkedGroups;

// may be nullptr
FormationEntrySPtr formationEntry;

This comment has been minimized.

Copy link
@insunaa

insunaa Dec 5, 2024

Contributor

is there a reason why the normal m_FormationEntry naming convention was not used here? Seemingly in the entire struct

This comment has been minimized.

Copy link
@killerwife

killerwife Dec 5, 2024

Author Contributor

Cyberium likes his CapitalCase naming of members

and m_ is only used if the class is larger. No point in using it if its not the case.

This comment has been minimized.

Copy link
@insunaa

insunaa Dec 5, 2024

Contributor

In this case it's causing a collision because the variable has the exact name as the type. What should the convention be to avoid such things in the future?

This comment has been minimized.

Copy link
@killerwife

killerwife Dec 5, 2024

Author Contributor

Its not a collision in c++. Its a collision in other languages.

This comment has been minimized.

Copy link
@insunaa

insunaa Dec 5, 2024

Contributor

It's literally causing build errors because of redefinition, tho.

std::unique_ptr<FormationEntry> FormationEntry;

int32 GetFormationSlotId(uint32 dbGuid) const
{
Expand Down
4 changes: 2 additions & 2 deletions src/game/MotionGenerators/MotionMaster.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,9 @@ void MotionMaster::Initialize()
// check if creature is part of formation and use waypoint path as path origin
WaypointPathOrigin pathOrigin = WaypointPathOrigin::PATH_NO_PATH;
auto creatureGroup = creature->GetCreatureGroup();
if (creatureGroup && creatureGroup->GetFormationEntry() && creatureGroup->GetGroupEntry().GetFormationSlotId(m_owner->GetDbGuid()) == 0)
if (creatureGroup && creatureGroup->GetFormationData() && creatureGroup->GetGroupEntry().GetFormationSlotId(m_owner->GetDbGuid()) == 0)
{
m_currentPathId = creatureGroup->GetFormationEntry()->MovementIdOrWander;
m_currentPathId = creatureGroup->GetFormationData()->GetFormationEntry().MovementIdOrWander;
pathOrigin = WaypointPathOrigin::PATH_FROM_WAYPOINT_PATH;
}

Expand Down

1 comment on commit cf9593d

@anlopo
Copy link

@anlopo anlopo commented on cf9593d Dec 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After this commit Linux build fail at: mangos-tbc/src/game/Maps/SpawnGroupDefines.h:105:37: error: declaration of 'std::unique_ptr SpawnGroupEntry::FormationEntry' changes meaning of 'FormationEntry'

Please sign in to comment.