[PATCH] UnitAI - alertGarrisoningTarget is undefined

[elexis]

This error appeared in an a18 match. Reproducible with the attached commands.txt file.

Turn 732 (500)... WARNING: JavaScript warning: simulation/components/UnitAI.js line 2746
Script value conversion check failed: v.isNumber() (got type undefined)

Turn 734 (500)... WARNING: JavaScript warning: simulation/components/UnitAI.js line 2746
Script value conversion check failed: v.isNumber() (got type undefined)

---

I could reproduce the error:

1. Fully garrisson your civic centre
2. Raise alert
3. Try to garrisson a woman into that CC
4. That error.

Somehow the error only seems to appear for women, strange.

The attached patch fixes the described issue by adding a simple check.

---

JFYI some forensics on that replay file:

The affected code line with some context:

// States for the special entity representing a group of units moving in formation:
"FORMATIONCONTROLLER": {

"GARRISON": {
...
"APPROACHING": {
...
"MoveCompleted": function() {
...
var cmpGarrisonHolder = Engine.QueryInterface(this.alertGarrisoningTarget, IID_GarrisonHolder);

I printed the contents of this UnitAI for turn 732 and 734, (the only property that differs is losRangeQuery).

{
    "orderQueue": [
        {
            "type": "Garrison",
            "data": {
                "target": 312,
                "force": true
            }
        }
    ],
    "order": {
        "type": "Garrison",
        "data": {
            "target": 312,
            "force": true
        }
    },
    "formationController": 0,
    "isGarrisoned": false,
    "isIdle": false,
    "lastFormationTemplate": "",
    "finishedOrder": false,
    "heldPosition": {
        "x": 272.9604797363281,
        "z": 344.2762451171875
    },
    "workOrders": [],
    "alertRaiser": 312,
    "stance": "passive",
    "fsmStateName": "INDIVIDUAL.GARRISON.APPROACHING",
    "losRangeQuery": 3415,
    "fsmReenter": false
}

So we can reasonably assume that the command that caused the error was:

turn 729 500
cmd 2 {"type":"garrison","entities":[7676,7678],"target":312,"queued":false}

Entity 312 is a briton civic centre of player 2, 7676 and 7678 are females:

    id: 312
    template: "structures/brit_civil_centre"

    id: 7676
    template: "units/brit_support_female_citizen"

    id: 7678
    template: "units/brit_support_female_citizen"

Some other commands that are probably relevant:

turn 656 500
cmd 2 {"type":"increase-alert-level","entities":[312]}

turn 685 500
cmd 2 {"type":"increase-alert-level","entities":[312]}

Notice that the "alert-end" command was never issued.

So the player has raised and increased the alert level (without reseting the alert), then tried to garrison 2 units into that CC and the error occured when they reached the CC.

---

When the second increase-alert-level command was issued, this.alertGarrisoningTarget was set to undefined in L726 when the Order.Alert was processed:

this.alertGarrisoningTarget = this.FindNearbyGarrisonHolder();

[elexis]

However there is another problem with this.

If the CC is fully garrissoned and you create a women, then she will walk to the rallypoint. If the alert was raised, then the women will float to the rallypoint without walking.

This is because the woman has the state INDIVIDUAL.IDLE instead of INDIVIDUAL.WALKING.

If you have a fully garrissoned CC, produce a woman (let her walk to a rallypoint far away), then she will walk correctly. But if you raise the alert while she is walking, she will transition into INDIVIDUAL.IDLE and float to the waypoint. If you stop the alert, then she will stop walking instead of going to that rallypoint. That raise alert button is only used by noobs anyway :o)

[mimo]

In 16968:

fix behavior when under alert, fixes #3273

[mimo]

Thanks for the detailed report.

From my understanding of the problem, this.alertGarrisoningTarget can be undefined there only if no garrisoned place was found when the alert was Raised but the current garrison order was issued later by the player himself. So in that case, we should not go inside the IsUnderAlert if and my fix is a bit different than your proposed one.

Concerning the second problem spotted in comment 1, the resulting state is indeed inconsistent: the unit is put in a IDLE state while still moving. I'm not convinced that going to the rallyPoint would be the wanted behaviour in such a case. Each situation will certainly require different behaviour, and just make the unit stop moving looked the best default to me.