search

TF2-DMB / CBaseNPC

Provides a friendly interface for plugins to use in order to create custom NPCs on the game Team Fortress 2
36 stars 5 forks source link

Fix error case in CNavMesh.BuildPath + add natives #20

Closed KitRifty closed 2 years ago

KitRifty commented 2 years ago

Passing NULL_AREA then a valid goal position into BuildPath will always return false due to CNavArea::Contains not implemented properly. https://github.com/TF2-DMB/CBaseNPC/blob/c5c2c4a255eccfa4867a3ba770b5ef160d51fa51/extension/sourcesdk/nav_area.h#L244

This fixes this case, along with exposing the following natives:

This also adds some more stocks that area useful for working with directions:

Kenzzer commented 2 years ago

Hijacked this PR to add my navmesh update as well.

Implemented our version of CNavMesh class, as CToolsNavMesh, using modern cpp containers. The native CNavMesh.GetNavAreaByID should be faster than valve's equivalent. Next to revisit, should be the position to grid lookup. Added the natives : CNavMesh.GetNavArea CNavMesh.GetNavAreaEntity Removed the following signatures from gamedata :
CNavMesh::SnapToGrid CNavMesh::GetNearestNavArea TheNavMesh

There's also some extra code added for the purpose of removing CNavMesh::GetGroundHeight signature but that hasn't been finished, and will be completed another time.

Kenzzer commented 2 years ago

I got a report from @Mikusch who's currently trying a manual build of this PR on his servers, if the extension is late loaded the extension's TheNavMesh is consequently null, due to CNavMesh::AddNavArea function being already called. This is a direct consequence of me changing to retrieval method of TheNavMesh however this also unearth another bug which plagues previous versions of CBaseNPC, as CNavMesh::AddNavArea detour is also used to fill TheNavAreas.

Should we address the late load issue, or should we let it slide and make it mandatory not to late load the extension ? This could also have consequences for our local testing of the extension (having to change map, between reloads).

KitRifty commented 2 years ago

however this also unearth another bug which plagues previous versions of CBaseNPC, as CNavMesh::AddNavArea detour is also used to fill TheNavAreas.

Can you elaborate on this bug?

Should we address the late load issue, or should we let it slide and make it mandatory not to late load the extension ?

The former, which my previous review addresses. The old retrieval method should work fine in late load circumstances, though personally I prefer to auto-load the extension.

Kenzzer commented 2 years ago

Can you elaborate on this bug?

Well, CNavMesh::AddArea detour, used to be the entry point to fill TheNavAreas vector. However, if the extension was late loaded, that vector would be empty (this plagues all versions of CBaseNPC pior to this PR). This has now been resolved with the latest commits.

KitRifty commented 2 years ago

No further comments, everything looks good to me. 👍