Fixed crash on testmaps/test_lotsaimps

[klaussilveira]

root->children[0] and root might be null here, which causes a crash in certain scenarios when playing testmaps/test_lotsaimps.

[DanielGibson]

Have you tried figuring out why that happens?
I.e. why is FindOptimalPath() called with root = NULL?
If I understand the code correctly, root is from root = BuildPathTree( ... ); in idAI::FindPathAroundObstacles().
BuildPathTree() does

root = pathNodeAllocator.Alloc();
root->Init();
...
return root;

So the code in that function is already completely broken if pathNodeAllocator.Alloc() returns NULL, really surprised that it doesn't crash there already

[klaussilveira]

I did not gdb into it. In my specific casing, testing test_lotsaimps, root->children[0] was null.

I added the root check because I've noticed that BFG has it: https://github.com/id-Software/DOOM-3-BFG/blob/master/neo/d3xp/ai/AI_pathing.cpp#L900

[DanielGibson]

Ok - root->children[0] being NULL is possible, the root check in BFG still makes no sense to me.

I haven't really understood that code, but probably one could skip a lot of code if the root returned by BuildPathTree() doesn't have any children (by checking that case right after the BuildPathTree() call and returning false if there are no children - Upd: But OTOH it also seems possible that root->children[0] is set to NULL in PrunePathTree()).. but checking for NULL before dereferencing root->children[0] certainly is the simpler, less invasive solution and done by BFG as well - you could've mentioned that the code originates there.

[klaussilveira]

Both Prey and Quake 4 do something similar, I assumed BFG to be newer:

https://github.com/FriskTheFallenHuman/prey-sdk/blob/64c62d5de8002c9d7df8b202b9a5d3e3f67290ec/src/game/ai/AI_pathing.cpp#L876

https://github.com/Xanz/Quake4SDK/blob/dd2beb850bdaea4038ddf6afca4a8ad26f99c955/src/game/ai/AI_pathing.cpp#L1101

[DanielGibson]

Can you remove the root != NULL parts?
That makes the changes more minimal, which makes porting mods easier (of this I was just reminded when applying the "HeXen: Edge Of Chaos" changes to the dhewm3-sdk: every change to the gamecode is a potential merge conflict, so keeping them minimal reduces friction)

[klaussilveira]

That makes sense. Just pushed it.

[DanielGibson]

Thanks - can you also adjust the commit message? ;)

[klaussilveira]

Done.

[DanielGibson]

Awesome, thanks a lot!