diff --git a/WebATM-integrated/tests/test_log_streamer.py b/WebATM-integrated/tests/test_log_streamer.py index dc837df..53b94d4 100644 --- a/WebATM-integrated/tests/test_log_streamer.py +++ b/WebATM-integrated/tests/test_log_streamer.py @@ -79,6 +79,83 @@ def test_history_is_bounded_to_max_history(): assert [item["line"] for item in streamer.history()] == ["l2", "l3", "l4"] +class ReentrantSocketIO(FakeSocketIO): + """Runs a newly scheduled flush task *during* an emit. + + FakeSocketIO drains tasks strictly one after another, which hides the real + threading-mode behaviour where a second flush task can emit while the first + is still working through its chunks. This stand-in models that by firing + ``on_first_emit`` mid-flush and giving any task it schedules a turn there + and then. + """ + + def __init__(self): + super().__init__() + self.on_first_emit = None + self._reentered = False + + def emit(self, event, payload): + super().emit(event, payload) + if self.on_first_emit and not self._reentered: + self._reentered = True + hook, self.on_first_emit = self.on_first_emit, None + hook() # a log line arrives mid-flush... + self.run_all() # ...and its flush task gets a turn + + +def test_a_line_arriving_mid_flush_does_not_overtake_the_batch_being_emitted(): + """Only one flush task may be live: a second one emitting concurrently + would interleave its newer lines among the older chunks, delivering the + stream out of order despite seq being assigned in order.""" + sio = ReentrantSocketIO() + streamer = LogStreamer(sio, batch_max=2) + + for i in range(5): + streamer.feed_line(f"l{i}") + sio.on_first_emit = lambda: streamer.feed_line("late") + sio.run_all() + + seqs = [item["seq"] for _, payload in sio.emitted for item in payload["lines"]] + assert seqs == sorted(seqs), f"lines delivered out of order: {seqs}" + assert _lines(sio) == ["l0", "l1", "l2", "l3", "l4", "late"] + + +def test_a_line_arriving_as_the_flusher_winds_down_is_still_delivered(): + """The scheduled flag is cleared under the same lock feed_line appends + under, so a line can never be left pending with no flush task coming.""" + sio = FakeSocketIO() + streamer = LogStreamer(sio) + + streamer.feed_line("first") + sio.run_all() + streamer.feed_line("second") + sio.run_all() + + assert _lines(sio) == ["first", "second"] + + +def test_stream_recovers_after_an_emit_raises(): + """A raising emit must not leave the flush flag stuck True -- feed_line only + schedules while it is False, so the stream would go silent for good.""" + sio = FakeSocketIO() + streamer = LogStreamer(sio) + + def boom(event, payload): + raise RuntimeError("socket write failed") + + sio.emit = boom + streamer.feed_line("during-outage") + with pytest.raises(RuntimeError): + sio.run_all() + + # The next line must schedule a fresh flush and get through. + sio.emit = lambda event, payload: sio.emitted.append((event, payload)) + streamer.feed_line("after-outage") + sio.run_all() + + assert _lines(sio) == ["after-outage"] + + def test_feed_line_recovers_after_a_failed_schedule(): """A failed start_background_task must not leave _flush_scheduled stuck True, which would silence the stream forever.""" diff --git a/WebATM-integrated/tests/test_process_manager.py b/WebATM-integrated/tests/test_process_manager.py index d531db6..c7b1782 100644 --- a/WebATM-integrated/tests/test_process_manager.py +++ b/WebATM-integrated/tests/test_process_manager.py @@ -18,6 +18,8 @@ import threading import time +import pytest +import webatm_integrated.process_manager as pm from webatm_integrated.process_manager import BlueSkyProcessManager @@ -147,6 +149,57 @@ def on_line(line: str) -> None: manager.kill() +def test_a_failing_signal_does_not_strand_the_state_at_stopping(monkeypatch): + """If killpg raises anything but ProcessLookupError, stop() must still leave + a usable state: a stranded "stopping" makes every later start() wait 15s and + then refuse, with no control surface able to clear it.""" + manager = BlueSkyProcessManager( + cmd=[sys.executable, "-c", "import time; time.sleep(60)"] + ) + try: + assert manager.start()["success"] is True + + def denied(pgid, sig): + raise PermissionError("operation not permitted") + + monkeypatch.setattr(pm.os, "killpg", denied) + with pytest.raises(PermissionError): + manager.stop() + + # The process really is still up, so that is what status must report -- + # and a retry must be able to proceed rather than hit the stop-wait. + assert manager.status()["status"] == "running" + monkeypatch.undo() + assert manager.stop()["success"] is True + assert manager.status()["running"] is False + finally: + manager.kill() + + +def test_kill_on_a_stopped_server_does_not_claim_it_killed_something(): + """The UI renders this message verbatim, so Kill on an already-stopped + server must say so rather than report a kill that never happened.""" + manager = BlueSkyProcessManager(cmd=[sys.executable, "-c", "pass"]) + + result = manager.kill() + + assert result["success"] is True + assert result["status"] == "stopped" + assert result["message"] == "BlueSky server is not running" + + +def test_kill_reports_a_kill_when_a_process_was_actually_running(): + manager = BlueSkyProcessManager( + cmd=[sys.executable, "-c", "import time; time.sleep(60)"] + ) + assert manager.start()["success"] is True + + result = manager.kill() + + assert result["success"] is True + assert result["message"] == "BlueSky server killed" + + def test_restart_propagates_stop_failure_instead_of_claiming_success(): """restart() must not report "restarted" while the old tree is still alive.""" manager = BlueSkyProcessManager(cmd=[sys.executable, "-c", "pass"]) diff --git a/WebATM-integrated/webatm_integrated/log_streamer.py b/WebATM-integrated/webatm_integrated/log_streamer.py index 08af5c4..4d5da70 100644 --- a/WebATM-integrated/webatm_integrated/log_streamer.py +++ b/WebATM-integrated/webatm_integrated/log_streamer.py @@ -66,15 +66,32 @@ def feed_line(self, line: str) -> None: raise def _flush_after_delay(self) -> None: - # Cooperative sleep via SocketIO so it matches the active async mode. - self._sio.sleep(self._batch_ms / 1000.0) - with self._lock: - batch = self._pending - self._pending = [] - self._flush_scheduled = False - for start in range(0, len(batch), self._batch_max): - chunk = batch[start : start + self._batch_max] - self._sio.emit(EVENT, {"lines": chunk}) + # Drain until empty rather than flushing once, so only one flush task is + # ever live: clearing the flag before emitting would let a second task + # emit concurrently, interleaving newer lines among the older chunks. + try: + while True: + # Cooperative sleep via SocketIO to match the active async mode. + self._sio.sleep(self._batch_ms / 1000.0) + with self._lock: + batch = self._pending + self._pending = [] + if not batch: + # Cleared under the lock feed_line appends under, so a + # line arriving now always gets a flush task scheduled + # for it -- it can never be stranded unflushed. + self._flush_scheduled = False + return + for start in range(0, len(batch), self._batch_max): + chunk = batch[start : start + self._batch_max] + self._sio.emit(EVENT, {"lines": chunk}) + except BaseException: + # A raising emit/sleep must not leave the flag stuck True: feed_line + # only schedules while it is False, so the stream would go silent + # for good. Clearing it lets the next line start a fresh flush. + with self._lock: + self._flush_scheduled = False + raise def history(self) -> list[dict]: """Return a snapshot of buffered lines for late-joining clients. diff --git a/WebATM-integrated/webatm_integrated/process_manager.py b/WebATM-integrated/webatm_integrated/process_manager.py index aa58298..8735988 100644 --- a/WebATM-integrated/webatm_integrated/process_manager.py +++ b/WebATM-integrated/webatm_integrated/process_manager.py @@ -32,6 +32,14 @@ def _default_spawn(target: Callable, *args) -> threading.Thread: return thread +def _signal_group(pgid: int, sig: int) -> None: + """Signal a process group, tolerating one that has already exited.""" + try: + os.killpg(pgid, sig) + except ProcessLookupError: + pass + + class BlueSkyProcessManager: """Thread-safe lifecycle manager for the ``bluesky --headless`` process tree. @@ -189,59 +197,77 @@ def stop(self, sig: int = signal.SIGTERM, escalate_after: float = 5.0) -> dict: "message": "BlueSky server is not running", } self._state = "stopping" - try: - pgid = os.getpgid(proc.pid) - except ProcessLookupError: - self._state = "stopped" - return { - "success": True, - "status": "stopped", - "message": "BlueSky server already exited", - } - # Signal the whole group (server + all node children) outside the lock. try: - os.killpg(pgid, sig) + return self._terminate(proc, sig, escalate_after) + except BaseException: + # Never leave the state stranded at "stopping": start() waits out a + # shutdown in that state, so it would block for 15s and then refuse + # to start, with no control surface able to clear it. + self._settle(proc) + raise + + def _terminate( + self, proc: subprocess.Popen, sig: int, escalate_after: float + ) -> dict: + """Signal ``proc``'s group, escalating to SIGKILL, and settle the state.""" + try: + pgid = os.getpgid(proc.pid) except ProcessLookupError: - pass + self._settle(proc) + return { + "success": True, + "status": "stopped", + "message": "BlueSky server already exited", + } + # Signal the whole group (server + all node children) outside the lock. + _signal_group(pgid, sig) try: proc.wait(timeout=escalate_after) except subprocess.TimeoutExpired: - try: - os.killpg(pgid, signal.SIGKILL) - except ProcessLookupError: - pass + _signal_group(pgid, signal.SIGKILL) try: proc.wait(timeout=5) except subprocess.TimeoutExpired: logger.error("BlueSky process group %s survived SIGKILL", pgid) - with self._lock: - if self._proc is proc: - self._state = "running" + self._settle(proc) return { "success": False, "status": "error", "message": "BlueSky server did not exit after SIGKILL", } - with self._lock: - if self._proc is proc: - self._state = "stopped" + self._settle(proc) return { "success": True, "status": "stopped", "message": "BlueSky server stopped", } + def _settle(self, proc: subprocess.Popen) -> None: + """Re-derive the state from whether ``proc`` is actually still alive. + + Skipped if a restart already installed a different process, whose own + state must not be clobbered by this one's teardown. + """ + with self._lock: + if self._proc is proc: + self._state = "running" if proc.poll() is None else "stopped" + def kill(self) -> dict: """Force-kill the whole process group immediately (no graceful wait). Returns: dict: Result with ``success``, ``status`` and ``message``. """ + with self._lock: + proc = self._proc + was_running = proc is not None and proc.poll() is None result = self.stop(sig=signal.SIGKILL, escalate_after=2.0) - if result.get("success") and result.get("status") == "stopped": + # Only claim a kill if there was actually a live process to kill -- + # otherwise keep stop()'s "is not running", which the UI shows verbatim. + if was_running and result.get("success") and result.get("status") == "stopped": result["message"] = "BlueSky server killed" return result diff --git a/WebATM/static/css/style.css b/WebATM/static/css/style.css index c8bf6d5..d5da0d0 100644 --- a/WebATM/static/css/style.css +++ b/WebATM/static/css/style.css @@ -957,20 +957,6 @@ html.wa-open-threeD-controls #threeD-controls { min-width: 80px; } -/* Aircraft model dropdown - ensure proper right alignment */ -#aircraft-model-container { - display: flex !important; - justify-content: space-between !important; - align-items: center !important; -} - -/* Aircraft 3D scale input - ensure proper right alignment */ -#aircraft-3d-scale-container { - display: flex !important; - justify-content: space-between !important; - align-items: center !important; -} - /* Aircraft appearance controls group */ .aircraft-appearance-controls { margin-top: 8px; diff --git a/WebATM/templates/index.html b/WebATM/templates/index.html index c718a5e..6d239b7 100644 --- a/WebATM/templates/index.html +++ b/WebATM/templates/index.html @@ -1229,34 +1229,6 @@

