From 28e656598ffc018b31ed21662697e073ddc83acc Mon Sep 17 00:00:00 2001 From: Andres Morfin Veytia <78442543+amorfinv@users.noreply.github.com> Date: Sat, 22 Aug 2026 08:18:22 +0200 Subject: [PATCH 1/3] Fix ECHO payload dispatch and duplicate REQUEST on node discovery Normalize ECHO payloads (list/dict/bare string) to (text, flags, sender_id) in one place; oversized list payloads no longer splat into the handler and silently drop the echo with a TypeError. Also drop the duplicated REQUEST send when a new node is announced, which made every node re-send its POLY shapes and full STACKCMDS dictionary twice, and simplify safe_decode (the ASCII fallback after a UTF-8 failure could never succeed). Co-Authored-By: Claude Fable 5 --- WebATM/bluesky_client.py | 90 ++++++++++------------------------ tests/test_bluesky_client.py | 95 ++++++++++++++++++++++++++++++++++++ 2 files changed, 121 insertions(+), 64 deletions(-) diff --git a/WebATM/bluesky_client.py b/WebATM/bluesky_client.py index 9855141..a71c0c5 100644 --- a/WebATM/bluesky_client.py +++ b/WebATM/bluesky_client.py @@ -17,6 +17,7 @@ """ import threading +import traceback from collections import defaultdict, deque from collections.abc import Callable @@ -147,10 +148,10 @@ def seqidx2id(seqidx): def safe_decode(data): """Decode bytes to a readable string without raising. - Attempts UTF-8 decoding first and returns the result only if it consists - entirely of printable ASCII characters; otherwise falls back to ASCII - decoding, and finally to an uppercase hexadecimal representation. Non-bytes - input is converted with ``str()``. + Attempts UTF-8 decoding and returns the result only if it consists + entirely of printable ASCII characters; otherwise falls back to an + uppercase hexadecimal representation. Non-bytes input is converted with + ``str()``. Args: data (bytes | object): Value to decode or stringify. @@ -160,22 +161,12 @@ def safe_decode(data): """ if isinstance(data, bytes): try: - # First try utf-8 decoding decoded = data.decode("utf-8") - # Check if the decoded string contains only printable ASCII characters - if all(32 <= ord(c) <= 126 for c in decoded): - return decoded - else: - # Contains non-printable characters, use hex representation - return data.hex().upper() except UnicodeDecodeError: - try: - # Try ASCII decoding - decoded = data.decode("ascii") - return decoded - except UnicodeDecodeError: - # Unable to decode as text, use hex representation - return data.hex().upper() + return data.hex().upper() + if all(32 <= ord(c) <= 126 for c in decoded): + return decoded + return data.hex().upper() return str(data) @@ -232,16 +223,11 @@ def emit(self, *args, **kwargs): *args (Any): Positional arguments forwarded to each callback. **kwargs (Any): Keyword arguments forwarded to each callback. """ - callbacks_snapshot = self.callbacks[ - : - ] # Make a copy to avoid concurrency issues - for callback in callbacks_snapshot: + for callback in self.callbacks[:]: try: callback(*args, **kwargs) except Exception as e: logger.warning(f"Signal {self.name}: Error in callback {callback}: {e}") - import traceback - traceback.print_exc() @@ -289,11 +275,7 @@ def emit(self, topic: str, *args, **kwargs): callback(*args, **kwargs) except Exception as e: logger.warning(f"Subscriber {topic}: Error in callback {callback}: {e}") - logger.debug(f"Subscriber {topic}: Error type: {type(e).__name__}") - logger.debug(f"Subscriber {topic}: Args: {args}") - logger.debug(f"Subscriber {topic}: Kwargs: {kwargs}") - import traceback - + logger.debug(f"Subscriber {topic}: Args: {args} Kwargs: {kwargs}") traceback.print_exc() @@ -711,31 +693,21 @@ def _process_data_message(self, msg): else: self.subscriber.emit(topic, data) # Pass as single argument elif topic == "ECHO": - # ECHO expects: text, flags, sender_id (can be called with varying args) - # Always include sender_id from message header to identify which node sent the echo - if isinstance(data, (list, tuple)): - # Ensure we always pass sender_id from message header - if len(data) >= 3: - # Data already contains [text, flags, sender_id] - self.subscriber.emit(topic, *data) - elif len(data) == 2: - # Data is [text, flags] - add sender_id from header - self.subscriber.emit(topic, data[0], data[1], sender_id) - elif len(data) == 1: - # Data is [text] - add default flags and sender_id from header - self.subscriber.emit(topic, data[0], 0, sender_id) - else: - # Empty list - send empty text with sender_id from header - self.subscriber.emit(topic, "", 0, sender_id) - elif isinstance(data, dict): + # ECHO handlers expect (text, flags, sender_id). Normalize + # the payload — [text], [text, flags], [text, flags, + # sender_id], a dict, or a bare string — filling missing + # flags with 0 and the sender from the message header. + if isinstance(data, dict): text = data.get("text", "") flags = data.get("flags", 0) - # Use sender_id from data if available, otherwise from message header - data_sender_id = data.get("sender_id", sender_id) - self.subscriber.emit(topic, text, flags, data_sender_id) + echo_sender = data.get("sender_id", sender_id) else: - # Simple string or other data - add defaults and sender_id from header - self.subscriber.emit(topic, str(data), 0, sender_id) + if not isinstance(data, (list, tuple)): + data = [str(data)] + text = data[0] if len(data) > 0 else "" + flags = data[1] if len(data) > 1 else 0 + echo_sender = data[2] if len(data) > 2 else sender_id + self.subscriber.emit(topic, text, flags, echo_sender) elif topic == "STATECHANGE": # STATECHANGE follows BlueSky's shared-state format: # [action_type, {"simstate": , ...}] @@ -973,20 +945,10 @@ def delnode(self, node_id): return self.send("DELNODE", node_id, target_server) def on_node_added_request_data(self, node_id): - """When a new node is announced, request the initial/current state of all - subscribed shared states.""" - logger.info("A new node has been added! request topics") - - # TODO: fix request - # Request all BlueSky topics we want to receive add #STACK - # topics = ['RESET', 'REQUEST', 'PLOT', 'SHOWDIALOG', 'SIMINFO', - # 'SIMSETTINGS', 'TRAILS', 'ROUTEDATA', 'ACDATA', 'DEFWPT', - # 'POLY', 'STACKCMDS'] - + """When a new node is announced, request the current state of the + subscribed shared states (shapes and the command dictionary).""" topics = ["POLY", "STACKCMDS"] - - logger.debug( + logger.info( f"Requesting topics {topics} from all nodes (triggered by new node {safe_decode(node_id)})" ) self.send("REQUEST", topics) - self.send("REQUEST", topics) diff --git a/tests/test_bluesky_client.py b/tests/test_bluesky_client.py index b194a15..fe8cc6e 100644 --- a/tests/test_bluesky_client.py +++ b/tests/test_bluesky_client.py @@ -383,6 +383,61 @@ def test_poly_single_action_sets_context_and_unwraps(self): assert received == [(b"U", payload)] assert client.context.sender_id == b"NODE\x81" + def test_echo_list_payload_gets_sender_from_header(self): + # BlueSky sends ECHO as [text, flags]; the header sender must be + # appended so the handler knows which node answered. + client = BlueSkyClient() + received = [] + client.subscriber.subscribe( + "ECHO", + lambda text, flags, sender_id: received.append((text, flags, sender_id)), + ) + + client._process_data_message(self._frame("ECHO", ["Roger", 4])) + + assert received == [("Roger", 4, b"NODE\x81")] + + def test_echo_dict_payload_prefers_its_own_sender(self): + client = BlueSkyClient() + received = [] + client.subscriber.subscribe( + "ECHO", + lambda text, flags, sender_id: received.append((text, flags, sender_id)), + ) + + client._process_data_message( + self._frame("ECHO", {"text": "hi", "flags": 1, "sender_id": "n2"}) + ) + + assert received == [("hi", 1, "n2")] + + def test_echo_bare_string_payload_gets_defaults(self): + client = BlueSkyClient() + received = [] + client.subscriber.subscribe( + "ECHO", + lambda text, flags, sender_id: received.append((text, flags, sender_id)), + ) + + client._process_data_message(self._frame("ECHO", "plain")) + + assert received == [("plain", 0, b"NODE\x81")] + + def test_echo_oversized_payload_still_delivers(self): + # A payload with more than three items used to be splatted straight + # into the handler, raising TypeError inside the subscriber and + # silently dropping the echo. Extra items are now ignored. + client = BlueSkyClient() + received = [] + client.subscriber.subscribe( + "ECHO", + lambda text, flags, sender_id: received.append((text, flags, sender_id)), + ) + + client._process_data_message(self._frame("ECHO", ["msg", 2, "n3", "extra"])) + + assert received == [("msg", 2, "n3")] + def test_poly_collected_multi_action_dispatches_each_pair(self): # The POLY publisher collects actions between send ticks, so one # message can carry several (action, payload) pairs — e.g. an update @@ -399,3 +454,43 @@ def test_poly_collected_multi_action_dispatches_each_pair(self): client._process_data_message(self._frame("POLY", [b"U", update, b"D", delete])) assert received == [(b"U", update), (b"D", delete)] + + +class TestNodeDiscovery: + """_process_subscription_message turns raw (un)subscribe frames into node + discovery, which triggers exactly one REQUEST for the shared states.""" + + @staticmethod + def _subscribe_frame(sender): + from WebATM.bluesky_client import MSG_SUBSCRIBE + + return [bytes([MSG_SUBSCRIBE]) + sender] + + def test_new_node_requests_shared_state_once(self, monkeypatch): + # Each REQUEST broadcast makes every node re-send its POLY shapes and + # the full STACKCMDS command dictionary, so a duplicate send doubles + # that traffic for no benefit. + client = BlueSkyClient() + sent = [] + monkeypatch.setattr( + client, + "send", + lambda topic, data="", to_group="": sent.append((topic, data, to_group)), + ) + node = b"S\x01\x02\x03\x81" + + client._process_subscription_message(self._subscribe_frame(node)) + + assert node in client.nodes + assert sent == [("REQUEST", ["POLY", "STACKCMDS"], "")] + + def test_rediscovered_node_is_not_reannounced(self, monkeypatch): + client = BlueSkyClient() + sent = [] + monkeypatch.setattr(client, "send", lambda *a, **k: sent.append(a)) + node = b"S\x01\x02\x03\x81" + + client._process_subscription_message(self._subscribe_frame(node)) + client._process_subscription_message(self._subscribe_frame(node)) + + assert len(sent) == 1 From 8860da7c7d9b939a08e90b6685424d14fa776aef Mon Sep 17 00:00:00 2001 From: Andres Morfin Veytia <78442543+amorfinv@users.noreply.github.com> Date: Sat, 22 Aug 2026 08:18:28 +0200 Subject: [PATCH 2/3] Scope connect-time shape envelopes to the connecting client On connect, emit the active node's poly/polyline envelopes only to the new client instead of broadcasting through _emit_active_node_poly_data, which re-sent (or cleared) shape data on every already-connected browser. The reconnecting client still gets complete envelopes so it can prune shapes deleted while it was away. Also add the standard connected-clients guard to the POLY broadcast in the shapes handler, and factor the duplicated hex-to-binary node ID lookup in the socket handlers into a helper. Co-Authored-By: Claude Fable 5 --- WebATM/proxy/core.py | 4 -- WebATM/proxy/handlers/shapes.py | 4 +- WebATM/server/socket_handlers.py | 78 ++++++++++++++++++-------------- tests/test_socket_handlers.py | 50 ++++++++++++++++++++ 4 files changed, 96 insertions(+), 40 deletions(-) diff --git a/WebATM/proxy/core.py b/WebATM/proxy/core.py index 27e9b44..c42f0b6 100644 --- a/WebATM/proxy/core.py +++ b/WebATM/proxy/core.py @@ -196,10 +196,6 @@ def _on_actnode_changed(self, node_id): """Callback when active node changes.""" return self.node_mgr._on_actnode_changed(node_id) - def _emit_active_node_poly_data(self): - """Emit POLY and POLYLINE data for the currently active node.""" - return self.node_mgr._emit_active_node_poly_data() - def _on_node_added(self, node_id): """Callback when a new node is discovered.""" return self.node_mgr._on_node_added(node_id) diff --git a/WebATM/proxy/handlers/shapes.py b/WebATM/proxy/handlers/shapes.py index 4924937..0aa4e61 100644 --- a/WebATM/proxy/handlers/shapes.py +++ b/WebATM/proxy/handlers/shapes.py @@ -335,7 +335,9 @@ def on_poly_received(data, *args, **kwargs): # sets (not just this message's shapes). active_node_id = proxy._get_safe_active_node() if sender_id and active_node_id and sender_id == active_node_id: - if proxy.socketio: + # Same guard as every other emit site; a client connecting later + # gets the stored sets from its initial_data/connect envelopes. + if proxy.socketio and proxy.connected_clients > 0: proxy.socketio.emit( "poly", proxy.poly_data_by_node.get(sender_id, {"polys": {}}) ) diff --git a/WebATM/server/socket_handlers.py b/WebATM/server/socket_handlers.py index 824b39e..bc82197 100644 --- a/WebATM/server/socket_handlers.py +++ b/WebATM/server/socket_handlers.py @@ -15,6 +15,27 @@ logger = get_logger() +def _tracked_binary_node_id(proxy, node_id): + """Map a frontend hex node ID to the tracked node's binary ID. + + Args: + proxy (BlueSkyProxy): The current BlueSky proxy. + node_id (str): Hex-string node ID as sent by the frontend. + + Returns: + bytes | None: The original binary node ID, or None when the node is + not tracked. + """ + node_data = proxy.tracked_nodes.get(node_id) + if node_data is None: + logger.debug( + f"Could not find node ID for: {node_id} " + f"(available: {list(proxy.tracked_nodes.keys())})" + ) + return None + return node_data.get("node_id") + + def register_socket_handlers(socketio, session_manager): """Register all Socket.IO event handlers. @@ -29,8 +50,8 @@ def on_connect(auth): """Handle a new web client connection (``connect`` event). Creates and tracks a session, increments the connected-client - counter, and sends the ``initial_data`` snapshot and the active - node's shapes. + counter, and sends this client the ``initial_data`` snapshot and the + active node's shape envelopes. Args: auth: Socket.IO auth payload (unused). @@ -52,11 +73,16 @@ def on_connect(auth): ) try: - emit("initial_data", current_app.bluesky_proxy.get_current_data()) - # Shapes created before this client connected. node_info is NOT - # sent here: it would show "Connected (No Data)" before the user + snapshot = current_app.bluesky_proxy.get_current_data() + emit("initial_data", snapshot) + # Complete shape envelopes for this client only. A reconnecting + # browser needs them to prune shapes deleted while it was away + # (initial_data only ever adds shapes); other clients are already + # in sync, so this must not broadcast. node_info is NOT sent + # here: it would show "Connected (No Data)" before the user # connects; it flows naturally once data arrives. - current_app.bluesky_proxy._emit_active_node_poly_data() + emit("poly", snapshot["poly_data"] or {"polys": {}}) + emit("polyline", snapshot["polyline_data"] or {"polys": {}}) except Exception as e: logger.info(f"Error sending initial data to {session_id}: {e}") @@ -64,11 +90,9 @@ def on_connect(auth): def on_disconnect(reason): """Handle a web client disconnect (``disconnect`` event). - Removes the session from the session manager and decrements the - connected-client counter. The counter is only decremented for - connections whose session was actually tracked, keeping it - symmetric with ``on_connect`` (a connection rejected there never - incremented it). + Removes the session and decrements the connected-client counter — + but only for connections whose session was actually tracked, keeping + the counter symmetric with ``on_connect``. Args: reason: Disconnect reason supplied by Flask-SocketIO. @@ -107,10 +131,6 @@ def on_command(data): def on_set_active_node(data): """Switch the active simulation node (``set_active_node`` event). - The frontend sends hex-string node IDs; the handler looks up the - original binary ID in the proxy's tracked nodes before delegating to - ``actnode``. - Args: data (dict): Payload with the hex-string ``node_id``. """ @@ -118,15 +138,10 @@ def on_set_active_node(data): if not node_id: return - node_data = current_app.bluesky_proxy.tracked_nodes.get(node_id) - if node_data is None: - logger.debug( - f"Could not find node ID for: {node_id} " - f"(available: {list(current_app.bluesky_proxy.tracked_nodes.keys())})" - ) + binary_node_id = _tracked_binary_node_id(current_app.bluesky_proxy, node_id) + if binary_node_id is None: return - binary_node_id = node_data.get("node_id") logger.info(f"Setting active node to: {node_id} (binary: {binary_node_id})") try: current_app.bluesky_proxy.actnode(binary_node_id) @@ -158,7 +173,7 @@ def on_add_nodes(data): if server_id and isinstance(server_id, str): server_id = server_id.encode() current_app.bluesky_proxy.addnodes(count, server_id=server_id) - logger.info(f"Added {count} nodes to server {server_id}") + logger.info(f"Requested {count} new node(s) on server {server_id}") except Exception as e: logger.info(f"Error adding nodes: {e}") @@ -166,11 +181,9 @@ def on_add_nodes(data): def on_del_node(data): """Terminate a single simulation node (``del_node`` event). - The frontend sends hex-string node IDs; the handler looks up the - original binary ID in the proxy's tracked nodes before delegating to - ``delnode``, which sends a DELNODE message to the owning server. The - node's removal flows back through the normal node-removed pipeline - (tracked-nodes cleanup, active-node failover, ``node_info`` emission). + Sends a DELNODE message to the owning server; the node's removal + flows back through the normal node-removed pipeline (tracked-nodes + cleanup, active-node failover, ``node_info`` emission). Args: data (dict): Payload with the hex-string ``node_id``. @@ -179,15 +192,10 @@ def on_del_node(data): if not node_id: return - node_data = current_app.bluesky_proxy.tracked_nodes.get(node_id) - if node_data is None: - logger.debug( - f"Could not find node ID for: {node_id} " - f"(available: {list(current_app.bluesky_proxy.tracked_nodes.keys())})" - ) + binary_node_id = _tracked_binary_node_id(current_app.bluesky_proxy, node_id) + if binary_node_id is None: return - binary_node_id = node_data.get("node_id") logger.info( f"Requesting node termination: {node_id} (binary: {binary_node_id})" ) diff --git a/tests/test_socket_handlers.py b/tests/test_socket_handlers.py index 50cfabf..e647eb1 100644 --- a/tests/test_socket_handlers.py +++ b/tests/test_socket_handlers.py @@ -45,6 +45,56 @@ def test_disconnect_untracked_session_does_not_decrement(self, sio): assert app.bluesky_proxy.connected_clients == 1 +class TestConnectShapeEnvelopes: + def test_connect_sends_scoped_shape_envelopes(self, sio): + """Every new connection gets complete poly/polyline envelopes (empty + here — no active node) so a reconnecting browser can prune shapes + deleted while it was away.""" + app, socketio, client = sio + received = {pkt["name"]: pkt["args"] for pkt in client.get_received()} + assert received["poly"][0] == {"polys": {}} + assert received["polyline"][0] == {"polys": {}} + + def test_connect_sends_active_node_shapes_to_new_client(self, sio): + """The connecting client receives the active node's stored shapes.""" + app, socketio, client = sio + proxy = app.bluesky_proxy + + class FakeClient: + act_id = b"\x01\x02" + + proxy.bluesky_client = FakeClient() + proxy.running = True + proxy.was_connected = True + node_id = FakeClient.act_id.hex() + proxy.tracked_nodes[node_id] = {"node_id": FakeClient.act_id} + shapes = {"polys": {"zone1": {"lat": [52.0], "lon": [4.0]}}} + proxy.poly_data_by_node[node_id] = shapes + + second = socketio.test_client(app) + try: + received = {pkt["name"]: pkt["args"] for pkt in second.get_received()} + assert received["poly"][0] == shapes + assert received["polyline"][0] == {"polys": {}} + finally: + second.disconnect() + + def test_connect_does_not_broadcast_shapes_to_other_clients(self, sio): + """A client connecting must not re-send (or clear) shape data on + every other connected client.""" + app, socketio, client = sio + client.get_received() # drain this client's own connect envelopes + + second = socketio.test_client(app) + try: + assert second.is_connected() + events = [pkt["name"] for pkt in client.get_received()] + assert "poly" not in events + assert "polyline" not in events + finally: + second.disconnect() + + class TestCommandEvent: def test_command_returns_result(self, sio): app, socketio, client = sio From b3e3628bcfac6587584e1d0102d5a37f38409135 Mon Sep 17 00:00:00 2001 From: Andres Morfin Veytia <78442543+amorfinv@users.noreply.github.com> Date: Sat, 22 Aug 2026 08:18:35 +0200 Subject: [PATCH 3/3] Fix ghost 3D mesh when the last aircraft is deleted Aircraft3DCustomLayer.updateAircraft returned early on an empty id array, so the removal loop never ran and the last deleted aircraft's mesh lingered on the map until the next non-empty tick or a reset. Only bail when the id array is missing entirely, and add tests covering the removal path. Also drop the unused selected/inconf fields from AircraftMeshData and the never-read lastUpdate timestamp from Aircraft3DFleet. Co-Authored-By: Claude Fable 5 --- .../aircraft/Aircraft3DCustomLayer.test.ts | 142 ++++++++++++++++++ .../ui/map/aircraft/Aircraft3DCustomLayer.ts | 7 +- .../ui/map/aircraft/Aircraft3DFleet.test.ts | 2 +- .../src/ui/map/aircraft/Aircraft3DFleet.ts | 3 - .../map/aircraft/Aircraft3DTransforms.test.ts | 2 +- .../ui/map/aircraft/Aircraft3DTransforms.ts | 2 - 6 files changed, 148 insertions(+), 10 deletions(-) create mode 100644 frontend/src/ui/map/aircraft/Aircraft3DCustomLayer.test.ts diff --git a/frontend/src/ui/map/aircraft/Aircraft3DCustomLayer.test.ts b/frontend/src/ui/map/aircraft/Aircraft3DCustomLayer.test.ts new file mode 100644 index 0000000..0dbd834 --- /dev/null +++ b/frontend/src/ui/map/aircraft/Aircraft3DCustomLayer.test.ts @@ -0,0 +1,142 @@ +/** + * Tests for Aircraft3DCustomLayer.updateAircraft removal handling. + * + * Focus: an empty aircraft batch (the last aircraft was deleted) must still + * fall through to the removal loop so the deleted aircraft's 3D mesh is torn + * down. A previous early-return on `id.length === 0` left it on the map as a + * ghost until the next non-empty tick or a full reset. + */ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import type { AircraftData, DisplayOptions } from '../../../data/types'; + +// Base class: only the members updateAircraft touches, plus a settable scene. +vi.mock('../rendering/CustomLayer3D', () => ({ + CustomLayer3D: class { + id: string; + scene: unknown = undefined; + camera = {}; + renderer = { capabilities: { getMaxAnisotropy: () => 16 } }; + map = null; + constructor(id: string) { + this.id = id; + } + createTransformMatrix() { + return {}; + } + isGlobeProjection() { + return false; + } + }, +})); + +// Stateful fake fleet: tracks live ids and records remove() calls. +const fleetState = new Map(); +const removeCalls: string[] = []; +vi.mock('./Aircraft3DFleet', () => ({ + Aircraft3DFleet: class { + constructor(_deps: unknown) {} + get(id: string) { + return fleetState.get(id); + } + create(id: string, _data: unknown, modelPath: string) { + fleetState.set(id, { modelPath }); + } + remove(id: string) { + removeCalls.push(id); + fleetState.delete(id); + } + update() {} + forEach(cb: (entry: unknown, id: string) => void) { + fleetState.forEach((entry, id) => cb(entry, id)); + } + refreshPending() {} + prunePending() {} + reapplyAllTransforms() {} + }, +})); + +vi.mock('./Aircraft3DModelLoader', () => ({ + Aircraft3DModelLoader: class { + constructor(_opts: unknown) {} + hasFailed() { + return false; + } + load() {} + clearCache() {} + clearAll() {} + }, +})); + +vi.mock('./Aircraft3DTransforms', () => ({ + Aircraft3DTransforms: class { + constructor(_deps: unknown) {} + updateSceneOrigin() { + return false; + } + updateMeshTransform() {} + }, +})); + +import { Aircraft3DCustomLayer } from './Aircraft3DCustomLayer'; + +function makeLayer(): Aircraft3DCustomLayer { + const layer = new Aircraft3DCustomLayer( + { selectedAircraftModel: 'auto' } as DisplayOptions, + null + ); + // Mark the scene ready so updateAircraft processes instead of queuing. + (layer as unknown as { scene: object }).scene = {}; + return layer; +} + +function batch(ids: string[]): AircraftData { + return { + id: ids, + lat: ids.map(() => 52), + lon: ids.map(() => 4), + alt: ids.map(() => 1000), + trk: ids.map(() => 90), + actype: ids.map(() => 'A320'), + inconf: ids.map(() => false), + } as AircraftData; +} + +describe('Aircraft3DCustomLayer.updateAircraft removal', () => { + beforeEach(() => { + fleetState.clear(); + removeCalls.length = 0; + }); + + it('removes a mesh when its aircraft disappears from a non-empty batch', () => { + const layer = makeLayer(); + layer.updateAircraft(batch(['AC1', 'AC2'])); + expect(fleetState.has('AC1')).toBe(true); + + layer.updateAircraft(batch(['AC2'])); + + expect(removeCalls).toContain('AC1'); + expect(fleetState.has('AC1')).toBe(false); + expect(fleetState.has('AC2')).toBe(true); + }); + + it('clears the last aircraft when an empty batch arrives (no ghost)', () => { + const layer = makeLayer(); + layer.updateAircraft(batch(['AC1'])); + expect(fleetState.has('AC1')).toBe(true); + + layer.updateAircraft(batch([])); + + expect(removeCalls).toContain('AC1'); + expect(fleetState.size).toBe(0); + }); + + it('ignores a batch with no id array without throwing', () => { + const layer = makeLayer(); + layer.updateAircraft(batch(['AC1'])); + + expect(() => layer.updateAircraft({} as AircraftData)).not.toThrow(); + // The existing aircraft is left untouched (guarded before removal). + expect(fleetState.has('AC1')).toBe(true); + expect(removeCalls).not.toContain('AC1'); + }); +}); diff --git a/frontend/src/ui/map/aircraft/Aircraft3DCustomLayer.ts b/frontend/src/ui/map/aircraft/Aircraft3DCustomLayer.ts index 1632b65..baf072a 100644 --- a/frontend/src/ui/map/aircraft/Aircraft3DCustomLayer.ts +++ b/frontend/src/ui/map/aircraft/Aircraft3DCustomLayer.ts @@ -114,7 +114,10 @@ export class Aircraft3DCustomLayer extends CustomLayer3D { return; } - if (!aircraftData.id || aircraftData.id.length === 0) { + // An empty batch (last aircraft deleted) must still fall through to + // the removal loop below, or the deleted aircraft's mesh lingers as + // a ghost — only bail when the id array is missing entirely. + if (!aircraftData.id) { return; } @@ -157,8 +160,6 @@ export class Aircraft3DCustomLayer extends CustomLayer3D { lon: aircraftData.lon[i], alt: aircraftData.alt[i], hdg: aircraftData.trk[i], - selected: false, - inconf: aircraftData.inconf ? aircraftData.inconf[i] : false, actype, }; diff --git a/frontend/src/ui/map/aircraft/Aircraft3DFleet.test.ts b/frontend/src/ui/map/aircraft/Aircraft3DFleet.test.ts index bc407fa..f2c9cf4 100644 --- a/frontend/src/ui/map/aircraft/Aircraft3DFleet.test.ts +++ b/frontend/src/ui/map/aircraft/Aircraft3DFleet.test.ts @@ -10,7 +10,7 @@ import { Aircraft3DFleet, type Aircraft3DFleetDeps } from './Aircraft3DFleet'; import type { AircraftMeshData } from './Aircraft3DTransforms'; const DATA: AircraftMeshData = { - lat: 52, lon: 4, alt: 1000, hdg: 90, selected: false, inconf: false, actype: 'A320', + lat: 52, lon: 4, alt: 1000, hdg: 90, actype: 'A320', }; /** A model whose single mesh carries an array of materials. */ diff --git a/frontend/src/ui/map/aircraft/Aircraft3DFleet.ts b/frontend/src/ui/map/aircraft/Aircraft3DFleet.ts index 5950855..cf59e00 100644 --- a/frontend/src/ui/map/aircraft/Aircraft3DFleet.ts +++ b/frontend/src/ui/map/aircraft/Aircraft3DFleet.ts @@ -15,7 +15,6 @@ export interface Aircraft3DMesh { mesh: THREE.Object3D; data: AircraftMeshData; modelPath: string; - lastUpdate: number; currentGroup: 'mercator' | 'globe'; /** Plays the GLB's baked animation clips; absent for static models. */ mixer?: THREE.AnimationMixer; @@ -124,7 +123,6 @@ export class Aircraft3DFleet { mesh, data, modelPath, - lastUpdate: Date.now(), currentGroup: isGlobe ? 'globe' : 'mercator', mixer, }); @@ -155,7 +153,6 @@ export class Aircraft3DFleet { this.applyTransform(aircraftMesh.mesh, data, aircraftMesh.currentGroup === 'globe'); aircraftMesh.data = data; - aircraftMesh.lastUpdate = Date.now(); } /** diff --git a/frontend/src/ui/map/aircraft/Aircraft3DTransforms.test.ts b/frontend/src/ui/map/aircraft/Aircraft3DTransforms.test.ts index a55ed3e..fdb0f40 100644 --- a/frontend/src/ui/map/aircraft/Aircraft3DTransforms.test.ts +++ b/frontend/src/ui/map/aircraft/Aircraft3DTransforms.test.ts @@ -32,7 +32,7 @@ function acData(positions: Array<[number, number]>): AircraftData { } function meshData(lat: number, lon: number): AircraftMeshData { - return { lat, lon, alt: 3000, hdg: 90, selected: false, inconf: false, actype: 'A320' }; + return { lat, lon, alt: 3000, hdg: 90, actype: 'A320' }; } describe('Aircraft3DTransforms scene origin vs invalid coordinates', () => { diff --git a/frontend/src/ui/map/aircraft/Aircraft3DTransforms.ts b/frontend/src/ui/map/aircraft/Aircraft3DTransforms.ts index e3c1878..a1581a9 100644 --- a/frontend/src/ui/map/aircraft/Aircraft3DTransforms.ts +++ b/frontend/src/ui/map/aircraft/Aircraft3DTransforms.ts @@ -21,8 +21,6 @@ export interface AircraftMeshData { lon: number; alt: number; hdg: number; - selected: boolean; - inconf: boolean; actype: string; }