[PATCHES] Multiple Town Bell defects

[Michael]

I got an JS error using the town bell during am MP game. (r14508)

The units went into the buildings but when I ended the alarm, I got this error and some units didn't leave the buildings. Perhaps the units which where not in the civic center did not leave thier shelter. But I'm not sure about that.

ERROR: JavaScript error: simulation/components/AlertRaiser.js line 89 TypeError: cmpUnitAI is null ()@simulation/components/AlertRaiser.js:89 ProcessCommand(4,[object Object])@simulation/helpers/Commands.js:440

I'm not sure, if this is enough information for you to get a clue whats the problem...

[Itms]

I think I corrected the problem with my last patch on #2342. I leave this ticket open, please tell me if it's solved once it is committed.

Thanks for your feedback ! :)

[mimo]

I've just commited #2342 in r14513, but here are some additionnal comments on the AlertRaiser:

We should take into account the possibility that the GarrisonHolder? is destroyed during the attack (and then its units are ejected before the EndOfAlert?) ? I've added a check to cmpGarrisonHolder in EndOfAlert in your patch, but we should also add a check to 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?

We could add a new fonction AddUnits to the AlertRaiser which would only add an array of units. This would be useful in ProductionQueue instead of calling UpdateUnits while we know exactly which units to add.

[Michael]

Replying to mimo:

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?

I had the same problem and would prefer that, too!

[Itms]

Ok, I wrote two small patches that correct separately -> the behaviour of units ejected out of destroyed buildings (along with the UpdateUnits function) -> unwanted work orders updates during alert

Additionally, I came across a problem with garrison order : when there is not enough room to garrison units in the first garrisonholder, a unit can try to go somewhere else, but comes back to the filled holder and stops. To reproduce it, playing with the Ptolemies, a house, a CC and four women closer to the house are sufficient.

[Itms]

Improved IsUnderAlert in UnitAI.

[Itms]

And also corrected the tooltip as suggested by mimo.

[mimo]

Thanks for the patches Itms. I had a look at the two last patches and here are some comments:

tooltip.patch: The alert button is now dupplicated once an alert is raised. I would have put first the if block with hasRaisedAlert, and then an elseif block with canIncreaseLevel

work_orders_under_alert.patch: You've added this.isUnderAlert, but isn't it a redondance with this.alertRaiser ? if we just add this.alertRaiser = undefined is this.ResetAlert, and return (this.alertRaiser !== undefined) in IsUnderAlert, that should work ?

I've not yet looked at your third patch. Concerning the bug you reported, is it fixed by your patch ?

[Itms]

I corrected this :)

The bug I reported is independent and not fixed by these patches, I'm currently working on it.

[Itms]

Okay, I found it, it was only a small mistake with UnitAI states.

[mimo]

In 14683:

keep work orders when in alert, patch by Itms, addresses #2353

[mimo]

In 14684:

improve alert tooltip, see #2353

[mimo]

In 14685:

fix bug when garrisoning under alert and garrisonHolder is full, patch by Itms, see #2353

[mimo]

In 14689:

fixes remaining town bell defects, closes #2353, patch by Itms

[mimo]

Thanks Itms for the patches.