Upload a Plugin

- - - diff --git a/frontend/package-lock.json b/frontend/package-lock.json index 1c79917..963c800 100644 --- a/frontend/package-lock.json +++ b/frontend/package-lock.json @@ -559,9 +559,6 @@ "arm64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -579,9 +576,6 @@ "arm64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -599,9 +593,6 @@ "ppc64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -619,9 +610,6 @@ "s390x" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -639,9 +627,6 @@ "x64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -659,9 +644,6 @@ "x64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -2849,9 +2831,6 @@ "arm64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MPL-2.0", "optional": true, "os": [ @@ -2873,9 +2852,6 @@ "arm64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MPL-2.0", "optional": true, "os": [ @@ -2897,9 +2873,6 @@ "x64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MPL-2.0", "optional": true, "os": [ @@ -2921,9 +2894,6 @@ "x64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MPL-2.0", "optional": true, "os": [ diff --git a/frontend/src/core/App.ts b/frontend/src/core/App.ts index 6eb8dda..0e68621 100644 --- a/frontend/src/core/App.ts +++ b/frontend/src/core/App.ts @@ -488,11 +488,6 @@ export class App { return this.aircraftInteractionManager; } - public setActiveNode(nodeId: string): void { - this.socketManager.setActiveNode(nodeId); - this.stateManager.setActiveNode(nodeId); - } - public getState(): AppState { return this.stateManager.getState(); } diff --git a/frontend/src/core/SocketManager.ts b/frontend/src/core/SocketManager.ts index b7931e3..4ce3292 100644 --- a/frontend/src/core/SocketManager.ts +++ b/frontend/src/core/SocketManager.ts @@ -351,22 +351,6 @@ export class SocketManager { return false; } - setActiveNode(nodeId: string): void { - if (this.isConnected() && this.socket) { - this.socket.emit('set_active_node', { node_id: nodeId }); - } else { - logger.warn('SocketManager', 'Cannot set active node: not connected to WebATM'); - } - } - - requestNodes(): void { - if (this.isConnected() && this.socket) { - this.socket.emit('get_nodes'); - } else { - logger.warn('SocketManager', 'Cannot request nodes: not connected to WebATM'); - } - } - destroy(): void { if (this.socket) { this.socket.removeAllListeners(); diff --git a/frontend/src/ui/SettingsModal.test.ts b/frontend/src/ui/SettingsModal.test.ts index 4bc19fb..824799d 100644 --- a/frontend/src/ui/SettingsModal.test.ts +++ b/frontend/src/ui/SettingsModal.test.ts @@ -62,7 +62,12 @@ function buildSettingsDom(): void { - + @@ -207,6 +212,34 @@ describe('SettingsModal connect flow', () => { expect(connect.disabled).toBe(true); }); + it('restores the API key of a persisted MapTiler style on open', async () => { + const maptilerUrl = 'https://api.maptiler.com/maps/streets/style.json?key=ABC123'; + // StorageManager namespaces with 'webatm-' and JSON-encodes values. + localStorage.setItem('webatm-webatm-map-style', JSON.stringify(maptilerUrl)); + + beforeOpenCallback?.('beforeOpen', 'settings-modal'); + await flushAsync(); + + const select = document.getElementById('map-style-select-modal') as HTMLSelectElement; + const keyInput = document.getElementById('maptiler-api-key-input') as HTMLInputElement; + expect(select.value).toBe('https://api.maptiler.com/maps/streets/style.json?key='); + expect(keyInput.value).toBe('ABC123'); + }); + + it('does not clobber a key the user already typed', async () => { + localStorage.setItem( + 'webatm-webatm-map-style', + JSON.stringify('https://api.maptiler.com/maps/streets/style.json?key=OLD') + ); + const keyInput = document.getElementById('maptiler-api-key-input') as HTMLInputElement; + keyInput.value = 'TYPED'; + + beforeOpenCallback?.('beforeOpen', 'settings-modal'); + await flushAsync(); + + expect(keyInput.value).toBe('TYPED'); + }); + it('a failed connect re-enables Connect when the server is still up', async () => { const input = document.getElementById('server-ip-input') as HTMLInputElement; const connect = document.getElementById('connect-server') as HTMLButtonElement; diff --git a/frontend/src/ui/SettingsModal.ts b/frontend/src/ui/SettingsModal.ts index 199f6e2..ac5516e 100644 --- a/frontend/src/ui/SettingsModal.ts +++ b/frontend/src/ui/SettingsModal.ts @@ -359,12 +359,18 @@ export class SettingsModal { // Direct match, or a MapTiler style where the saved value has // ?key=ABC123 and the option value ends with a bare ?key= - if ( - option.value === savedStyle || - (option.value.endsWith('?key=') && savedStyle.startsWith(option.value)) - ) { + const isKeyedMatch = + option.value.endsWith('?key=') && savedStyle.startsWith(option.value); + if (option.value === savedStyle || isKeyedMatch) { this.elements.mapStyleSelect.selectedIndex = i; matchFound = true; + + // Restore the key embedded in the saved URL so Apply works + // without re-entering it; keep any key the user has typed. + const keyInput = this.elements.mapTilerApiKeyInput; + if (isKeyedMatch && keyInput && !keyInput.value.trim()) { + keyInput.value = savedStyle.slice(option.value.length); + } break; } } diff --git a/frontend/src/ui/map/MapStyleManager.selector.test.ts b/frontend/src/ui/map/MapStyleManager.selector.test.ts new file mode 100644 index 0000000..def020f --- /dev/null +++ b/frontend/src/ui/map/MapStyleManager.selector.test.ts @@ -0,0 +1,134 @@ +// @vitest-environment happy-dom +/** + * Characterizes the settings-modal style selector wiring in MapStyleManager: + * which controls are visible for each kind of selection, and the save/delete + * flow for user-saved custom styles. + * + * Regressions covered: + * - the "Apply Map Style" button must hide when the placeholder is selected + * (it used to stay visible after deleting a saved style) + * - saving a style whose URL duplicates a predefined option must select the + * saved optgroup entry, not the predefined option (which hid Delete) + */ +import { describe, it, expect, beforeEach, vi } from 'vitest'; + +vi.mock('../../utils/Logger', () => ({ + logger: { + error: vi.fn(), + warn: vi.fn(), + info: vi.fn(), + debug: vi.fn(), + verbose: vi.fn() + } +})); + +import { MapStyleManager } from './MapStyleManager'; +import { loadSavedStyles } from './customStyles'; + +const PREDEFINED_URL = '/static/map/offline-style-light.json'; + +function buildSelectorDom(): void { + document.body.innerHTML = ` + + + + +
+ `; +} + +const el = (id: string): T => + document.getElementById(id) as T; +const visible = (id: string): boolean => el(id).style.display !== 'none'; + +function selectValue(value: string): void { + const select = el('map-style-select-modal'); + select.value = value; + select.dispatchEvent(new Event('change')); +} + +function saveStyle(name: string, url: string): void { + el('custom-style-url-modal').value = url; + el('custom-style-name-modal').value = name; + el('save-custom-style-modal').click(); +} + +describe('MapStyleManager style selector', () => { + beforeEach(() => { + localStorage.clear(); + buildSelectorDom(); + vi.stubGlobal('alert', vi.fn()); + vi.stubGlobal('confirm', vi.fn(() => true)); + // Map access is irrelevant here: changeStyle() bails out on a null + // map after the persistence step, which is all these tests need. + new MapStyleManager(() => null).setupStyleSelector(); + }); + + it('shows Apply and hides the custom control for a predefined style', () => { + selectValue('https://tiles.example/positron'); + expect(visible('apply-map-style-btn')).toBe(true); + expect(visible('custom-style-control-modal')).toBe(false); + }); + + it('shows the custom control and hides Apply for "custom"', () => { + selectValue('custom'); + expect(visible('custom-style-control-modal')).toBe(true); + expect(visible('apply-map-style-btn')).toBe(false); + }); + + it('hides both Apply and the custom control for the placeholder', () => { + selectValue('https://tiles.example/positron'); + selectValue(''); + expect(visible('apply-map-style-btn')).toBe(false); + expect(visible('custom-style-control-modal')).toBe(false); + }); + + it('saving a style adds it to the Saved Styles group, selects it, and offers Delete', () => { + selectValue('custom'); + saveStyle('My Style', 'https://my/style.json'); + + const select = el('map-style-select-modal'); + const selected = select.selectedOptions[0]; + expect(selected.textContent).toBe('My Style'); + expect(selected.dataset.savedStyle).toBe('true'); + expect(selected.closest('optgroup')?.label).toBe('Saved Styles'); + expect(visible('delete-saved-style-btn')).toBe(true); + expect(loadSavedStyles()).toEqual([ + { name: 'My Style', url: 'https://my/style.json' } + ]); + }); + + it('saving a URL that duplicates a predefined option still selects the saved entry', () => { + selectValue('custom'); + saveStyle('My Light', PREDEFINED_URL); + + const selected = el('map-style-select-modal').selectedOptions[0]; + expect(selected.textContent).toBe('My Light'); + expect(selected.dataset.savedStyle).toBe('true'); + expect(visible('delete-saved-style-btn')).toBe(true); + }); + + it('deleting a saved style resets to the placeholder with Apply and Delete hidden', () => { + selectValue('custom'); + saveStyle('My Style', 'https://my/style.json'); + + el('delete-saved-style-btn').click(); + + const select = el('map-style-select-modal'); + expect(select.value).toBe(''); + expect(visible('apply-map-style-btn')).toBe(false); + expect(visible('delete-saved-style-btn')).toBe(false); + expect(loadSavedStyles()).toEqual([]); + expect(select.querySelector('#saved-custom-styles-group')).toBeNull(); + }); +}); diff --git a/frontend/src/ui/map/MapStyleManager.ts b/frontend/src/ui/map/MapStyleManager.ts index 296bd28..3381b1b 100644 --- a/frontend/src/ui/map/MapStyleManager.ts +++ b/frontend/src/ui/map/MapStyleManager.ts @@ -41,18 +41,14 @@ export class MapStyleManager { private currentStyle: string = ''; // How long the first-load reachability probe waits for the remote style - // document before declaring it unreachable. Generous enough that a slow - // but working connection never trips it (the style document is a few KB), - // short enough that an air-gapped first load recovers in seconds instead - // of hanging on the browser's connect timeout (which can be minutes). + // document (a few KB) before declaring it unreachable — generous for a + // slow link, but far shorter than the browser's connect timeout. private readonly FIRST_LOAD_PROBE_TIMEOUT_MS = 5000; constructor( private readonly getMap: () => Map | null, - // Invoked synchronously right before every map.setStyle() call, from - // every trigger (user selection, first-load probe, network-error - // fallback) - lets other renderers tear down layers they attached to - // the outgoing style's sources before MapLibre's diff runs. + // Invoked synchronously right before every map.setStyle() call, so + // other renderers can tear down layers on the outgoing style first. private readonly onBeforeStyleChange?: () => void ) {} @@ -115,19 +111,13 @@ export class MapStyleManager { * Make the first-load offline fallback deterministic. Call once right * after the map is constructed, when the initial style may be remote. * - * The 'error'-event fallback in handleMapError only helps when the failed - * style fetch actually *rejects*. On an air-gapped network the request to - * the basemap CDN often just hangs (dropped packets, DNS blackhole, proxy - * sink) — no error event ever fires, the map never gets a style, and the - * user stares at a blank basemap until the OS connect timeout (minutes) - * finally rejects the fetch. Only then does the fallback fire and persist - * the offline style, which is why a later reload "fixed" it. - * - * So probe the same style document the map is fetching, with our own - * timeout. If the probe fails (network error, timeout, or HTTP error) - * while the map still has no loaded style, swap to the bundled offline - * basemap immediately. If the map's style loads first — or the user picks - * a different style meanwhile — the probe result is ignored. + * The 'error'-event fallback in handleMapError only fires if the style + * fetch rejects; on an air-gapped network it often just hangs for + * minutes, leaving a blank basemap. So probe the same style document + * with our own timeout, and if the probe fails while the map still has + * no loaded style, swap to the bundled offline basemap immediately. If + * the style loads first — or the user picks another style meanwhile — + * the probe result is ignored. */ public armFirstLoadFallback(): void { const map = this.getMap(); @@ -212,25 +202,14 @@ export class MapStyleManager { /** * Decide whether a MapLibre error warrants swapping to the offline style. + * Only once, and only when the current style is a remote URL — a local + * style failing usually means a config mistake, not missing internet. * - * MapLibre surfaces a few shapes for network failures: AJAXError objects - * with a `status` field (0 when the browser couldn't reach the host at - * all), and generic `TypeError: Failed to fetch` for cross-origin / DNS - * problems. We only fall back once, and only if the current style is a - * remote URL — local styles failing usually mean a config mistake, not - * missing internet. - * - * Crucially we do NOT fall back on individual *tile* fetch failures (errors - * that carry a `tile`). Those are common and transient — a single dropped - * or CORS-blocked vector tile while panning should not swap the entire - * basemap to offline. Swapping the style mid-session reloads every layer: - * the basemap visibly flickers, and (with the 3D overlay on) rebuilding the - * Three.js custom layer disrupts the viewport and snaps the camera back to - * the default view. The map degrades gracefully on a missing tile (the - * tile is simply blank until it succeeds), so a tile error is never reason - * enough to nuke the user's chosen basemap. A genuinely-offline boot still - * triggers the fallback because the *style document* fetch fails, and that - * error carries no `tile`. + * Individual *tile* failures (errors carrying `tile`) never trigger the + * fallback: they are common and transient, and swapping the whole basemap + * mid-session reloads every layer and disrupts the 3D overlay's camera. A + * genuinely-offline boot still falls back, because there the *style + * document* fetch fails and that error carries no `tile`. */ private shouldFallBackToOffline(e: MapErrorEvent): boolean { if (this.hasFallenBackToOffline) return false; @@ -289,35 +268,18 @@ export class MapStyleManager { deleteSavedStyleBtn.style.display = isSaved ? 'block' : 'none'; }; - // Handle style select change - only toggle custom input visibility - styleSelect.addEventListener('change', (e) => { - const target = e.target as HTMLSelectElement; - - if (target.value === 'custom') { - // Show custom style input - if (customStyleControl) { - customStyleControl.style.display = 'block'; - } - // Hide apply button for predefined styles - if (applyMapStyleBtn) { - applyMapStyleBtn.style.display = 'none'; - } - } else if (target.value === '') { - // User selected "Select a map style..." placeholder - if (customStyleControl) { - customStyleControl.style.display = 'none'; - } - } else { - // User selected a predefined style - just hide custom input - if (customStyleControl) { - customStyleControl.style.display = 'none'; - } - // Show apply button for predefined styles - if (applyMapStyleBtn) { - applyMapStyleBtn.style.display = 'block'; - } + // Match the controls to the selection: "custom" shows the custom-URL + // input, the placeholder shows neither, and any concrete style shows + // the Apply button. + styleSelect.addEventListener('change', () => { + const value = styleSelect.value; + if (customStyleControl) { + customStyleControl.style.display = value === 'custom' ? 'block' : 'none'; + } + if (applyMapStyleBtn) { + applyMapStyleBtn.style.display = + value === 'custom' || value === '' ? 'none' : 'block'; } - updateDeleteButton(); }); @@ -385,7 +347,7 @@ export class MapStyleManager { this.renderSavedStyles(styleSelect); // Select and apply the newly-saved style; clear the name field. - styleSelect.value = url; + this.selectSavedStyle(styleSelect, url); if (customStyleNameInput) customStyleNameInput.value = ''; this.changeStyle(url); styleSelect.dispatchEvent(new Event('change')); @@ -439,6 +401,24 @@ export class MapStyleManager { logger.debug('MapStyleManager', 'Map style selector initialized'); } + /** + * Select the just-saved option inside the "Saved Styles" optgroup. A plain + * `select.value = url` would land on the first option with that value — + * which, when the URL duplicates a predefined option's, is the predefined + * one, leaving the saved entry unselectable and hiding its Delete button. + */ + private selectSavedStyle(select: HTMLSelectElement, url: string): void { + const group = select.querySelector(`#${this.SAVED_GROUP_ID}`); + const option = group + ? Array.from(group.querySelectorAll('option')).find(o => o.value === url) + : undefined; + if (option) { + option.selected = true; + } else { + select.value = url; + } + } + /** * (Re)build the "Saved Styles" optgroup in the style dropdown from the * persisted custom-style list. The group is inserted just above the diff --git a/frontend/src/ui/map/shapes/ShapeDrawingManager.test.ts b/frontend/src/ui/map/shapes/ShapeDrawingManager.test.ts index a2700e4..9dc7842 100644 --- a/frontend/src/ui/map/shapes/ShapeDrawingManager.test.ts +++ b/frontend/src/ui/map/shapes/ShapeDrawingManager.test.ts @@ -173,6 +173,25 @@ describe('ShapeDrawingManager create validation and finish', () => { expect(manager.isDrawing()).toBe(false); }); + it('rejects a non-numeric altitude instead of sending NaN to BlueSky', () => { + fillModal('TMA1'); + (document.getElementById('polygon-top-input') as HTMLInputElement).value = 'abc'; + (document.getElementById('polygon-bottom-input') as HTMLInputElement).value = '2000'; + clickCreate(); + + expect(alertMock).toHaveBeenCalledWith('Altitudes must be numbers (in feet)'); + expect(manager.isDrawing()).toBe(false); + }); + + it('rejects a half-filled altitude pair instead of silently dropping it', () => { + fillModal('TMA2'); + (document.getElementById('polygon-top-input') as HTMLInputElement).value = '10000'; + clickCreate(); + + expect(alertMock).toHaveBeenCalledWith('Enter both top and bottom altitudes, or leave both empty'); + expect(manager.isDrawing()).toBe(false); + }); + it('draws and sends exactly one command on finish', async () => { fillModal('AREA1'); clickCreate(); diff --git a/frontend/src/ui/map/shapes/ShapeDrawingManager.ts b/frontend/src/ui/map/shapes/ShapeDrawingManager.ts index 4a39ad0..73f0b1f 100644 --- a/frontend/src/ui/map/shapes/ShapeDrawingManager.ts +++ b/frontend/src/ui/map/shapes/ShapeDrawingManager.ts @@ -13,7 +13,7 @@ import { setLayerVisibility, updateSourceFeatures } from '../../../utils/maplibre'; -import { buildShapeCommand, ShapeType } from './shapeCommand'; +import { buildShapeCommand, parseAltitudeInputs, ShapeType } from './shapeCommand'; import { boxCornerPoints, circleRingPoints, distanceNm } from './shapeGeometry'; /** Modal title per shape type. */ @@ -151,33 +151,34 @@ export class ShapeDrawingManager extends BaseDrawingManager { return; } - this.currentShapeName = name; - this.currentShapeType = shapeType; - // Altitudes apply to everything but lines; on an area both filled // makes a POLYALT, on a box/circle they become the trailing // [top,bottom] arguments. + let topAltitude: number | null = null; + let bottomAltitude: number | null = null; if (shapeType !== 'line') { - const topValue = topInput?.value; - const bottomValue = bottomInput?.value; - - if (topValue && bottomValue) { - this.topAltitude = parseFloat(topValue); - this.bottomAltitude = parseFloat(bottomValue); - - if (this.topAltitude <= this.bottomAltitude) { - alert('Top altitude must be greater than bottom altitude'); - return; - } - } else { - this.topAltitude = null; - this.bottomAltitude = null; + // A number input showing unparseable text (e.g. "1e999") reads + // back as value '' - catch that as bad input, not as "empty". + if (topInput?.validity?.badInput || bottomInput?.validity?.badInput) { + alert('Altitudes must be numbers (in feet)'); + topInput?.focus(); + return; } - } else { - this.topAltitude = null; - this.bottomAltitude = null; + const parsed = parseAltitudeInputs(topInput?.value ?? '', bottomInput?.value ?? ''); + if (!parsed.ok) { + alert(parsed.error); + topInput?.focus(); + return; + } + topAltitude = parsed.top; + bottomAltitude = parsed.bottom; } + this.currentShapeName = name; + this.currentShapeType = shapeType; + this.topAltitude = topAltitude; + this.bottomAltitude = bottomAltitude; + modalManager.close('polygon-name-modal'); this.startDrawing(); } @@ -219,24 +220,15 @@ export class ShapeDrawingManager extends BaseDrawingManager { logger.info('ShapeDrawingManager', 'Stopped drawing'); } - /** - * Set up temporary drawing layers when drawing starts. - */ protected onDrawingEnabled(): void { this.setupTemporaryDrawingLayers(); } - /** - * Clear and remove temporary drawing layers when drawing stops. - */ protected onDrawingDisabled(): void { this.clearTemporaryDrawing(); this.removeTemporaryDrawingLayers(); } - /** - * Handle a placed point - update banner and preview. - */ protected onPointAdded(point: DrawingPoint): void { this.drawingPoints.push(point); @@ -258,9 +250,6 @@ export class ShapeDrawingManager extends BaseDrawingManager { return this.currentShapeType === 'box' || this.currentShapeType === 'circle'; } - /** - * Handle mouse move - update cursor preview - */ protected onCursorMove(point: DrawingPoint): void { if (this.drawingPoints.length === 0) return; this.updateCursorPreview(point); @@ -334,9 +323,6 @@ export class ShapeDrawingManager extends BaseDrawingManager { this.stopDrawing(); } - /** - * Generate the BlueSky command string for the current shape. - */ private generateCommand(): string { return buildShapeCommand({ name: this.currentShapeName ?? '', diff --git a/frontend/src/ui/map/shapes/shapeCommand.test.ts b/frontend/src/ui/map/shapes/shapeCommand.test.ts index 2de2f17..4d43041 100644 --- a/frontend/src/ui/map/shapes/shapeCommand.test.ts +++ b/frontend/src/ui/map/shapes/shapeCommand.test.ts @@ -1,5 +1,41 @@ import { describe, it, expect } from 'vitest'; -import { buildShapeCommand } from './shapeCommand'; +import { buildShapeCommand, parseAltitudeInputs } from './shapeCommand'; + +describe('parseAltitudeInputs', () => { + it('accepts both fields empty (no vertical extent)', () => { + expect(parseAltitudeInputs('', '')).toEqual({ ok: true, top: null, bottom: null }); + expect(parseAltitudeInputs(' ', ' ')).toEqual({ ok: true, top: null, bottom: null }); + }); + + it('accepts a valid top/bottom pair, trimming whitespace', () => { + expect(parseAltitudeInputs(' 10000 ', '2000')).toEqual({ ok: true, top: 10000, bottom: 2000 }); + }); + + it('rejects a half-filled pair instead of silently dropping it', () => { + expect(parseAltitudeInputs('10000', '')).toEqual({ + ok: false, + error: 'Enter both top and bottom altitudes, or leave both empty' + }); + expect(parseAltitudeInputs('', '2000')).toMatchObject({ ok: false }); + }); + + it('rejects non-numeric values instead of passing NaN through', () => { + expect(parseAltitudeInputs('abc', '2000')).toEqual({ + ok: false, + error: 'Altitudes must be numbers (in feet)' + }); + expect(parseAltitudeInputs('10000', '20oo')).toMatchObject({ ok: false }); + expect(parseAltitudeInputs('Infinity', '2000')).toMatchObject({ ok: false }); + }); + + it('rejects top at or below bottom', () => { + expect(parseAltitudeInputs('2000', '10000')).toEqual({ + ok: false, + error: 'Top altitude must be greater than bottom altitude' + }); + expect(parseAltitudeInputs('2000', '2000')).toMatchObject({ ok: false }); + }); +}); describe('buildShapeCommand', () => { const points = [ diff --git a/frontend/src/ui/map/shapes/shapeCommand.ts b/frontend/src/ui/map/shapes/shapeCommand.ts index e27ca96..8f371e2 100644 --- a/frontend/src/ui/map/shapes/shapeCommand.ts +++ b/frontend/src/ui/map/shapes/shapeCommand.ts @@ -14,6 +14,38 @@ export interface ShapePoint { /** The shape kinds offered by the draw modal. */ export type ShapeType = 'area' | 'line' | 'circle' | 'box'; +export type AltitudeParseResult = + | { ok: true; top: number | null; bottom: number | null } + | { ok: false; error: string }; + +/** + * Parse the draw modal's optional top/bottom altitude inputs. Both empty + * means no vertical extent; anything else needs two finite numbers with top + * above bottom, so a non-numeric value can't leak into the generated command + * as NaN and a half-filled pair isn't silently dropped. + */ +export function parseAltitudeInputs(topRaw: string, bottomRaw: string): AltitudeParseResult { + const top = topRaw.trim(); + const bottom = bottomRaw.trim(); + + if (!top && !bottom) { + return { ok: true, top: null, bottom: null }; + } + if (!top || !bottom) { + return { ok: false, error: 'Enter both top and bottom altitudes, or leave both empty' }; + } + + const topNum = Number(top); + const bottomNum = Number(bottom); + if (!Number.isFinite(topNum) || !Number.isFinite(bottomNum)) { + return { ok: false, error: 'Altitudes must be numbers (in feet)' }; + } + if (topNum <= bottomNum) { + return { ok: false, error: 'Top altitude must be greater than bottom altitude' }; + } + return { ok: true, top: topNum, bottom: bottomNum }; +} + export interface ShapeCommandSpec { name: string; type: ShapeType; diff --git a/frontend/src/ui/panels/left/DisplayOptionsPanel.test.ts b/frontend/src/ui/panels/left/DisplayOptionsPanel.test.ts index b305cec..5c2fcad 100644 --- a/frontend/src/ui/panels/left/DisplayOptionsPanel.test.ts +++ b/frontend/src/ui/panels/left/DisplayOptionsPanel.test.ts @@ -3,6 +3,8 @@ import { describe, it, expect, beforeEach, vi } from 'vitest'; import { DisplayOptionsPanel } from './DisplayOptionsPanel'; import { StateManager } from '../../../core/StateManager'; import { storage } from '../../../utils/StorageManager'; +import { DisplayOptions } from '../../../data/types'; +import type { App } from '../../../core/App'; // loadAvailableAircraftModels fetches the model catalog during setup; // let it fail fast and resolve to []. @@ -38,11 +40,27 @@ function buildDom(): void { + +
+ + +
`; } +/** Minimal App stand-in exposing a MapOverlay with the given toggle result. */ +function fakeApp(updateDisplayOptions: (options: Partial) => Promise): App { + return { + getMapOverlay: () => ({ updateDisplayOptions }), + } as unknown as App; +} + function createPanel(): { panel: DisplayOptionsPanel; stateManager: StateManager } { const panel = new DisplayOptionsPanel(); panel.init(); @@ -216,4 +234,78 @@ describe('DisplayOptionsPanel', () => { expect(panel.setBooleanOption('aircraftIconColor')).toBe(null); }); }); + + describe('3D overlay toggle', () => { + function containerVisible(id: string): boolean { + return (document.getElementById(id) as HTMLElement).style.display !== 'none'; + } + + function toggleOverlay(checked: boolean): void { + const box = checkbox('show-3d-overlay'); + box.checked = checked; + box.dispatchEvent(new Event('change')); + } + + it('enabling the overlay reveals the 3D scale and model controls', async () => { + const { stateManager } = createPanel(); + // Regression: nothing drove these containers' visibility (a CSS + // !important rule kept them shown even with the overlay off), so + // the controls ignored the overlay state entirely. + expect(containerVisible('aircraft-3d-scale-container')).toBe(false); + expect(containerVisible('aircraft-model-container')).toBe(false); + + toggleOverlay(true); + + await vi.waitFor(() => expect(containerVisible('aircraft-3d-scale-container')).toBe(true)); + expect(containerVisible('aircraft-model-container')).toBe(true); + expect(storage.get('show-3d-overlay')).toBe(true); + expect(stateManager.getDisplayOptions().show3DOverlay).toBe(true); + }); + + it('disabling the overlay hides the 3D controls again', async () => { + createPanel(); + toggleOverlay(true); + await vi.waitFor(() => expect(containerVisible('aircraft-3d-scale-container')).toBe(true)); + + toggleOverlay(false); + + await vi.waitFor(() => expect(containerVisible('aircraft-3d-scale-container')).toBe(false)); + expect(containerVisible('aircraft-model-container')).toBe(false); + }); + + it('a stored overlay=true reveals the 3D controls on load', () => { + storage.set('show-3d-overlay', true); + + const { stateManager } = createPanel(); + + expect(stateManager.getDisplayOptions().show3DOverlay).toBe(true); + expect(containerVisible('aircraft-3d-scale-container')).toBe(true); + expect(containerVisible('aircraft-model-container')).toBe(true); + }); + + it('passes the new overlay value to the MapOverlay', async () => { + const update = vi.fn().mockResolvedValue(undefined); + const { panel } = createPanel(); + panel.setApp(fakeApp(update)); + + toggleOverlay(true); + + await vi.waitFor(() => expect(update).toHaveBeenCalledWith({ show3DOverlay: true })); + }); + + it('a failed overlay toggle rolls back checkbox, storage, state and controls', async () => { + const { panel, stateManager } = createPanel(); + panel.setApp(fakeApp(() => Promise.reject(new Error('WebGL unavailable')))); + + toggleOverlay(true); + + await vi.waitFor(() => expect(checkbox('show-3d-overlay').checked).toBe(false)); + // Regression: storage used to keep true here, resurrecting the + // failed overlay on the next page load. + expect(storage.get('show-3d-overlay')).toBe(false); + expect(stateManager.getDisplayOptions().show3DOverlay).toBe(false); + expect(containerVisible('aircraft-3d-scale-container')).toBe(false); + expect(containerVisible('aircraft-model-container')).toBe(false); + }); + }); }); diff --git a/frontend/src/ui/panels/left/DisplayOptionsPanel.ts b/frontend/src/ui/panels/left/DisplayOptionsPanel.ts index c411158..1289d0e 100644 --- a/frontend/src/ui/panels/left/DisplayOptionsPanel.ts +++ b/frontend/src/ui/panels/left/DisplayOptionsPanel.ts @@ -208,6 +208,7 @@ export class DisplayOptionsPanel extends BasePanel { } update.show3DOverlay = show3DOverlay; this.setChecked('show-3d-overlay', show3DOverlay); + this.apply3DControlsVisibility(show3DOverlay); const aircraft3DScale = this.loadStored('aircraft-3d-scale', defaults.aircraft3DScale); update.aircraft3DScale = aircraft3DScale; @@ -324,39 +325,40 @@ export class DisplayOptionsPanel extends BasePanel { private setupRenderModeControl(): void { this.bindCheckbox('show-3d-overlay', async (checked) => { storage.set('show-3d-overlay', checked); - - if (this.stateManager) { - this.stateManager.updateDisplayOptions({ show3DOverlay: checked }); - } - - if (this.app) { - const mapOverlay = this.app.getMapOverlay(); - if (mapOverlay) { - try { - logger.info('DisplayOptionsPanel', `${checked ? 'Enabling' : 'Disabling'} 3D overlay...`); - await mapOverlay.updateDisplayOptions({ show3DOverlay: checked }); - logger.info('DisplayOptionsPanel', `3D overlay ${checked ? 'enabled' : 'disabled'} successfully`); - } catch (error) { - // Roll the checkbox and state back so UI matches reality - logger.error('DisplayOptionsPanel', `Failed to toggle 3D overlay: ${error}`); - this.setChecked('show-3d-overlay', !checked); - if (this.stateManager) { - this.stateManager.updateDisplayOptions({ show3DOverlay: !checked }); - } - } + this.stateManager?.updateDisplayOptions({ show3DOverlay: checked }); + + const mapOverlay = this.app?.getMapOverlay(); + if (mapOverlay) { + try { + logger.info('DisplayOptionsPanel', `${checked ? 'Enabling' : 'Disabling'} 3D overlay...`); + await mapOverlay.updateDisplayOptions({ show3DOverlay: checked }); + } catch (error) { + // Roll the checkbox, storage and state back so the UI and + // the next page load both match reality + logger.error('DisplayOptionsPanel', `Failed to toggle 3D overlay: ${error}`); + this.setChecked('show-3d-overlay', !checked); + storage.set('show-3d-overlay', !checked); + this.stateManager?.updateDisplayOptions({ show3DOverlay: !checked }); + this.apply3DControlsVisibility(!checked); + return; } } - // Reveal the 3D controls section when the overlay turns on - if (checked) { - const threeDControls = document.getElementById('threeD-controls'); - if (threeDControls && threeDControls.style.display === 'none') { - this.toggleCollapsibleSection('threeD-controls', 'threeD-visible'); - } - } + this.apply3DControlsVisibility(checked); }); } + /** + * Show the global 3D scale and model controls only while the overlay + * is on. Clearing the inline style falls back to their CSS flex layout. + */ + private apply3DControlsVisibility(visible: boolean): void { + for (const id of ['aircraft-3d-scale-container', 'aircraft-model-container']) { + const container = document.getElementById(id); + if (container) container.style.display = visible ? '' : 'none'; + } + } + private setup3DScaleControl(): void { // Apply on Enter (then drop focus) and on blur this.bindEvent('aircraft-3d-scale', 'keydown', (e) => { diff --git a/frontend/src/ui/panels/left/SimulationNodesPanel.test.ts b/frontend/src/ui/panels/left/SimulationNodesPanel.test.ts index 1be0e6c..45c1ee2 100644 --- a/frontend/src/ui/panels/left/SimulationNodesPanel.test.ts +++ b/frontend/src/ui/panels/left/SimulationNodesPanel.test.ts @@ -202,4 +202,66 @@ describe('SimulationNodesPanel', () => { expect(emitted).toEqual([['set_active_node', { node_id: 'b' }]]); }); }); + + describe('socket wiring', () => { + type Listener = (data: NodeInfo) => void; + + // Fake with socket.io off() semantics: off(event) drops every + // listener for the event, off(event, fn) drops only fn. + const makeFakeSocket = () => { + const listeners = new Map>(); + const socket = { + connected: true, + on: (event: string, handler: Listener) => { + if (!listeners.has(event)) listeners.set(event, new Set()); + listeners.get(event)!.add(handler); + }, + off: (event: string, handler?: Listener) => { + if (handler) listeners.get(event)?.delete(handler); + else listeners.delete(event); + }, + emit: vi.fn(), + }; + return { socket, listeners }; + }; + + const asManager = (socket: unknown): SocketManager => + ({ getSocket: () => socket }) as unknown as SocketManager; + + it('destroy() removes only its own node_info listener', () => { + const { socket, listeners } = makeFakeSocket(); + // Stands in for SocketManager's own forward listener, which + // shares the socket and must survive the panel's teardown. + const outsider = vi.fn(); + socket.on('node_info', outsider); + panel.setSocketManager(asManager(socket)); + expect(listeners.get('node_info')!.size).toBe(2); + + panel.destroy(); + + const remaining = listeners.get('node_info'); + expect(remaining && [...remaining]).toEqual([outsider]); + }); + + it('re-setting the socket manager does not double-subscribe', () => { + const { socket, listeners } = makeFakeSocket(); + const manager = asManager(socket); + + panel.setSocketManager(manager); + panel.setSocketManager(manager); + + expect(listeners.get('node_info')!.size).toBe(1); + }); + + it('switching sockets unsubscribes from the old one', () => { + const first = makeFakeSocket(); + const second = makeFakeSocket(); + + panel.setSocketManager(asManager(first.socket)); + panel.setSocketManager(asManager(second.socket)); + + expect(first.listeners.get('node_info')?.size ?? 0).toBe(0); + expect(second.listeners.get('node_info')!.size).toBe(1); + }); + }); }); diff --git a/frontend/src/ui/panels/left/SimulationNodesPanel.ts b/frontend/src/ui/panels/left/SimulationNodesPanel.ts index ad1f722..fdc4d34 100644 --- a/frontend/src/ui/panels/left/SimulationNodesPanel.ts +++ b/frontend/src/ui/panels/left/SimulationNodesPanel.ts @@ -32,6 +32,7 @@ export class SimulationNodesPanel extends BasePanel { private socketManager: SocketManager | null = null; private nodeData: NodeInfo | null = null; private nodeItems: Map = new Map(); + private readonly nodeInfoHandler = (data: NodeInfo): void => this.handleNodeInfo(data); // DOM elements private totalNodesSpan: HTMLElement | null = null; @@ -85,7 +86,7 @@ export class SimulationNodesPanel extends BasePanel { // Add node button if (this.addNodeButton) { this.addEventListener(this.addNodeButton, 'click', () => { - this.showAddNodeModal(); + this.requestAddNode(); }); } } @@ -94,15 +95,10 @@ export class SimulationNodesPanel extends BasePanel { * Set the socket manager for communication with backend */ public setSocketManager(socketManager: SocketManager): void { + // Re-wiring must not leave a previous socket still calling our handler + this.socketManager?.getSocket()?.off('node_info', this.nodeInfoHandler); this.socketManager = socketManager; - - // Subscribe to node_info events - const socket = socketManager.getSocket(); - if (socket) { - socket.on('node_info', (data: NodeInfo) => { - this.handleNodeInfo(data); - }); - } + socketManager.getSocket()?.on('node_info', this.nodeInfoHandler); } /** @@ -335,9 +331,8 @@ export class SimulationNodesPanel extends BasePanel { return; } - // Get friendly node name for logging const nodeData = this.nodeData?.nodes[nodeId]; - const friendlyName = nodeData ? `Node ${nodeData.node_num || 1}` : nodeId; + const friendlyName = nodeData ? this.getNodeAlias(nodeData) : nodeId; logger.info('SimulationNodesPanel', 'Switching to node:', friendlyName); @@ -370,9 +365,9 @@ export class SimulationNodesPanel extends BasePanel { } /** - * Add a new node to the simulation + * Request a new simulation node from the backend */ - private showAddNodeModal(): void { + private requestAddNode(): void { if (!this.socketManager) { logger.warn('SimulationNodesPanel', 'Cannot add node: SocketManager not set'); return; @@ -416,7 +411,7 @@ export class SimulationNodesPanel extends BasePanel { } const nodeData = this.nodeData?.nodes[nodeId]; - const friendlyName = nodeData ? `Node ${nodeData.node_num || 1}` : nodeId; + const friendlyName = nodeData ? this.getNodeAlias(nodeData) : nodeId; if (!confirm(`Kill ${friendlyName}? Its running simulation will be lost.`)) { return; @@ -444,31 +439,13 @@ export class SimulationNodesPanel extends BasePanel { } } - /** - * Get current node data - */ - public getNodeData(): NodeInfo | null { - return this.nodeData; - } - - /** - * Get active node ID - */ - public getActiveNode(): string | null { - return this.nodeData?.active_node || null; - } - /** * Cleanup */ protected override onDestroy(): void { - // Unsubscribe from socket events - if (this.socketManager) { - const socket = this.socketManager.getSocket(); - if (socket) { - socket.off('node_info'); - } - } + // Remove only this panel's listener; a bare off('node_info') would + // also detach SocketManager's forward listener for the same event + this.socketManager?.getSocket()?.off('node_info', this.nodeInfoHandler); this.nodeData = null; this.socketManager = null;