Covert actions are not being cleaned up properly on month end

[pledbrook]

Through debugging, I've discovered that this loop in XCGS_ResistanceFaction.CleanupFactionCovertActions() is broken because the array is being modified (elements removed) while the function is iterating over it. That leads to some of the covert actions been skipped and left in the CovertActions property.

[MrNiceUK]

Actually a basegame bug, the original iterative loop before #435 changed it to a ForEach loop started at 0, so still "skipped" an element when one was removed. Needed to start at the end to avoid it...

[pledbrook]

Is it something that should even be fixed then? I guess any fix should be gated behind a config var, since it would change behaviour that players have gotten used to.

[MrNiceUK]

What impact does the current "bugged" behaviour have on an unmodded game in practice?

[pledbrook]

I've confirmed that vanilla without the highlander is working just fine, so this is definitely a bug introduced by the highlander.

[pledbrook]

OK, so the vanilla loop was also buggy, but it never manifested because the GenerateCovertActions() function used to just clear the CovertActions array itself. Perhaps that was a "quick fix" when someone noticed some of the covert actions being retained 😆 Anyway, the highlander removed that unconditional array clearing, resulting in the manifestation of the bug.

Just tested a reverse traversal loop and it fixes the issue, as one would expect.

[MrNiceUK]

Well, with the Vanilla behaviour the various bits of cleanup done by RemoveEntity() would still be skipped for about half the covert ops, even if GenerateCovertActions() covered up the most obvious issue. But largely academic now really.