Refactor SimulationController to callback-based lifecycle API and move EDT UI updates into Frame (#488)

* refactor: decouple SimulationController from Swing components

Agent-Logs-Url: https://github.com/bedaHovorka/interlockSim/sessions/01419796-a691-46e1-ae10-8c1373a9ee18

Co-authored-by: bedaHovorka <5263405+bedaHovorka@users.noreply.github.com>

* test: adapt SimulationController tests to callback-based UI wiring

Agent-Logs-Url: https://github.com/bedaHovorka/interlockSim/sessions/01419796-a691-46e1-ae10-8c1373a9ee18

Co-authored-by: bedaHovorka <5263405+bedaHovorka@users.noreply.github.com>

* fix: handle stopped runner detachment and initial speed callback

Agent-Logs-Url: https://github.com/bedaHovorka/interlockSim/sessions/e88ff78a-7a85-4ecd-9542-da0b8e356164

Co-authored-by: bedaHovorka <5263405+bedaHovorka@users.noreply.github.com>

* fix: resolve four Important issues from code review

- StatusBar.propertyChange: dispatch label update via invokeLater when
  called from non-EDT thread (EDT violation now always reachable since
  StatusBar stays visible during simulation)
- SimulationControlPanel: add onSpeedChanged callback wired to
  SimulationController.setSpeed() so desiredSpeed stays in sync; speed
  selected via slider/preset is now honoured after stop+start
- Frame.setContext: close previous SimulationContext after new wiring to
  prevent Koin scope leak on repeated Simulation > Start... invocations
- SimulationControllerTest: replace Thread.sleep(100) with flushEDT(times=3)
  to eliminate timing-dependent flakiness in stale-monitor test

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: bedaHovorka <5263405+bedaHovorka@users.noreply.github.com>
Co-authored-by: Bedrich Hovorka <bedrich.hovorka@gmail.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

Author: 3 people

core-test/src/commonMain/kotlin/cz/vutbr/fit/interlockSim/testutil/MockSimulationContext.kt

@@ -48,6 +48,10 @@ class MockSimulationContext(
 	private var stopped: Boolean = false
 	private var runListeners: List<ContextPropertyChangeListener> = emptyList()
 
+	/** Number of times [close] has been called. Used by tests to verify scope cleanup. */
+	var closeCount: Int = 0
+		private set
+
 	/**
 	 * On-demand cache for [DynamicTrack] wrappers.
 	 *
@@ -104,6 +108,10 @@ class MockSimulationContext(
 		runListeners.forEach { it.propertyChange(event) }
 	}
 
+	override fun close() {
+		closeCount++
+	}
+
 	override fun stop() {
 		stopped = true
 	}

desktop-ui/src/main/kotlin/cz/vutbr/fit/interlockSim/gui/Frame.kt

@@ -26,6 +26,7 @@ import javax.swing.JFrame
 import javax.swing.JOptionPane
 import javax.swing.JPanel
 import javax.swing.JScrollPane
+import javax.swing.SwingUtilities
 import javax.swing.Timer
 
 /**
@@ -113,7 +114,30 @@ class Frame : JFrame(PROGRAM_FULL_NAME) {
 	}
 
 	// Simulation lifecycle delegated to SimulationController for testability (Issue #189)
-	internal val simulationController: SimulationController = SimulationController(controlPanel, toolBar, statusBar)
+	internal val simulationController: SimulationController =
+		SimulationController(
+			onStateChanged = { state ->
+				runOnEdt {
+					when (state) {
+						SimulationController.SimulationStatus.RUNNING -> {
+							toolBar.showSimulationControls()
+							controlPanel.updateStatus(ControlPanel.SimulationStatus.RUNNING)
+							controlPanel.setStopEnabled(true)
+						}
+
+						SimulationController.SimulationStatus.STOPPED -> {
+							toolBar.hideSimulationControls()
+							simulationControlPanel.runner = null
+							controlPanel.setStopEnabled(false)
+							controlPanel.updateStatus(ControlPanel.SimulationStatus.STOPPED)
+						}
+					}
+				}
+			},
+			onSpeedChanged = { speed ->
+				runOnEdt { statusBar.updateSpeedIndicator(speed) }
+			}
+		)
 	private var currentSimulationContext: SimulationContext? = null
 
 	/**
@@ -141,6 +165,10 @@ class Frame : JFrame(PROGRAM_FULL_NAME) {
 		northContainer.add(simulationControlPanel)
 		contentPane.add(northContainer, BorderLayout.NORTH)
 
+		// Route speed changes from SimulationControlPanel through SimulationController so
+		// desiredSpeed stays in sync and is applied to the next simulation start.
+		simulationControlPanel.onSpeedChanged = { speed -> simulationController.setSpeed(speed) }
+
 		// South panel contains StatusBar (edit mode) and EventTimelinePanel (simulation mode)
 		statusBar.registerProducer(railwayNetGridCanvas)
 		southPanel.add(statusBar)
@@ -256,6 +284,8 @@ class Frame : JFrame(PROGRAM_FULL_NAME) {
 		stopSimulation() // Stop any running simulation before switching context
 		stopAnimationUpdates() // Cleanup existing timer
 
+		val previousSimulationContext = currentSimulationContext
+
 		when (context) {
 			is SimulationContext -> {
 				currentSimulationContext = context
@@ -291,6 +321,8 @@ class Frame : JFrame(PROGRAM_FULL_NAME) {
 		}
 
 		context.addPropertyChangeListener(statusBar)
+
+		previousSimulationContext?.close()
 	}
 
 	/**
@@ -397,6 +429,20 @@ class Frame : JFrame(PROGRAM_FULL_NAME) {
 			}
 	}
 
+	/**
+	 * Execute [action] on EDT.
+	 *
+	 * Runs immediately if already on EDT; otherwise schedules asynchronously via
+	 * [javax.swing.SwingUtilities.invokeLater] to avoid blocking monitor/background threads.
+	 */
+	private fun runOnEdt(action: () -> Unit) {
+		if (SwingUtilities.isEventDispatchThread()) {
+			action()
+		} else {
+			SwingUtilities.invokeLater(action)
+		}
+	}
+
 	/**
 	 * Handles window closing event.
 	 * Shows confirmation dialog if there are unsaved changes.

desktop-ui/src/main/kotlin/cz/vutbr/fit/interlockSim/gui/SimulationControlPanel.kt

@@ -86,6 +86,18 @@ class SimulationControlPanel : JPanel() {
 		}
 	}
 
+	/**
+	 * Optional callback invoked whenever the user changes the speed (slider or preset button).
+	 *
+	 * Wire this in [cz.vutbr.fit.interlockSim.gui.Frame] to route speed changes through
+	 * [SimulationController.setSpeed] so that [SimulationController.desiredSpeed] stays in sync
+	 * and is honoured on the next [SimulationController.start] call.
+	 *
+	 * Not called when the runner fires a [SimulationRunner.PROP_SPEED_MULTIPLIER] event (i.e.
+	 * when the UI is updated from the runner rather than by the user).
+	 */
+	var onSpeedChanged: ((Double) -> Unit)? = null
+
 	/** Flag to suppress recursive slider → runner → slider feedback loops. */
 	private var updatingFromRunner = false
 
@@ -110,6 +122,7 @@ class SimulationControlPanel : JPanel() {
 				val speed = sliderToSpeed(slider.value)
 				speedLabel.text = formatSpeedLabel(speed)
 				runner?.speedMultiplier = speed
+				onSpeedChanged?.invoke(speed)
 			}
 		}
 		sliderRow.add(slider)
@@ -141,10 +154,11 @@ class SimulationControlPanel : JPanel() {
 	private fun speedToSlider(speed: Double): Int =
 		Math.round(speed * SLIDER_SCALE).toInt().coerceIn(SLIDER_MIN, SLIDER_MAX)
 
-	/** Apply a preset speed: update runner, slider, and label. */
+	/** Apply a preset speed: update runner, slider, label, and notify controller. */
 	private fun applyPreset(speed: Double) {
 		syncUiToSpeed(speed)
 		runner?.speedMultiplier = speed
+		onSpeedChanged?.invoke(speed)
 	}
 
 	/**

desktop-ui/src/main/kotlin/cz/vutbr/fit/interlockSim/gui/SimulationController.kt

@@ -11,10 +11,8 @@
 package cz.vutbr.fit.interlockSim.gui
 
 import cz.vutbr.fit.interlockSim.context.SimulationContext
-import cz.vutbr.fit.interlockSim.gui.animation.ControlPanel
 import io.github.oshai.kotlinlogging.KotlinLogging
 import java.beans.PropertyChangeListener
-import javax.swing.SwingUtilities
 
 /**
  * Manages the simulation lifecycle on behalf of [Frame] (Issue #189).
@@ -27,30 +25,29 @@ import javax.swing.SwingUtilities
  * - Starting the runner synchronously (before the monitor thread) to prevent the
  *   race condition where [stop] is called before the monitor thread starts the runner
  * - Polling for completion on a daemon "SimulationMonitor" thread
- * - Enabling/disabling the Stop button in [ControlPanel] as the lifecycle changes
- * - Dispatching [onCompleted] back to EDT when the simulation finishes naturally
+ * - Reporting lifecycle and speed changes via callbacks
  *
  * ## Thread Safety
- * - [start] and [stop] are designed to be called from the same thread (typically EDT
- *   in production, but also from test threads in unit tests). They are NOT thread-safe
- *   for concurrent calls from different threads; external callers are responsible for
- *   serialization. [Frame] enforces EDT-only access via its own `require()` guards.
+ * - [start] and [stop] are designed to be called from the same thread. They are NOT
+ *   thread-safe for concurrent calls from different threads; external callers are
+ *   responsible for serialization.
  * - [runner] is `@Volatile` so the monitor thread reads a fresh value when [stop]
  *   nulls it.
- * - [onCompleted] is always dispatched to EDT via [SwingUtilities.invokeLater].
+ * - Callbacks are invoked on whichever thread performs the lifecycle change.
  *
- * @param controlPanel ControlPanel whose Stop button and status label are managed here.
- * @param toolBar Optional [ToolBar] to show/hide simulation controls on lifecycle changes.
- * @param statusBar Optional [StatusBar] to display speed indicator when speed != 1.0x.
- * @param onCompleted Callback invoked on EDT when the simulation finishes naturally.
- *   Defaults to a no-op if not provided.
+ * @param onStateChanged Callback for lifecycle state updates. Invoked on the same
+ *   thread that performs the state change (caller thread for [start]/[stop], monitor
+ *   thread for natural completion).
+ * @param onSpeedChanged Callback for speed indicator updates. Invoked on the thread
+ *   that emits the speed change; callers are responsible for EDT marshalling as needed.
+ * @param onCompleted Callback invoked when the simulation finishes naturally on the
+ *   monitor thread. Defaults to a no-op.
  * @since 2026-04-20 (extracted from Frame for testability)
  * @see Frame
  */
 internal class SimulationController(
-	private val controlPanel: ControlPanel,
-	private val toolBar: ToolBar? = null,
-	private val statusBar: StatusBar? = null,
+	private val onStateChanged: (SimulationStatus) -> Unit = {},
+	private val onSpeedChanged: (Double) -> Unit = {},
 	private val onCompleted: () -> Unit = {},
 ) {
 	/**
@@ -82,9 +79,9 @@ internal class SimulationController(
 	 * 2. Calls [SimulationRunner.start] **synchronously** (before the monitor thread) to
 	 *    eliminate the race condition where [stop] could be invoked before the monitor
 	 *    thread has a chance to start the runner.
-	 * 3. Updates [ControlPanel] status to "Running" and enables the Stop button.
+	 * 3. Emits [SimulationStatus.RUNNING] via [onStateChanged].
 	 * 4. Launches a daemon "SimulationMonitor" thread that polls [SimulationRunner.isRunning]
-	 *    and on completion dispatches [onCompleted] and resets the panel via EDT.
+	 *    and on completion emits [SimulationStatus.STOPPED] and invokes [onCompleted].
 	 *
 	 * @param context The simulation context to run.
 	 */
@@ -103,35 +100,31 @@ internal class SimulationController(
 		// thread. This ensures stopSimulation() always has a live thread to interrupt.
 		newRunner.start()
 
-		// Wire speed indicator: notify StatusBar whenever SimulationRunner speed changes.
+		// Wire speed callback for SimulationRunner speed changes.
 		// The listener is removed when the simulation stops (in stop() or monitor finally).
 		val listener = PropertyChangeListener { evt ->
 			val multiplier = evt.newValue as? Double
 			if (multiplier == null) {
 				logger.debug { "Ignoring unexpected ${SimulationRunner.PROP_SPEED_MULTIPLIER} value: ${evt.newValue}" }
 				return@PropertyChangeListener
 			}
-			statusBar?.updateSpeedIndicator(multiplier)
+			onSpeedChanged(multiplier)
 		}
 		speedListener = listener
 		newRunner.addPropertyChangeListener(SimulationRunner.PROP_SPEED_MULTIPLIER, listener)
+		onSpeedChanged(newRunner.speedMultiplier)
 
-		// Show simulation controls in ToolBar (EDT-safe: start() is called from EDT).
-		toolBar?.showSimulationControls()
-
-		controlPanel.updateStatus(ControlPanel.SimulationStatus.RUNNING)
-		controlPanel.setStopEnabled(true)
+		onStateChanged(SimulationStatus.RUNNING)
 
 		launchMonitorThread(newRunner)
 	}
 
 	/**
 	 * Launch a daemon "SimulationMonitor" thread that polls [newRunner] for completion
-	 * and dispatches panel reset and [onCompleted] to EDT when done.
+	 * and dispatches callback notifications when done.
 	 *
-	 * Guards against stale-monitor: the [SwingUtilities.invokeLater] callback checks
-	 * `runner === newRunner` before mutating state so that a stop+start cycle started
-	 * before the lambda fires cannot clobber the new run's panel state.
+	 * Guards against stale-monitor by checking `runner === newRunner` before mutating
+	 * state so that a stop+start cycle cannot clobber the new run's state.
 	 */
 	private fun launchMonitorThread(newRunner: SimulationRunner) {
 		val monitorThread =
@@ -144,20 +137,15 @@ internal class SimulationController(
 					} catch (e: InterruptedException) {
 						Thread.currentThread().interrupt()
 					} finally {
-						SwingUtilities.invokeLater {
-							// Guard against stale-monitor: if stop() + start(ctxB) ran on EDT
-							// before this callback fired, runner has been replaced with a new
-							// instance. Skip the reset to avoid clobbering the new run's panel
-							// state (and avoid firing onCompleted for the old run).
-							if (runner === newRunner) {
-								cleanupSpeedListener(newRunner)
-								runner = null
-								toolBar?.hideSimulationControls()
-								statusBar?.updateSpeedIndicator(SimulationRunner.DEFAULT_SPEED)
-								controlPanel.updateStatus(ControlPanel.SimulationStatus.STOPPED)
-								controlPanel.setStopEnabled(false)
-								onCompleted()
-							}
+						// Guard against stale-monitor: if stop() + start(ctxB) ran before
+						// this callback executes, runner has been replaced with a new instance.
+						// Skip reset to avoid clobbering the new run's state.
+						if (runner === newRunner) {
+							cleanupSpeedListener(newRunner)
+							runner = null
+							onSpeedChanged(SimulationRunner.DEFAULT_SPEED)
+							onStateChanged(SimulationStatus.STOPPED)
+							onCompleted()
 						}
 					}
 				},
@@ -168,7 +156,7 @@ internal class SimulationController(
 	}
 
 	/**
-	 * Stop a running simulation and reset the [ControlPanel].
+	 * Stop a running simulation and emit [SimulationStatus.STOPPED].
 	 *
 	 * Safe to call when no simulation is running (no-op in that case).
 	 */
@@ -177,10 +165,8 @@ internal class SimulationController(
 		cleanupSpeedListener(r)
 		r.stop()
 		runner = null
-		toolBar?.hideSimulationControls()
-		statusBar?.updateSpeedIndicator(SimulationRunner.DEFAULT_SPEED)
-		controlPanel.setStopEnabled(false)
-		controlPanel.updateStatus(ControlPanel.SimulationStatus.STOPPED)
+		onSpeedChanged(SimulationRunner.DEFAULT_SPEED)
+		onStateChanged(SimulationStatus.STOPPED)
 	}
 
 	/** Removes the speed [PropertyChangeListener] from [r] and clears the reference. */
@@ -215,4 +201,10 @@ internal class SimulationController(
 		/** Poll interval (ms) for the monitor thread to detect simulation completion. */
 		internal const val SIMULATION_POLL_INTERVAL_MS: Long = 500L
 	}
+
+	/** Simulation lifecycle states emitted via [onStateChanged]. */
+	enum class SimulationStatus {
+		RUNNING,
+		STOPPED,
+	}
 }

desktop-ui/src/main/kotlin/cz/vutbr/fit/interlockSim/gui/StatusBar.kt

@@ -86,10 +86,12 @@ class StatusBar :
 	}
 
 	override fun propertyChange(event: ContextChangeEvent) {
-		val newValue = event.newValue
-		when {
-			newValue is CharSequence -> text = newValue.toString()
-			newValue != null -> text = newValue.toString()
+		val newValue = event.newValue ?: return
+		val newText = newValue.toString()
+		if (SwingUtilities.isEventDispatchThread()) {
+			text = newText
+		} else {
+			SwingUtilities.invokeLater { text = newText }
 		}
 	}
 

desktop-ui/src/test/kotlin/cz/vutbr/fit/interlockSim/gui/FrameTest.kt

@@ -285,4 +285,23 @@ class FrameTest : AbstractFrameTestBase() {
 		simContext.close()
 		editContext.close()
 	}
+
+	@Test
+	@Timeout(value = 5, unit = TimeUnit.SECONDS)
+	@DisplayName("setContext closes previous SimulationContext when switching to a new one")
+	fun setContextClosesPreviousSimulationContext() {
+		val context1 = cz.vutbr.fit.interlockSim.testutil.createMockSimulationContext(
+			cz.vutbr.fit.interlockSim.testutil.TestFixtures.loadShuntingXml()
+		)
+		val context2 = cz.vutbr.fit.interlockSim.testutil.createMockSimulationContext(
+			cz.vutbr.fit.interlockSim.testutil.TestFixtures.loadShuntingXml()
+		)
+		runOnEDT {
+			frame.setContext(context1)
+			frame.setContext(context2)
+		}
+		assertThat(context1.closeCount).isEqualTo(1)
+		runOnEDT { frame.stopSimulation() }
+		context2.close()
+	}
 }