Auto garrison units produced during an Alert

[Itms]

During an Alert, newly trained units should be put under alert if the level of alert is high enough.

There is also a bunch of bugs to correct with the feature.

[Marcio]

Female one can be, great, military I don't know.

[Itms]

I change the behaviour of the feature : it should not rely on the production building.

[Itms]

This is a way of doing this.

However, I wonder if it's a good idea to use a global message : could it lead to a performance loss ? Please share your thoughts.

[leper]

Why not call into cmpAlertRaiser instead of using a message?

[Itms]

Because the production building is not necessarily an alert raiser.

But if you want, I can also do it like this, so the production buildings are also put "under alert". The problem is that buildings constructed during alert will not put newly trained units under alert ; I'd need to broadcast a GlobalConstructionFinished message and that would raise the same problem.

NB : I also add component tests in EndOfAlert, these should fail when a unit under alert has been killed (which is likely to happen during an attack)

[mimo]

This comment is rather on #2353, but as the patch is in this ticket, let's group both discussions here.

I've not yet looked in details at your patch, but do you also take into account the possibility that the GarrisonHolder is destroyed during the attack (and then its units are ejected before the EndOfAlert) ?

I would guess that we also need a check in AlertRaiser line 104 to check on cmpGarrisonHolder and on cmpUnitAI.IsGarrisoned(), or/and may-be the AlertRaiser should listen to messages from GarrisonedUnitsChanged and if it is from one of its units, move the unit from this.garrisonedUnits <-> this.walkingUnits

In addition, while playing, I had this situation: my CC was nearly destroyed, so the units under alert were ejected, and I asked them to repair the cc and save it. But I think my units lost their previous order due to this repair order. So may-be we should not consider Repair order while under alert (or even no order at all while under alert) as a workOrder in UnitAI::UpdateWorkOrders.

When I say I don't know if my units'order were lost, it's because my game segfaulted before the EndOfAlert, without any backtrace nor JS error. Thus I don't know if the crash had any connection with the Alert or not.

Finally, in UnitAI.js, lines 706 and 717 would need a tab instead

[Itms]

Ok, I see. This patch will take a little more time/work than expected. Holidays are ending and I'll be busy soon.

I'll correct these problems soon, don't hesitate to group any unexpected alert behaviour here. Thanks for your help !

[mimo]

Replying to Itms:

Ok, I see. This patch will take a little more time/work than expected. Holidays are ending and I'll be busy soon.

yes, I've the same issue :)

I'll correct these problems soon, don't hesitate to group any unexpected alert behaviour here. Thanks for your help !

we should nonetheless protect the present code for the time being. If you think the present version patch is still worth, I may try to review and commit it with the only addition of a test on cmpGarrisonHolder in EndOfAlert; otherwise I'dd add only the cmpUnitAI and cmpGarrisonHolder checks and wait for the final version.

[Itms]

I think it's still worth ; thanks for reviewing it, in this case we may close this ticket and group bug reports into #2353 (including possible bugs caused by this patch).

[mimo]

In 14513:

Put into alert units trained during an alert, fixes #2342, patch by Itms

[mimo]

Thanks Itms for the patch. I'll report my previous comment 6 here and a new one in #2353