Pathfinder memory leak fix and actually use saved polyRefs.

[mostlikely4r]

🍰 Pullrequest

This pull request fixes the pathfinders ability to actually use previously saved polyRefs and stops the same from being saved over and over.

An issue was found by looking for memory leaks. For a server that was running for multiple hours Pathfinder.m_pathPolyRefs

mangos-classic/src/game/MotionGenerators/PathFinder.h

Line 102 in 1debc9c

std::vector<dtPolyRef> m_pathPolyRefs; // array of detour polygon references

was filled with many duplicate records. Futher inspection showed these duplicate records are not used.

This issue is introduced in (9c0a31b) and probably the result of an imcomplete change.

This PR fixes the issue where saved polyRefs are not properly found for later use. This also attempts to prevent the same polyRef from being saved over and over again.

Proof

When looking for polys the pathfinder will first look at polys saved in the current path:

mangos-classic/src/game/MotionGenerators/PathFinder.cpp

Line 172 in 1debc9c

dtPolyRef polyRef = getPathPolyByPosition(m_pathPolyRefs.data(), m_polyLength, point, distance);

However if no suitable poly is found two broader searches are executed to find a result:

mangos-classic/src/game/MotionGenerators/PathFinder.cpp

Line 180 in 1debc9c

dtStatus dtResult = m_navMeshQuery->findNearestPoly(point, NearPolySearchBound, &m_filter, &polyRef, closestPoint);

Any found poly is then saved in Pathfinder.m_pathPolyRefs with a pushback in:

mangos-classic/src/game/MotionGenerators/PathFinder.cpp

Line 184 in 1debc9c

m_pathPolyRefs.push_back(polyRef);

Pathfinder.m_pathPolyRefs however used to be an array but is now a vector that is initalized with a default size of MAX_POINT_PATH_LENGTH in:

mangos-classic/src/game/MotionGenerators/PathFinder.cpp

Line 38 in 1debc9c

m_cachedPoints(m_pointPathLimit * VERTEX_SIZE), m_pathPolyRefs(m_pointPathLimit), m_polyLength(0),

This means that new polyRefs pushed_back into Pathfinder.m_pathPolyRefs will be at the end at location MAX_POINT_PATH_LENGTH+(number of saved polyPaths).
However when looping over this vector it loops from 0 to m_polyLength in:

mangos-classic/src/game/MotionGenerators/PathFinder.cpp

Line 144 in 1debc9c

for (uint32 i = 0; i < polyPathSize; ++i)

Since m_polyLength is the length of the path the extra Polys at the end of the vector are never found.

The result is in:

mangos-classic/src/game/MotionGenerators/PathFinder.cpp

Line 167 in 1debc9c

dtPolyRef PathFinder::getPolyByLocation(const float* point, float* distance)

-The path is checked for a suitable poly.
-If none is found a broader search is done using the navmesh query.
-Any result is saved at the end of the vector containing the path.
-The next search will still not find this previously saved poly in the path part of the vector.
-Again a search is done with navmesh query.
-The result is saved again in the vector.
-Ect.

Issues

This fixes a memory leak for units that do pathfinding over a longer time.
This 'should' give some performance increase because it prevents some extra navmesh queries.

How2Test

While the server is running for a little while, put a breakpoint at:

mangos-classic/src/game/MotionGenerators/PathFinder.cpp

Line 167 in 1debc9c

dtPolyRef PathFinder::getPolyByLocation(const float* point, float* distance)

Notice that this->m_pathPolyRefs does not contain many duplicate records at the end of the vector and the size isn't too many times MAX_POINT_PATH_LENGTH.

Todo / Checklist

An alternative to this PR is to simply remove the push_backs until futher notice or refactor the pathfinding code to initialize m_pathPolyRefs without a size and dynamically size the vector when needed instead. Ideally I'd like to discuss proper implementation of 9c0a31b with the autor or anyone who has a decent graps of the pathfinder code.

[mostlikely4r]

I see I added a merge commit to this PR. I'll remake the PR.