[rc2] Korolov raid attacking dead station.

[Atarlost]

I just had a Korolov mission to take out the Charon fortress in the system and the Korolov gunships are not registering the station as dead and going home. I think they didn't get the attack order until the station was dead. They wait a bit at a distance from the target, presumably for slow players, but then don't check if the station's still active before dedicating themselves to blasting the wreck. This came up using a plain vanilla un-augmented PK-25 on a Freyr.

[george moromisato]

This is caused by a subtle bug that is too risky to fix for 1.2, but which definitely should be fixed right afterwards.

The way the gunships are told that the station is destroyed is by the normal method of registering for events (objRegisterForEvents) and a call to <OnObjDestroyed>.

The bug happens because BOTH the Korolov station and the lead gunship register for events on the Charon Stronghold. The way this is implemented, the Charon Stronghold keeps a list of all the objects that have subscribed to its events. So once the mission is set, the Stronghold has a list with two elements:

Subscribers:
[0] = Korolov Station
[1] = Lead Gunship

When the Stronghold is destroyed, it loops over its list and fires events:

Code: Select all

//  pseudocode
for (i = 0; i < Subscribers.GetCount(); i++)
   Subscribers[i].FireOnObjDestroyed();

So far so good. But what happens if, inside <OnObjDestroyed> the code decides to UNSUBSCRIBE from the event? This is exactly what the Korolov Station code does.

The unsubscribe code naturally removes the object from the list. This means that the list now looks like this:

Subscribers:
[0] = Lead Gunship

In the middle of iterating over the list we've deleted one of the elements! After the first iteration, GetCount() return 1 (instead of 2) and since we've already iterated once, the loop exits--it thinks it's done. The result is that we never call <OnObjDestroyed> for the lead gunship and so it never realizes that the station was destroyed.

To fix this I have to make a COPY of the subscription list before firing events. Then the loop can iterate over the copy and not worry about having it change. I'll have to do this for all loops that deal with registered events (OnObjDestroyed, OnObjDocked, etc.) Unfortunately, this is risky enough that I'd rather wait until after 1.2.

I believe this bug has been around since the 1.2 Alpha, so it should be relatively rare.

[Atarlost]

I don't think this is that problem. It's not relatively rare, it's repeatable. If you kill the station while the gunships are waiting before their attack they will attack the dead station.

If you get out in front of them and have a good in-depth weapon this isn't difficult.

[george moromisato]

I don't think this is that problem. It's not relatively rare, it's repeatable. If you kill the station while the gunships are waiting before their attack they will attack the dead station.

If you get out in front of them and have a good in-depth weapon this isn't difficult.

I agree that it is repeatable. I could reproduce it easily and I saw exactly where the call failed (in the debugger). I probably should have said "relatively harmless" instead.

In any case, I agree it has to be fixed--I'm just too nervous to fix it in 1.2 because we won't have enough time to test the fix.

[Aury]

Is this possible:

Just replace the unsubscribe with a timer event for 1 tick (the next tick), which then triggers a dummy event containing only the unsubscribe?