Fix STACK command forwarding, ACDATA caching, and assorted UI bugs

WebATM/proxy/handlers/commands.py

@@ -3,11 +3,18 @@
 STACKCMDS carries the server's command dictionary, which is cached on the
 proxy and forwarded to browsers as the ``cmddict`` Socket.IO event so the web
 console can validate and autocomplete commands. STACK carries command lines
-forwarded by the server — commands the simulation did not recognize and
-assumes are GUI/client commands (typically PAN/ZOOM lines in scenario files).
-They are handled locally and never echoed back to the server.
+forwarded by the simulation — commands it did not recognize and assumes are
+GUI/client commands (typically PAN/ZOOM lines in scenario files). They are
+forwarded to browsers as ``stack_command`` events, where the frontend's
+CommandHandler executes the ones it implements (PAN, ZOOM, ...) exactly as if
+they had been typed into the console — matching BlueSky's own GUIs, which run
+forwarded commands through their client stack. Nothing is ever sent back to
+the server (that would loop: the simulation forwarded the line precisely
+because it cannot execute it).
 """
 
+import time
+
 from ...logger import get_logger
 from ._base import REPLACE_ACTIONS, active_proxy, is_active_node, shared_context
 
@@ -61,58 +68,58 @@ def on_stackcmds_received(data):
 
 
 def on_stack_received(data):
-    """Process a BlueSky STACK event carrying server-forwarded command lines.
+    """Process a BlueSky STACK event carrying sim-forwarded command lines.
 
-    Normalizes the payload to a list of command lines and runs each non-empty
-    line through ``_process_server_command``, which executes it locally and
-    reports the outcome via the proxy's echo channel. Nothing is sent back to
-    the BlueSky server.
+    Normalizes the payload to individual command lines — BlueSky's
+    ``stack.forward()`` joins multiple commands into one semicolon-separated
+    string, and its reference client splits them back apart (``stack()`` in
+    ``bluesky.stack.stackbase``), so the same split happens here — and
+    forwards each non-empty line to browsers via ``_forward_to_browsers``.
 
     Args:
-        data (str | list | tuple): One command line, or a sequence of command
-            lines, forwarded by the server. Other types are logged and ignored.
+        data (str | list | tuple): One command line (possibly
+            semicolon-joined), or a sequence of such lines. Other types are
+            logged and ignored.
     """
     proxy = active_proxy()
     if not proxy:
         return
 
     if isinstance(data, str):
-        commands_to_process = [data]
+        received = [data]
     elif isinstance(data, (list, tuple)):
-        commands_to_process = list(data)
+        received = list(data)
     else:
         logger.warning(f"Unexpected STACK data format: {type(data)}")
         return
 
-    for cmdline in commands_to_process:
-        if isinstance(cmdline, str) and cmdline.strip():
-            _process_server_command(proxy, cmdline)
+    for cmdline in received:
+        if not isinstance(cmdline, str):
+            continue
+        for line in cmdline.split(";"):
+            line = line.strip()
+            if line:
+                _forward_to_browsers(proxy, line)
 
 
-def _process_server_command(proxy, cmdline):
-    """Execute a server-forwarded command locally without sending it back.
+def _forward_to_browsers(proxy, cmdline):
+    """Emit one sim-forwarded command line to browsers as ``stack_command``.
 
-    Runs the command through ``proxy._execute_local_command``. Commands the
-    web client does not implement (e.g. desktop-GUI view commands like PAN or
-    ZOOM from a scenario file) are benign no-ops here, so they are reported to
-    web clients as a warning, not an error.
+    The frontend executes the line through its CommandHandler (so PAN/ZOOM
+    lines from scenario files move the map, like they move BlueSky's own
+    GUIs) and reports unsupported commands itself. With no browser connected
+    there is nothing to act on the command, so it is only logged.
 
     Args:
