[PATCH] Eliminate delay in path finding

[Kieran P]

When you task a unit to something, there is a delay. Get rid of this delay! It's really horrible, and long enough to cause problems when you need that extra second it waits.

[historic_bruno]

If the path finding can't respond faster, maybe the unit can just start walking in roughly the direction of the target, until path finding kicks in. That should be about the same in the end, and good enough to fool the user.

[Kenny Long]

Patch to reduce movement time from command to actual movement

[Kenny Long]

This patch adds logic to update so that moves can occur in the same turn they are generated. Previously all moves were queued up until the next turn and then executed. There is a new value in /data/mods/public/simulation/data/pathfinder.xml called MaxSameTurnMoves. This value puts a limit on how many moves we will process during a single turn. It is defaulted to 16 which is just a number I came to that worked well on my slowest machine. If you set this number to 0 it bypasses this functionality all together. So if you are testing and want to rule out this change as an issue then you can turn it off.

[historic_bruno]

Works great for me, nice job! Feels very responsive even on the combat demos :)

The only thing I would ask is do we really need the MaxSameTurnMoves constant in pathfinder.xml? It makes it slightly more convenient to test, but maybe we should just find a value that works reasonably well and stick with that (i.e. define it somewhere in the pathfinding code instead).

[Kenny Long]

Hi Bruno, thank you for the feedback! Yea, when I get the stuck units thing fixed I think the combination will make a huge difference in the responsiveness of units.

I am of the school that you expose everything as a "knob", at least during development. Even in production those knobs can be great for solving performance problems and debugging on a remote users machine. It has really helped me when supporting folks out in the field. But I can gladly remove it if that is not the teams philosophy.

[Philip Taylor]

Looks good :-) . Just some minor nitpicking etc:

I think it's nice to make these values tweakable, if it doesn't add complexity and doesn't hurt performance (time or memory usage - I'd be less happy with constants that were wastefully duplicated for every entity), and in this case it seems fine.

I think the long comment in pathfinder.xml should probably be in the C++ instead, since it's useful for people who are reading the code and who are probably not reading the XML, and only put a very brief comment in the XML (since anyone who's tweaking it should be reading the code to understand the consequences first).

CCmpPathfinder::Serialize and Deserialize ought to handle m_SameTurnMovesCount since it's dynamic state.

m_SameTurnMovesCount should be set to 0 in CCmpPathfinder::Init, else it'll be uninitialised data if we e.g. serialise the game state before the first TurnStart.

"if (m_AsyncLongPathRequests.size() > 0)" should possibly be "if (!m_AsyncLongPathRequests.empty())" since that's more direct (and potentially a very tiny bit more efficient, not that that matters here).

"virtual void ProcessLongRequests(...)" (and Short) don't need to be virtual, and I think it's helpful to consistently use virtual only for functions that were declared in the interface (i.e. not these).

"we will procees some moves" - typo.

[Kenny Long]

Thanks for the specific feeback, it helps me to deliver code that is appropriate for 0AD in style, consistency.

I made the changes and attached them in a second patch.

Cheers!

[Kenny Long]

Changes committed.