Add locking to some UI actions.

This is by no means complete. The bugs that this solves were already in
6.x, but we'd hidden the speed controls for the sim in that release, and
have always said that anything done after pressing "go" the first time
is undefined behavior. This is the first step on making those mid-sim
actions behave correctly.

UI actions such as creating a new package need to be executed between
ticks of the sim. We can either do this synchronously by blocking the UI
until the tick is done executing, acquiring a lock on the sim, executing
the action, then releasing the lock; or asynchronously by queueing
events and letting the sim execute them when it completes the current
tick (or instantly if the sim is paused).

Anything that comes from the new UI (currently just the map) must be
asynchronous because it goes through the REST API, but for the old UI
it's simpler (and because the lock will only be acquired as quickly as
the user can act, shouldn't slow anything down) to do this
synchronously, since it's difficult to use coroutines in Qt.

https://github.com/dcs-liberation/dcs_liberation/issues/1680

game/sim/gameloop.py

@@ -1,9 +1,10 @@
 from __future__ import annotations
 
 import logging
+from contextlib import contextmanager
 from datetime import datetime, timedelta
 from pathlib import Path
-from typing import TYPE_CHECKING
+from typing import Iterator, TYPE_CHECKING
 
 from .gamelooptimer import GameLoopTimer
 from .gameupdatecallbacks import GameUpdateCallbacks
@@ -52,6 +53,11 @@ class GameLoop:
             self.start()
         self.timer.set_speed(simulation_speed)
 
+    @contextmanager
+    def paused_sim(self) -> Iterator[None]:
+        with self.timer.locked_pause():
+            yield
+
     def run_to_first_contact(self) -> None:
         self.pause()
         if not self.started:

game/sim/gamelooptimer.py

@@ -1,4 +1,6 @@
-from threading import Lock, Timer
+from collections.abc import Iterator
+from contextlib import contextmanager
+from threading import RLock, Timer
 from typing import Callable, Optional
 
 from .simspeedsetting import SimSpeedSetting
@@ -9,18 +11,37 @@ class GameLoopTimer:
         self.callback = callback
         self.simulation_speed = SimSpeedSetting.PAUSED
         self._timer: Optional[Timer] = None
-        self._timer_lock = Lock()
+        # Reentrant to allow a single thread nested use of `locked_pause`.
+        self._timer_lock = RLock()
 
     def set_speed(self, simulation_speed: SimSpeedSetting) -> None:
         with self._timer_lock:
-            self._stop()
+            self._set_speed(simulation_speed)
-            self.simulation_speed = simulation_speed
-            self._recreate_timer()
 
     def stop(self) -> None:
         with self._timer_lock:
             self._stop()
 
+    @contextmanager
+    def locked_pause(self) -> Iterator[None]:
+        # NB: This must be a locked _pause_ and not a locked speed, because nested use
+        # of this method is allowed. That's okay if all nested callers set the same
+        # speed (paused), but not okay if a parent locks a speed and a child locks
+        # another speed. That's okay though, because we're unlikely to ever want to lock
+        # any speed but paused.
+        with self._timer_lock:
+            old_speed = self.simulation_speed
+            self._stop()
+            try:
+                yield
+            finally:
+                self._set_speed(old_speed)
+
+    def _set_speed(self, simulation_speed: SimSpeedSetting) -> None:
+        self._stop()
+        self.simulation_speed = simulation_speed
+        self._recreate_timer()
+
     def _stop(self) -> None:
         if self._timer is not None:
             self._timer.cancel()

qt_ui/models.py

@@ -158,6 +158,7 @@ class PackageModel(QAbstractListModel):
 
     def add_flight(self, flight: Flight) -> None:
         """Adds the given flight to the package."""
+        with self.game_model.sim_controller.paused_sim():
             self.beginInsertRows(QModelIndex(), self.rowCount(), self.rowCount())
             self.package.add_flight(flight)
             # update_tot is not called here because the new flight does not have a
@@ -169,6 +170,7 @@ class PackageModel(QAbstractListModel):
         self.cancel_or_abort_flight(self.flight_at_index(index))
 
     def cancel_or_abort_flight(self, flight: Flight) -> None:
+        with self.game_model.sim_controller.paused_sim():
             if flight.state.cancelable:
                 self.delete_flight(flight)
                 EventStream.put_nowait(GameUpdateEvents().delete_flight(flight))
@@ -178,6 +180,7 @@ class PackageModel(QAbstractListModel):
 
     def delete_flight(self, flight: Flight) -> None:
         """Removes the given flight from the package."""
+        with self.game_model.sim_controller.paused_sim():
             index = self.package.flights.index(flight)
             self.beginRemoveRows(QModelIndex(), index, index)
             self.package.remove_flight(flight)
@@ -191,14 +194,17 @@ class PackageModel(QAbstractListModel):
         return self.package.flights[index.row()]
 
     def set_tot(self, tot: datetime.datetime) -> None:
+        with self.game_model.sim_controller.paused_sim():
             self.package.time_over_target = tot
             self.update_tot()
 
     def set_asap(self, asap: bool) -> None:
+        with self.game_model.sim_controller.paused_sim():
             self.package.auto_asap = asap
             self.update_tot()
 
     def update_tot(self) -> None:
+        with self.game_model.sim_controller.paused_sim():
             if self.package.auto_asap:
                 self.package.set_tot_asap(
                     self.game_model.sim_controller.current_time_in_sim
@@ -263,10 +269,11 @@ class AtoModel(QAbstractListModel):
 
     def add_package(self, package: Package) -> None:
         """Adds a package to the ATO."""
+        with self.game_model.sim_controller.paused_sim():
             self.beginInsertRows(QModelIndex(), self.rowCount(), self.rowCount())
             self.ato.add_package(package)
-        # We do not need to send events for new flights in the package here. Events were
+            # We do not need to send events for new flights in the package here. Events
-        # already sent when the flights were added to the in-progress package.
+            # were already sent when the flights were added to the in-progress package.
             self.endInsertRows()
             # noinspection PyUnresolvedReferences
             self.client_slots_changed.emit()
@@ -277,6 +284,7 @@ class AtoModel(QAbstractListModel):
         self.cancel_or_abort_package(self.package_at_index(index))
 
     def cancel_or_abort_package(self, package: Package) -> None:
+        with self.game_model.sim_controller.paused_sim():
             with EventStream.event_context() as events:
                 if all(f.state.cancelable for f in package.flights):
                     events.delete_flights_in_package(package)

qt_ui/simcontroller.py

@@ -1,6 +1,8 @@
 from __future__ import annotations
 
 import logging
+from collections.abc import Iterator
+from contextlib import contextmanager
 from datetime import datetime, timedelta
 from pathlib import Path
 from typing import Callable, Optional, TYPE_CHECKING
@@ -76,6 +78,11 @@ class SimController(QObject):
             self.started = True
         self.game_loop.set_simulation_speed(simulation_speed)
 
+    @contextmanager
+    def paused_sim(self) -> Iterator[None]:
+        with self.game_loop.paused_sim():
+            yield
+
     def run_to_first_contact(self) -> None:
         self.game_loop.run_to_first_contact()