-        proxy (BlueSkyProxy): The active proxy used for command execution
-            and echoing.
-        cmdline (str): The raw command line received from the server.
+        proxy (BlueSkyProxy): The active proxy.
+        cmdline (str): A single, stripped command line from the server.
     """
-    cmd, _, argstring = cmdline.strip().partition(" ")
-    cmd = cmd.upper()
-
+    if not (proxy.socketio and proxy.connected_clients > 0):
+        logger.debug(f"No browsers connected; dropping forwarded command: {cmdline}")
+        return
     try:
-        success, echotext = proxy._execute_local_command(cmd, argstring.strip())
+        proxy.socketio.emit(
+            "stack_command", {"command": cmdline, "timestamp": time.time()}
+        )
     except Exception as e:
-        logger.error(f"Error executing server command '{cmd}': {e}")
-        proxy._echo_response(f"Error executing server command '{cmd}': {e}", 1)
-        return
-
-    if success:
-        proxy._echo_response(echotext, 0)
-    else:
-        proxy._echo_response(f"{cmd}: not supported by the web client (ignored)", 2)
+        logger.error(f"Error forwarding '{cmdline}' to browsers: {e}")

WebATM/proxy/handlers/shapes.py

@@ -23,11 +23,8 @@
 # Nautical miles per degree of latitude (1 arc-minute == 1 nm).
 _NM_PER_DEGREE = 60.0
 
-# BlueSky stores a shape's vertical extent (metres) on ``Shape.top``/``bottom``,
-# defaulting to +/-1e9 for "unbounded". amvlab BlueSky additionally publishes
-# these on the POLY payload so WebATM can extrude the shape into a 3D volume;
-# vanilla BlueSky omits them. Any bound at or beyond this magnitude is treated
-# as unbounded (no vertical extent -> flat 2D, the vanilla behaviour).
+# BlueSky uses +/-1e9 m as the "unbounded" vertical extent of a shape; any
+# bound at or beyond this magnitude is treated as unbounded (renders flat).
 _ALT_UNBOUNDED = 9e8
 
 # Cap on stored shapes per node and kind (demo limit): oldest are dropped.
@@ -38,15 +35,11 @@ def _normalize_altitudes(shape_dict):
     """Keep only finite vertical bounds on a shape dict, in place.
 
     amvlab BlueSky publishes ``top``/``bottom`` (metres) on each shape so the
-    frontend can extrude it; vanilla BlueSky sends neither. BlueSky uses
-    +/-1e9 as its "unbounded" sentinel, which must never reach the 3D renderer
-    as a literal extrusion height. This keeps ``top`` only when it is a finite
-    upper bound and ``bottom`` only when it is a finite lower bound, popping
-    anything else (missing, non-numeric, or sentinel) so the shape renders flat.
-
-    The frontend treats the mere presence of ``top`` as "has altitude", so a
-    fully-unbounded shape ends up with neither key and renders in 2D, exactly
-    like it does against vanilla BlueSky.
+    frontend can extrude it; vanilla BlueSky sends neither. The +/-1e9
+    "unbounded" sentinels must never reach the 3D renderer as literal
+    extrusion heights, so anything non-numeric or at sentinel magnitude is
+    popped and the shape renders flat (the frontend treats the presence of
+    ``top`` as "has altitude").
 
     Args:
         shape_dict (dict): A per-shape payload that may carry ``top``/``bottom``.
@@ -192,18 +185,33 @@ def _deleted_names(poly_data):
     return []
 
 
+def _is_full_definition(info):
+    """Check whether a payload entry is a full shape (re)definition.
+
+    BlueSky's ``defineArea`` always sends ``shape`` and ``coordinates``
+    together; partial updates (e.g. COLOR) carry neither.
+
+    Args:
+        info (Any): Separated per-shape payload.
+
+    Returns:
+        bool: True when the entry defines the shape rather than patching it.
+    """
+    return isinstance(info, dict) and "shape" in info
+
+
 def _merge_shape(target, name, info):
-    """Merge one shape payload into a store, patching partial updates in place.
+    """Patch one partial shape update into a store, in place.
 
-    An Update action may carry only the changed fields (e.g. a COLOR change
-    sends ``{"color": ...}`` without coordinates); replacing the stored entry
-    would wipe the shape's geometry, so dict payloads are merged into the
-    existing dict instead.
+    A partial Update carries only the changed fields (e.g. COLOR sends
+    ``{"color": ...}`` without coordinates); replacing the stored entry would
+    wipe the shape's geometry, so dict payloads are merged into the existing
+    dict instead.
 
     Args:
         target (dict): Shape store (name -> shape info) to merge into.
         name (str): Shape name.
