Skip to content

Commit

Permalink
VMAP: Resolve VMAP manager race conditions with iInstanceMapTrees
Browse files Browse the repository at this point in the history 
Closes #584
  • Loading branch information
@killerwife
killerwife committed Aug 17, 2024
1 parent 431adc5 commit 10284a1
Show file tree
Hide file tree
Showing 5 changed files with 68 additions and 31 deletions.
10 changes: 10 additions & 0 deletions src/game/World/World.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -938,6 +938,16 @@ void World::SetInitialWorldSettings()
DetectDBCLang();
sObjectMgr.SetDbc2StorageLocaleIndex(GetDefaultDbcLocale()); // Get once for all the locale index of DBC language (console/broadcasts)

if (VMAP::IVMapManager* vmmgr2 = VMAP::VMapFactory::createOrGetVMapManager()) // after map store init
{
std::vector<uint32> mapIds;
for (uint32 mapId = 0; mapId < sMapStore.GetNumRows(); mapId++)
if (sMapStore.LookupEntry(mapId))
mapIds.push_back(mapId);

vmmgr2->InitializeThreadUnsafe(mapIds);
}

// Loading cameras for characters creation cinematic
sLog.outString("Loading cinematic...");
LoadM2Cameras(m_dataPath);
Expand Down
2 changes: 2 additions & 0 deletions src/game/vmap/IVMapManager.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ namespace VMAP

virtual ~IVMapManager(void) {}

virtual void InitializeThreadUnsafe(const std::vector<uint32>& mapIds) = 0;

This comment has been minimized.

Sign in to view

Copy link
@cpevors

cpevors Aug 17, 2024

vector is not a member of std

This comment has been minimized.

Sign in to view

Copy link
@cpevors

cpevors Aug 17, 2024

image

Indeed, getting a few errors with this change.

This comment has been minimized.

Sign in to view

Copy link
@insunaa

insunaa Aug 17, 2024 •
edited
Loading

Contributor

vector is part of std. may need an

#include <vector>

tho
I guess a general include for our type defines is missing too, given that uint32 is not recognized

virtual VMAPLoadResult loadMap(const char* pBasePath, unsigned int pMapId, int x, int y) = 0;

virtual bool existsMap(const char* pBasePath, unsigned int pMapId, int x, int y) = 0;
Expand Down
13 changes: 3 additions & 10 deletions src/game/vmap/VMapFactory.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ namespace VMAP
}
}

std::mutex m_vmapMutex;
IVMapManager* gVMapManager = nullptr;

//===============================================
Expand Down Expand Up @@ -83,22 +82,16 @@ namespace VMAP
// just return the instance
IVMapManager* VMapFactory::createOrGetVMapManager()
{
if (!gVMapManager)
{
std::lock_guard<std::mutex> lock(m_vmapMutex);
if (!gVMapManager)
gVMapManager = new VMapManager2(); // should be taken from config ... Please change if you like :-)
}
if (!gVMapManager) // called once on load
gVMapManager = new VMapManager2(); // should be taken from config ... Please change if you like :-)
return gVMapManager;
}

//===============================================
// delete all internal data structures
void VMapFactory::clear()
{
std::lock_guard<std::mutex> lock(m_vmapMutex);

delete gVMapManager;
delete gVMapManager; // called once on world destruction where single threaded is guaranteed
gVMapManager = nullptr;
}
}
69 changes: 48 additions & 21 deletions src/game/vmap/VMapManager2.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ namespace VMAP
{
//=========================================================

VMapManager2::VMapManager2()
VMapManager2::VMapManager2() : m_thread_safe_environment(true)
{
}

Expand All @@ -50,6 +50,15 @@ namespace VMAP
}
}

void VMapManager2::InitializeThreadUnsafe(const std::vector<uint32>& mapIds)
{
// the caller must pass the list of all mapIds that will be used in the VMapManager2 lifetime
for (const uint32& mapId : mapIds)
iInstanceMapTrees.insert(InstanceTreeMap::value_type(mapId, nullptr));

m_thread_safe_environment = false;
}

//=========================================================

Vector3 VMapManager2::convertPositionToInternalRep(float x, float y, float z) const
Expand All @@ -63,6 +72,16 @@ namespace VMAP
return pos;
}

