[PATCH] Idle units undetected for one second

[elexis]

If a unit becomes idle, it will immediately enter "INDIVIDUAL.IDLE" and update the animation. Meanwhile "isIdle" is still false and only updated exactly one second later on Timer (see r8891). This results in the idle-worker button not finding the unit for one second while it is blatantly idling.

This issue is recognizable in particular if one reduces the gamespeed (replay-speed) to 0.1x.

In order to solve this issue, isIdle must be set true when entering INDIVIDUAL.IDLE instead of waiting one second for the timer to catch it.

But it must be done at the end of the function, since the unit might decide to do things like healing or attacking enemies when entering idle.

[mimo]

I think this timer was used in the past because units in between two orders would go in the IDLE state. But that was causing some errors (for example units in between orders looking for new target), and lines 1447-1448 were added, so that all the code in IDLE state is bypassed in that case. So i think removing this timer now fine.

Concerning the patch itself:

• you should remove all tracks of the timer: the stopTimer in the leave function and the (now empty) Timer body function.
• these temporary idle units won't have the this.idle flag set, so the test in the leave function line 1489 is needed (and maybe add a comment about that), but the test in the enter function line 1470 could be removed (although it does not hurt).

[elexis]

Observation After testing more (with sheep, units with an without formations, garrisoning, rallypoints and queued orders), I found apparently dead code in three places: (1) this.isIdle is always false in INDIVIDUAL.IDLE.enter (as mentioned by mimo) (2) this.isIdle is always true in INDIVIDUAL.IDLE.leave (contrary to mimo's statement) (3) INDIVIDUAL.IDLE.enter is not triggered while this.orderQueue.length > 0 && !this.IsGarrisoned()

Theory The explanation for (1) is this.isIdle being set to

• false only by UnitAI.prototype.Init and INDIVIDUAL.IDLE.leave
• true only by UnitAI.prototype.OnCreate and INDIVIDUAL.IDLE.Timer (i.e. 1 second after INDIVIDUAL.IDLE.enter)

The explanation for (2) is that we return true in INDIVIDUAL.IDLE.enter in case we decide to switch to another task (f.e. going to the guarded object when entering idle, or finding a new enemy in LoS to attack).

The explanation for (3) is that the UnitAI.prototype.FinishOrder doesn't SetNextState("IDLE") if this.orderQueue is empty.

Patch The updated patch removes the dead code, but triggers a warning in case the code (future revisions?) would do one of those unfortunate state-changes.

Refs:

• r8891
• r17304
• source:/ps/trunk/binaries/data/mods/public/simulation/components/UnitAI.js?annotate=blame​

[mimo]

I'm convinced your tests are not enough as these were very rare bugs. These lines (test on this.orderQueue.length > 0 && !this.IsGarrisoned()) were added in r17304, so you should go there and it says that it fixes #3630 which had the same origin as #3466. Both were fixed by this commit, and if you want to test it back, you should use one of the commands.txt given in these tickets (and the corresponding revision).

[sanderd17]

Removing the review tag until the regressions mentioned by mimo are tested.

[elexis]

Guess we don't care.

[Imarok]

Move tickets to Simulation as UI & Simulation got some sub components.