-        info (Any): New shape payload.
+        info (Any): Partial shape payload.
     """
     existing = target.get(name)
     if isinstance(existing, dict) and isinstance(info, dict):
@@ -215,22 +223,37 @@ def _merge_shape(target, name, info):
 def _merge_shapes(store_polys, store_lines, new_polys, new_lines):
     """Merge separated new shapes into a node's polygon and polyline stores.
 
+    A full definition decides its kind authoritatively: BlueSky's
+    ``defineArea`` overwrites any existing shape with the same name (even one
+    of a different kind) with a fresh object, so the stored entry is replaced
+    and the name is evicted from the other store. Otherwise a redefinition
+    from LINE to POLY would be trapped in the polyline store (rendering an
+    area as a thin line), the reverse would leave a stale polygon behind, and
+    dropped server-side fields (a previous COLOR, POLYALT bounds) would
+    linger. Partial updates merge into whichever store already holds the
+    shape.
+
     Args:
         store_polys (dict): Stored polygon mapping for the node.
         store_lines (dict): Stored polyline mapping for the node.
         new_polys (dict): Newly received polygon shapes.
         new_lines (dict): Newly received polyline shapes.
     """
     for name, info in new_polys.items():
-        # A partial update carries no 'shape' field and lands in the polygon
-        # bucket by default; route it to the store that already holds the shape
-        # so a polyline's update doesn't create a phantom polygon.
-        if name in store_lines and name not in store_polys:
+        if _is_full_definition(info):
+            store_lines.pop(name, None)
+            store_polys[name] = info
+        elif name in store_lines and name not in store_polys:
+            # Partial update for a stored polyline: it lands in the polygon
+            # bucket by default (no 'shape' field), so route it home.
             _merge_shape(store_lines, name, info)
         else:
             _merge_shape(store_polys, name, info)
     for name, info in new_lines.items():
-        _merge_shape(store_lines, name, info)
+        # Line-bucket entries always carry shape == "LINE" (that is the
+        # routing criterion), so they are full definitions.
+        store_polys.pop(name, None)
+        store_lines[name] = info
 
 
 def _trim_shape_store(polys, kind):
@@ -326,16 +349,13 @@ def on_poly_received(data, *args, **kwargs):
 def _separate_poly_and_polyline_data(poly_data):
     """Split combined POLY data into polygons and polylines by ``shape`` field.
 
-    Shapes marked ``LINE`` become polylines; ``POLYALT`` shapes are rewritten
-    as ``POLY`` polygons; ``BOX`` (two opposite corners) and ``CIRCLE`` (centre
-    plus radius in nautical miles) are expanded into ``POLY`` polygon rings;
-    everything else is treated as a polygon. Flat ``coordinates`` arrays
-    (``[lat1, lon1, lat2, lon2, ...]``) are converted to separate ``lat``/``lon``
-    lists for web-client compatibility. Finite ``top``/``bottom`` vertical
-    bounds (metres) are passed through so the frontend can extrude the shape in
-    3D; BlueSky's unbounded +/-1e9 sentinels (and vanilla BlueSky's absent
-    altitudes) are dropped so those shapes stay flat. On errors or unexpected
-    formats the whole payload is treated as polygons.
+    Shapes marked ``LINE`` become polylines; ``POLYALT`` is rewritten as
+    ``POLY``; ``BOX`` (two opposite corners) and ``CIRCLE`` (centre plus
+    radius in nautical miles) are expanded into ``POLY`` polygon rings;
+    everything else is treated as a polygon. Flat ``coordinates`` arrays are
+    split into the ``lat``/``lon`` lists the web client wants, and only finite
+    ``top``/``bottom`` bounds survive (see ``_normalize_altitudes``). On
+    errors or unexpected formats the whole payload is treated as polygons.
 
     Args:
         poly_data (dict): JSON-serializable POLY payload, expected to contain a
@@ -368,20 +388,13 @@ def _separate_poly_and_polyline_data(poly_data):
                     polylines_polys[name] = info
                     continue
 
