HomeWildfire Games

UnitMotion - Send messages to UnitAI when obstructed, to allow stopping early…
AuditedrP22526

Description

UnitMotion - Send messages to UnitAI when obstructed, to allow stopping early when walking and avoiding pathfinding lag.

As reported by #5521, Ordering units to walk to a point in a forest can lag terribly, as units will end up computing long short paths in the forest, which can result in ComputeShortPath calls that individually take upwards of 80ms.

This can be alleviated by allowing units to stop a bit earlier. A23 handled this in UnitMotion directly, but it's better to handle this in UnitAI to avoid surprises and to make it customisable on a per-state level.

This diff further speeds up using the short pathfinder by starting with a smaller search range, limiting the max-range more and moving the range slightly towards the goal.

This also refactors UM sending messages to UnitAI so that we may in the future push more information (in particular, the entity_id that a unit was obstructed by could be interesting for COMBAT).

This doesn't fix the possibility of lag, but it reduces its occurrence to levels that should be similar to A23 and thus acceptable enough.

Tested By: Freagarach

Fixes #5521

Differential Revision: https://code.wildfiregames.com/D2105

Event Timeline

Freagarach added inline comments.
/ps/trunk/binaries/data/mods/public/simulation/components/UnitAI.js
2064

What if likelyFailure (e.g. target death)? #5610

Freagarach raised a concern with this commit.Oct 6 2019, 12:49 PM

When a target promotes, all units having that as a target will receive a msg.likelyFailure == true, mostly resulting in aborting orders where that is not expected. Can CCmpUnitMotion be informed of units renaming?

This commit now has outstanding concerns.Oct 6 2019, 12:49 PM
elexis added a subscriber: elexis.Feb 5 2020, 1:34 PM
elexis added inline comments.
/ps/trunk/source/simulation2/helpers/VertexPathfinder.cpp
537

Multiply can overflow without warning, the greatest value that fixed can represent is 2 * 128².
So toGoal.Multiply(std::min(toGoalLength / 2, request.range * 3 / 5) sounds like it could overflow without being caught, is that possible?

elexis added inline comments.Feb 5 2020, 1:36 PM
/ps/trunk/source/simulation2/helpers/VertexPathfinder.cpp
537

(And all other Multiply calls I find multiply with a number that is close to 1 or obstruction size)

elexis added inline comments.Feb 5 2020, 3:16 PM
/ps/trunk/source/simulation2/helpers/VertexPathfinder.cpp
537

One possible value printed with current code (r23480):

toGoal(-96.798767, -291.258987), toGoalLength(306.923111), request.range(6.000000)

In another output, request range was 24.
Then is the current request range by a factor of 10 +/- away from overflows?

2^15/300 ≈ 100
100/25 = 4

fixed stores at most 2^15=32k
divided by a big toGoal coordinate of 300 in current svn
divided by a request range of 24 in current svn
yields factor 4 before the overflow barrier.

So it sounds like there should be safeguards around user chosen constants that go into the equation if the magnitude of the multiplied vectors cant be reduced otherwise.

Freagarach accepted this commit.EditedApr 3 2020, 8:25 PM

I still think that UnitMotion being informed of entity renaming would be the best solution.

Nevertheless, my concern raised on this commit has been fixed (by rP23566).

All concerns with this commit have now been addressed.Apr 3 2020, 8:25 PM