Race condition can cause squads to briefly lose captured targets

[Decane]

Example scenario:

Before:

• Freedom squad 125 is at the "Makeshift Camp" (and assigned to it).
• Freedom squad 111 is a moment away from landing on it (and assigned to the "Eastern Entrance").

After:

• Freedom squad 111 is at the "Makeshift Camp" (and assigned to it).
• Freedom squad 125 is now moving away from it (and assigned to the "Eastern Entrance").

How it can go wrong:

The scenario above usually works fine; the squads 'trade' targets cleanly and life goes on. But sometimes, due to a race condition, it doesn't. Instead, this happens:

Explanation:

When squad 111 reaches the "Makeshift Camp", both 111 and 125 are, briefly, 0 meters away from it. Just then, se_sim_faction:calculate_squad_tasks gets called. Its task is to assign squads to smart terrain targets in the way described in https://github.com/Decane/SRP/issues/62#issuecomment-454157525. We can see how it decides what squad to assign to the "Makeshift Camp" by uncommenting the SRP dbglog statements in the function:

Printing distance_table before squad-to-smart assignment:
[1] target_smart = [Makeshift camp], value = [2], pop = [0/1], squads =
  [1] squad_id = [111], dist = [0]
  [2] squad_id = [125], dist = [0]
  [3] squad_id = [37], dist = [228.30281066895]
  (...)
Assigned freedom squad with ID [111] to [Makeshift camp].
Removed [Makeshift camp] from distance_table since its capacity (1) is saturated (1).
(...)
Printing distance_table before squad-to-smart assignment:
[1] target_smart = [Eastern entrance], value = [2], pop = [0/1], squads =
  [1] squad_id = [125], dist = [243.93707275391]
  [2] squad_id = [47], dist = [685.32049560547]
  (...)
Assigned freedom squad with ID [125] to [Eastern entrance].
Removed [Eastern entrance] from distance_table since its capacity (1) is saturated (1).

We can see that squad 111 gets assigned to the "Makeshift Camp". Why not 125? Because the sorting algorithm used in se_sim_faction:calculate_squad_tasks is agnostic between squads equidistant to the relevant smart terrain (both squads are 0 meters away), and 111 happens to be first in the table.

Usually, none of the above is problematic, because usually, the following happens right afterwards:

Squad [111]: exited sim_attack_point:is_finished() with 'true'
Squad [111]: entered sim_stay_point:make()
Squad [111]: exited sim_stay_point:make()
Squad [125]: entered sim_stay_point:is_finished()
Squad [125]: exited sim_stay_point:is_finished() with 'true'
Squad [125]: entered sim_attack_point:make()
(Squad 111 gets re-assigned to "Makeshift Camp")
(Squad 125 gets re-assigned to "Eastern Entrance")
Squad [125]: exited sim_attack_point:make()

So, in sequence, the usual course of events is:

• squad 111 finishes its attack and is registered to the "Makeshift Camp"
• squad 125 finishes its stay on the "Makeshift Camp" and is booted off towards the "Eastern Entrance"

However, there is a race condition in the engine code governing the respective squads' update() routines! So occasionally, this happens instead:

Squad [125]: entered sim_stay_point:is_finished()
Squad [125]: exited sim_stay_point:is_finished() with 'true'
Squad [125]: entered sim_attack_point:make()
(Squad 111 gets re-assigned to "Makeshift Camp")
(Squad 125 gets assigned to "Eastern Entrance")
Squad [125]: exited sim_attack_point:make()
Squad [111]: entered sim_attack_point:is_finished()
(Squad 111 gets re-assigned to "Makeshift Camp")
(Squad 125 gets re-assigned to "Eastern Entrance")
Squad [111]: exited sim_attack_point:is_finished() with 'true'
Squad [111]: entered sim_stay_point:make()
Squad [111]: exited sim_stay_point:make()

Oh, dear - here, the sequence is the "wrong" way around:

• squad 125 finishes its stay on the "Makeshift Camp" and is booted off towards the "Eastern Entrance"
• squad 111 finishes its attack and is registered to the "Makeshift Camp"

Between squad 125 leaving the "Makeshift Camp" and squad 111 entering it, there is a call to faction_brain_human:calculate_current_expansion, which checks that all precondition_target camps are still owned by Freedom. When it notices that no-one is on the "Makeshift Camp" anymore, it prompts a faction contraction, causing the mess we see in the 5th screenshot above, where the task to capture the "Makeshift Camp" is given and immediately cancelled to the tune of Chekhov lamenting loss and celebrating victory near-simultaneously.

[Decane]

For posterity, here are the full console logs for the scenarios where the race condition doesn't and does break the smart terrain capture, respectively:

• Working despite race condition
• Broken by race condition

[Decane]

Fixed in https://github.com/Decane/SRP/commit/387826659aa723079182ed66639191966766d834.