-                if shape == "BOX":
-                    # Two opposite corners; expand into a 4-corner POLY ring
-                    # (the flat-pair path below can't - a box has only 2 points).
-                    info["shape"] = "POLY"
-                    if not ("lat" in info and "lon" in info):
-                        corners = _box_corners(info.get("coordinates"))
-                        if corners:
-                            info["lat"], info["lon"] = corners
-                            info["name"] = name
-                elif shape == "CIRCLE":
-                    # Centre + radius (nm); tessellate into a POLY ring.
+                if shape in ("BOX", "CIRCLE"):
+                    # Corner-pair / centre+radius encodings; expand into a
+                    # POLY ring (the flat-pair path below can't handle them).
                     info["shape"] = "POLY"
                     if not ("lat" in info and "lon" in info):
-                        ring = _circle_ring(info.get("coordinates"))
+                        expand = _box_corners if shape == "BOX" else _circle_ring
+                        ring = expand(info.get("coordinates"))
                         if ring:
                             info["lat"], info["lon"] = ring
                             info["name"] = name

WebATM/proxy/handlers/simulation.py

@@ -84,17 +84,20 @@ def on_siminfo_received(
             proxy.socketio.emit("siminfo", proxy.sim_data)
             proxy.last_siminfo_emit = current_time
         except Exception as e:
-            logger.error(f"Proxy→Web: Error sending SIMINFO: {e}")
+            logger.error(f"Error emitting SIMINFO: {e}")
 
 
 def on_acdata_received(data):
     """Process a BlueSky ACDATA traffic frame and emit ``acdata`` to web clients.
 
-    Hot path: the network timer delivers ACDATA at up to 50 Hz, but it is only
-    emitted to browsers at ``acdata_interval`` (10 Hz) and only for the active
-    node. ``make_json_serializable`` is the dominant per-frame cost, so it is
-    deferred until after the active-node filter and the emit throttle decide the
-    frame is actually sent. Set WEBATM_PERF=1 (WebATM.proxy.perf) to measure it.
+    Hot path: the network timer delivers ACDATA at up to 50 Hz, but frames are
+    only serialized and cached at ``acdata_interval`` (10 Hz), only for the
+    active node, and only emitted to browsers while any are connected.
+    ``make_json_serializable`` is the dominant per-frame cost, so it is deferred
+    until after the active-node filter and the throttle accept the frame. The
+    cache is refreshed even with no browser attached — ``initial_data``, the
+    backup timer and the ROUTEDATA filter serve it, so it must track the live
+    feed. Set WEBATM_PERF=1 (WebATM.proxy.perf) to measure the hot path.
 
     Aircraft clearing on a simulation reset is NOT handled here: BlueSky
     signals a reset on the dedicated RESET topic (events.on_reset_received),
@@ -136,24 +139,28 @@ def on_acdata_received(data):
             return
 
         # Throttle BEFORE serializing: serialization is the dominant per-frame
-        # cost, so pay it only for frames that are actually emitted.
+        # cost, so pay it at most once per acdata_interval.
         current_time = time.time()
-        if not (
-            proxy.socketio
-            and proxy.connected_clients > 0
-            and (current_time - proxy.last_acdata_emit) >= proxy.acdata_interval
-        ):
+        if (current_time - proxy.last_acdata_emit) < proxy.acdata_interval:
             return
 
+        # Refresh the cache even with no browser attached: the initial_data
+        # snapshot, the 0.5 s backup timer and the ROUTEDATA aircraft filter
+        # all serve proxy.traffic_data, so a cache frozen while unwatched
+        # would hand the next client the aircraft from the moment the last
+        # one left (ghosts at stale positions).
         t0 = time.perf_counter()
         serializable_data = make_json_serializable(data)
         data_path_perf.record_serialize(time.perf_counter() - t0)
         proxy.traffic_data = serializable_data
+        proxy.last_acdata_emit = current_time
+
+        if not (proxy.socketio and proxy.connected_clients > 0):
+            return
 
         try:
             t1 = time.perf_counter()
             proxy.socketio.emit("acdata", serializable_data)
-            proxy.last_acdata_emit = current_time
             data_path_perf.record_emit(time.perf_counter() - t1)
         except Exception:
             logger.exception("Error emitting ACDATA")
@@ -210,6 +217,6 @@ def on_statechange_received(data, sender_id=None):
         try:
             proxy.socketio.emit("statechange", payload)
         except Exception as e:
-            logger.error(f"Proxy->Web: Error sending STATECHANGE: {e}")
+            logger.error(f"Error emitting STATECHANGE: {e}")
 
     logger.info(f"STATECHANGE from {sender_id_str}: simstate={simstate}")