InstanceTreeMap::const_iterator VMapManager2::GetMapTree(uint32 mapId) const
{
// return the iterator if found or end() if not found/NULL
InstanceTreeMap::const_iterator itr = iInstanceMapTrees.find(mapId);
if (itr != iInstanceMapTrees.cend() && !itr->second)
itr = iInstanceMapTrees.cend();

return itr;
}

// move to MapTree too?
std::string VMapManager2::getMapFileName(unsigned int pMapId)
{
Expand Down Expand Up @@ -92,21 +111,29 @@ namespace VMAP
bool VMapManager2::IsTileLoaded(uint32 mapId, uint32 x, uint32 y) const
{
InstanceTreeMap::const_iterator instanceTree = iInstanceMapTrees.find(mapId);
if (instanceTree == iInstanceMapTrees.end())
if (instanceTree == iInstanceMapTrees.end() || instanceTree->second == nullptr)
return false;
return instanceTree->second->IsTileLoaded(x, y);
}

//=========================================================
// load one tile (internal use only)

bool VMapManager2::_loadMap(unsigned int pMapId, const std::string& basePath, uint32 tileX, uint32 tileY)
bool VMapManager2::_loadMap(unsigned int mapId, const std::string& basePath, uint32 tileX, uint32 tileY)
{
InstanceTreeMap::iterator instanceTree = iInstanceMapTrees.find(pMapId);
InstanceTreeMap::iterator instanceTree = iInstanceMapTrees.find(mapId);
if (instanceTree == iInstanceMapTrees.end())
{
std::string mapFileName = getMapFileName(pMapId);
StaticMapTree* newTree = new StaticMapTree(pMapId, basePath);
if (m_thread_safe_environment)
instanceTree = iInstanceMapTrees.insert(InstanceTreeMap::value_type(mapId, nullptr)).first;
else
MANGOS_ASSERT(false && "Invalid mapId passed to VMapManager2 after startup in thread unsafe environment");
}

if (!instanceTree->second)
{
std::string mapFileName = getMapFileName(mapId);
StaticMapTree* newTree = new StaticMapTree(mapId, basePath);
if (!newTree->InitMap(mapFileName, this))
{
delete newTree;
Expand All @@ -116,7 +143,7 @@ namespace VMAP
// insert new data
{
std::lock_guard<std::mutex> lock(m_vmStaticMapMutex);
instanceTree = iInstanceMapTrees.insert(InstanceTreeMap::value_type(pMapId, newTree)).first;
instanceTree->second = newTree;
}
}
return instanceTree->second->LoadMapTile(tileX, tileY, this);
Expand All @@ -127,13 +154,13 @@ namespace VMAP
void VMapManager2::unloadMap(unsigned int pMapId)
{
InstanceTreeMap::iterator instanceTree = iInstanceMapTrees.find(pMapId);
if (instanceTree != iInstanceMapTrees.end())
if (instanceTree != iInstanceMapTrees.end() && instanceTree->second)
{
instanceTree->second->UnloadMap(this);
if (instanceTree->second->numLoadedTiles() == 0)
{
delete instanceTree->second;
iInstanceMapTrees.erase(pMapId);
instanceTree->second = nullptr;
}
}
}
Expand All @@ -143,24 +170,24 @@ namespace VMAP
void VMapManager2::unloadMap(unsigned int pMapId, int x, int y)
{
InstanceTreeMap::iterator instanceTree = iInstanceMapTrees.find(pMapId);
if (instanceTree != iInstanceMapTrees.end())
if (instanceTree != iInstanceMapTrees.end() && instanceTree->second)
{
instanceTree->second->UnloadMapTile(x, y, this);
if (instanceTree->second->numLoadedTiles() == 0)
{
delete instanceTree->second;
iInstanceMapTrees.erase(pMapId);
instanceTree->second = nullptr;
}
}
}

//==========================================================

bool VMapManager2::isInLineOfSight(unsigned int pMapId, float x1, float y1, float z1, float x2, float y2, float z2, bool ignoreM2Model)
bool VMapManager2::isInLineOfSight(unsigned int mapId, float x1, float y1, float z1, float x2, float y2, float z2, bool ignoreM2Model)
{
if (!isLineOfSightCalcEnabled()) return true;
bool result = true;
InstanceTreeMap::iterator instanceTree = iInstanceMapTrees.find(pMapId);
InstanceTreeMap::const_iterator instanceTree = GetMapTree(mapId);
if (instanceTree != iInstanceMapTrees.end())
{
Vector3 pos1 = convertPositionToInternalRep(x1, y1, z1);
Expand All @@ -177,15 +204,15 @@ namespace VMAP
get the hit position and return true if we hit something
otherwise the result pos will be the dest pos
*/
bool VMapManager2::getObjectHitPos(unsigned int pMapId, float x1, float y1, float z1, float x2, float y2, float z2, float& rx, float& ry, float& rz, float pModifyDist)
bool VMapManager2::getObjectHitPos(unsigned int mapId, float x1, float y1, float z1, float x2, float y2, float z2, float& rx, float& ry, float& rz, float pModifyDist)
{
bool result = false;
rx = x2;
ry = y2;
rz = z2;
if (isLineOfSightCalcEnabled())
{
InstanceTreeMap::iterator instanceTree = iInstanceMapTrees.find(pMapId);
InstanceTreeMap::const_iterator instanceTree = GetMapTree(mapId);
if (instanceTree != iInstanceMapTrees.end())
{
Vector3 pos1 = convertPositionToInternalRep(x1, y1, z1);
Expand All @@ -206,12 +233,12 @@ namespace VMAP
get height or INVALID_HEIGHT if no height available
*/

float VMapManager2::getHeight(unsigned int pMapId, float x, float y, float z, float maxSearchDist)
float VMapManager2::getHeight(unsigned int mapId, float x, float y, float z, float maxSearchDist)
{
float height = VMAP_INVALID_HEIGHT_VALUE; // no height
if (isHeightCalcEnabled())
{
InstanceTreeMap::iterator instanceTree = iInstanceMapTrees.find(pMapId);
InstanceTreeMap::const_iterator instanceTree = GetMapTree(mapId);
if (instanceTree != iInstanceMapTrees.end())
{
Vector3 pos = convertPositionToInternalRep(x, y, z);
Expand All @@ -227,10 +254,10 @@ namespace VMAP

//=========================================================

bool VMapManager2::getAreaInfo(unsigned int pMapId, float x, float y, float& z, uint32& flags, int32& adtId, int32& rootId, int32& groupId) const
bool VMapManager2::getAreaInfo(unsigned int mapId, float x, float y, float& z, uint32& flags, int32& adtId, int32& rootId, int32& groupId) const
{
bool result = false;
InstanceTreeMap::const_iterator instanceTree = iInstanceMapTrees.find(pMapId);
InstanceTreeMap::const_iterator instanceTree = GetMapTree(mapId);
if (instanceTree != iInstanceMapTrees.end())
{
Vector3 pos = convertPositionToInternalRep(x, y, z);
Expand Down Expand Up @@ -319,8 +346,8 @@ namespace VMAP
}
//=========================================================

bool VMapManager2::existsMap(const char* pBasePath, unsigned int pMapId, int x, int y)
bool VMapManager2::existsMap(const char* pBasePath, unsigned int mapId, int x, int y)
{
return StaticMapTree::CanLoadMap(std::string(pBasePath), pMapId, x, y);
return StaticMapTree::CanLoadMap(std::string(pBasePath), mapId, x, y);
}
} // namespace VMAP
5 changes: 5 additions & 0 deletions src/game/vmap/VMapManager2.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ namespace VMAP
std::mutex m_vmStaticMapMutex;
std::mutex m_vmModelMutex;

bool m_thread_safe_environment;

protected:
// Tree to check collision
ModelFileMap iLoadedModelFiles;
Expand All @@ -80,11 +82,14 @@ namespace VMAP
public:
// public for debug
G3D::Vector3 convertPositionToInternalRep(float x, float y, float z) const;
InstanceTreeMap::const_iterator GetMapTree(uint32 mapId) const;
static std::string getMapFileName(unsigned int pMapId);

VMapManager2();
~VMapManager2();

void InitializeThreadUnsafe(const std::vector<uint32>& mapIds);

VMAPLoadResult loadMap(const char* pBasePath, unsigned int pMapId, int x, int y) override;
bool IsTileLoaded(uint32 mapId, uint32 x, uint32 y) const override;

Expand Down

0 comments on commit 10284a1

Please sign in to comment.