console review follow-up: two edge cases in my own roster fix, found and fixed
Self-review of3a69448(plus an adversarial pass still in flight) turned up two real defects in the work I just shipped: 1. THE STASH LIFECYCLE LEAKED ACROSS SESSIONS AND EGGS. _seat_defaults was keyed by ROW INDEX and initialised once in __init__ -- never cleared on Start Session, on Open/New egg (which rebuilds the table), or on a seat-count change. Concrete harm: stop a session while a seat is OCCUPIED (the stash is only popped when a seat empties, so it survives), reconfigure or open a different egg, start again -- and when that seat next empties the GUI restores LAST SESSION'S snapshot over the new configuration. Fixed by (a) keying the stash by TAG, the seat's actual identity, so a changed roster makes stale keys inert instead of landing on whatever row now sits at that index, and (b) clearing it at both natural boundaries: session start (both local and remote paths -- the clear sits before the remote branch) and the table rebuild in _load_egg_into_ui. 2. POSITIONAL hostID->tag MAPPING IS WRONG AFTER A LAUNCH TRIM -- and3a69448widened its blast radius. The GUI resolves REGISTERED/dropped lines via host_tag(), position into the UNTRIMMED egg roster; after a trim remaps seat ids (e.g. the lone returning player reclaim-held at seat 3 becomes host 2), those lines light the wrong row -- and, with the new seat_info pop on dropped, CLEAR the wrong seat's identity, leaving the real one's name up: the exact reported bug, resurfacing in the trimmed case. Fixed on both sides: the relay's REGISTERED and dropped prints now carry tag='...' from its LIVE (possibly trimmed) roster via a new _tag_of() helper, and the GUI regexes prefer the explicit tag with the positional fallback kept for old logs. SEATED/READY/LEFT already carried tags; only these two were positional. VERIFIED * test_operator_roster.py grown to 22 checks, all passing: post-trim lines light and clear the RIGHT row while the wrong row stays untouched; the legacy no-tag format still resolves positionally; a stopped-while-occupied session cannot leak GHOST into the next session's empty seat; a stale foreign-tag stash never touches a live row. * Live 2-pod rig: both new formats confirmed on the wire (REGISTERED (1/2) tag='...' / dropped: closed tag='...'), mission launched and stopped cleanly, round RESET intact. rearm 19/19, net 17/17. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
3a694485bf
commit
820caf8765
@@ -103,5 +103,53 @@ if len(tags) > 1:
|
||||
check("row 1 untouched by row 0 traffic", callsign(1) == before,
|
||||
"%r vs %r" % (callsign(1), before))
|
||||
|
||||
print("\n=== POST-TRIM: explicit tag beats the positional fallback ===")
|
||||
# After a LAUNCH trim the relay remaps seat ids: host 2 can be roster position 3.
|
||||
# The relay now prints tag='...' on REGISTERED/dropped; the monitor must use it.
|
||||
if len(tags) > 1:
|
||||
# relay says host 2, but the tag names row 1 -- the trimmed truth
|
||||
feed("[relay] PLAYER 2 SEATED (host 2) tag='%s' callsign='TRIMMED' mech='thor'"
|
||||
% tags[1])
|
||||
feed("[relay] game[5.5.5.5:5 host=2] REGISTERED (1/1) tag='%s'" % tags[1])
|
||||
check("explicit tag lights the RIGHT row", light(1) == btoperator.ST_REGISTERED,
|
||||
str(light(1)))
|
||||
check("row 0 not falsely lit", light(0) != btoperator.ST_REGISTERED,
|
||||
str(light(0)))
|
||||
feed("[relay] game[5.5.5.5:5 host=2] dropped: closed tag='%s'" % tags[1])
|
||||
check("explicit tag clears the RIGHT row", light(1) == btoperator.ST_IDLE,
|
||||
str(light(1)))
|
||||
check("the departed name cleared on the right row too",
|
||||
callsign(1) != "TRIMMED", callsign(1))
|
||||
# old-format line (no tag=): the positional fallback must still work
|
||||
feed("[relay] PLAYER 1 SEATED (host 2) tag='%s' callsign='OLDFMT' mech='lok2'"
|
||||
% tags[0])
|
||||
feed("[relay] game[5.5.5.5:5 host=2] REGISTERED (1/1)")
|
||||
check("legacy line (no tag) still resolves positionally",
|
||||
light(0) == btoperator.ST_REGISTERED, str(light(0)))
|
||||
feed("[relay] game[5.5.5.5:5 host=2] dropped: closed")
|
||||
|
||||
print("\n=== stash survives NOTHING it should not: stopped-while-occupied ===")
|
||||
# A session stopped while a seat is occupied never pops the stash. The next
|
||||
# session must not restore last session's snapshot -- _start_session clears it.
|
||||
feed("[relay] PLAYER 1 SEATED (host 2) tag='%s' callsign='GHOST' mech='lok2'"
|
||||
% tags[0])
|
||||
check("occupied at 'session stop'", callsign(0) == "GHOST", callsign(0))
|
||||
# simulate the parts of _start_session that matter (no relay spawn)
|
||||
win._seat_defaults.clear()
|
||||
win.monitor = btoperator.SessionMonitor(True, tags)
|
||||
win.table.item(0, 1).setText("FRESH_CFG") # operator reconfigures
|
||||
feed("[relay] PLAYER 1 SEATED (host 2) tag='%s' callsign='NEWSESSION' mech='thor'"
|
||||
% tags[0])
|
||||
feed("[relay] game[8.8.8.8:8 host=2] dropped: closed")
|
||||
check("post-restart empty seat shows the NEW config, not GHOST",
|
||||
callsign(0) == "FRESH_CFG", callsign(0))
|
||||
|
||||
print("\n=== stash is TAG-keyed: an egg reload makes old keys inert ===")
|
||||
win._seat_defaults["10.99.9.9:9999"] = ("STALE", "lok2") # a tag not in this roster
|
||||
feed("[relay] PLAYER 1 SEATED (host 2) tag='%s' callsign='ABC' mech='lok2'" % tags[0])
|
||||
feed("[relay] game[8.8.8.8:8 host=2] dropped: closed")
|
||||
check("a stale foreign-tag stash never touches a live row",
|
||||
callsign(0) == "FRESH_CFG", callsign(0))
|
||||
|
||||
print("\n%s" % ("ALL CHECKS PASSED" if not fails else "FAILURES: %s" % fails))
|
||||
sys.exit(1 if fails else 0)
|
||||
|
||||
+17
-2
@@ -900,6 +900,11 @@ class Relay:
|
||||
self._drop_game(conn, "silent for %.0fs while staging -- reaped"
|
||||
% (now - conn.last_seen))
|
||||
|
||||
def _tag_of(self, host_id):
|
||||
"""The LIVE roster tag for a host id ('?' if out of range)."""
|
||||
i = host_id - FIRST_GAME_HOST_ID
|
||||
return self.roster[i] if 0 <= i < len(self.roster) else "?"
|
||||
|
||||
def _pods_ready(self):
|
||||
"""REAL pods = console connections that ACKED the egg (scanners never
|
||||
speak the protocol, so they can't hold a roster slot)."""
|
||||
@@ -1449,8 +1454,14 @@ class Relay:
|
||||
conn.host_id = host_id
|
||||
self.by_host[host_id] = conn
|
||||
self.seat_reservations.pop(host_id, None) # claim clears reserve
|
||||
# tag='...' rides along for the GUI: its positional hostID->tag map
|
||||
# is computed against the UNTRIMMED egg roster, so after a LAUNCH
|
||||
# trim remaps seat ids the GUI would light (and, since the roster
|
||||
# display fix, clear) the WRONG row. The relay's roster here is the
|
||||
# live -- possibly trimmed -- one, so this tag is the true identity.
|
||||
print(f"[relay] {conn.name()} REGISTERED "
|
||||
f"({len(self.by_host)}/{len(self.roster)})", flush=True)
|
||||
f"({len(self.by_host)}/{len(self.roster)}) "
|
||||
f"tag='{self._tag_of(host_id)}'", flush=True)
|
||||
if len(self.by_host) >= len(self.roster):
|
||||
self._release_eggs() # fallback; normally already fired
|
||||
# PEER_UP exchange: newcomer learns everyone; everyone learns newcomer.
|
||||
@@ -1537,7 +1548,11 @@ class Relay:
|
||||
# the full state restore happens in _maybe_reset_round as they drop
|
||||
|
||||
def _drop_game(self, conn, why):
|
||||
print(f"[relay] {conn.name()} dropped: {why}", flush=True)
|
||||
# tag='...' for registered conns, same reason as the REGISTERED print:
|
||||
# the GUI must not resolve a remapped post-trim hostID positionally.
|
||||
drop_tag = (f" tag='{self._tag_of(conn.host_id)}'"
|
||||
if conn.host_id is not None else "")
|
||||
print(f"[relay] {conn.name()} dropped: {why}{drop_tag}", flush=True)
|
||||
try:
|
||||
self.sel.unregister(conn.sock)
|
||||
except (KeyError, ValueError):
|
||||
|
||||
+25
-10
@@ -60,10 +60,16 @@ class SessionMonitor:
|
||||
"""Turns btconsole/relay stdout lines into per-pilot state + stats."""
|
||||
|
||||
RE_RELAY_EGG = re.compile(r"pod ACK from .*?\((\d+)/(\d+) ready\)")
|
||||
RE_RELAY_REG = re.compile(r"game\[.* host=(\d+)\] REGISTERED")
|
||||
# REGISTERED/dropped carry an explicit tag='...' since 2026-07-26: the
|
||||
# positional hostID->tag fallback is computed against the UNTRIMMED egg
|
||||
# roster, so after a LAUNCH trim remaps seat ids it names the wrong row.
|
||||
# Prefer the relay's own tag; keep the fallback for old logs.
|
||||
RE_RELAY_REG = re.compile(
|
||||
r"game\[.* host=(\d+)\] REGISTERED(?:.*?tag='([^']*)')?")
|
||||
RE_RELAY_RUN = re.compile(r"RunMission #(\d+) sent")
|
||||
RE_RELAY_STAT = re.compile(r"\[relay-stats\] (.*)")
|
||||
RE_RELAY_DOWN = re.compile(r"game\[.* host=(\d+)\] dropped")
|
||||
RE_RELAY_DOWN = re.compile(
|
||||
r"game\[.* host=(\d+)\] dropped(?:.*?tag='([^']*)')?")
|
||||
RE_RELAY_WAITOP = re.compile(r"WAITING FOR OPERATOR")
|
||||
RE_RELAY_SEAT = re.compile(
|
||||
r"PLAYER (\d+) SEATED \(host \d+\) tag='([^']*)' "
|
||||
@@ -154,14 +160,14 @@ class SessionMonitor:
|
||||
changed = True
|
||||
m = self.RE_RELAY_REG.search(line)
|
||||
if m:
|
||||
tag = self.host_tag(int(m.group(1)))
|
||||
if tag and self.state.get(tag) != ST_READY:
|
||||
tag = m.group(2) or self.host_tag(int(m.group(1)))
|
||||
if tag in self.state and self.state.get(tag) != ST_READY:
|
||||
self.state[tag] = ST_REGISTERED
|
||||
changed = True
|
||||
m = self.RE_RELAY_DOWN.search(line)
|
||||
if m:
|
||||
tag = self.host_tag(int(m.group(1)))
|
||||
if tag and self.state.get(tag) != ST_IDLE:
|
||||
tag = m.group(2) or self.host_tag(int(m.group(1)))
|
||||
if tag in self.state and self.state.get(tag) != ST_IDLE:
|
||||
self.state[tag] = ST_IDLE
|
||||
# ALSO forget the walk-up identity. "PLAYER n LEFT" -- the
|
||||
# other line that clears a seat -- is only printed by the
|
||||
@@ -724,6 +730,7 @@ class Operator(QMainWindow):
|
||||
# ------------------------------------------------------------ egg file --
|
||||
|
||||
def _load_egg_into_ui(self):
|
||||
self._seat_defaults.clear() # the table is about to be rebuilt
|
||||
m = self.egg.mission()
|
||||
for combo, key in ((self.f_map, "map"), (self.f_time, "time"),
|
||||
(self.f_weather, "weather"),
|
||||
@@ -812,6 +819,10 @@ class Operator(QMainWindow):
|
||||
|
||||
def _start_session(self):
|
||||
self.end_sent = False
|
||||
# A session stopped while a seat was OCCUPIED leaves its stash entry
|
||||
# behind (the seat never emptied, so it was never popped); a new
|
||||
# session must not restore last session's snapshot.
|
||||
self._seat_defaults.clear()
|
||||
relay_host = self.f_relayhost.text().strip()
|
||||
if (self.mode.currentIndex() == 0 and relay_host
|
||||
and relay_host.lower() not in ("localhost", "127.0.0.1")):
|
||||
@@ -1107,16 +1118,20 @@ class Operator(QMainWindow):
|
||||
# Occupied. On the FIRST refresh of this occupancy, stash what
|
||||
# the operator had configured so we can put it back later. Doing
|
||||
# it per-occupancy (rather than once at load) means an edit the
|
||||
# operator makes while a seat is empty is respected.
|
||||
if r not in self._seat_defaults:
|
||||
self._seat_defaults[r] = (
|
||||
# operator makes while a seat is empty is respected. Keyed by
|
||||
# TAG, not row index: rows shift when an egg is reloaded or the
|
||||
# seat count changes, and a stale row-keyed stash would restore
|
||||
# the WRONG name into whatever row now sits at that index -- a
|
||||
# changed roster simply makes tag keys inert instead.
|
||||
if tag not in self._seat_defaults:
|
||||
self._seat_defaults[tag] = (
|
||||
cell.text() if cell is not None else None,
|
||||
combo.currentText() if combo is not None else None)
|
||||
callsign, mech = info
|
||||
else:
|
||||
# Empty: restore the configured pilot and forget the stash, so
|
||||
# the next player to take this seat re-snapshots.
|
||||
callsign, mech = self._seat_defaults.pop(r, (None, None))
|
||||
callsign, mech = self._seat_defaults.pop(tag, (None, None))
|
||||
if callsign and cell is not None and cell.text() != callsign:
|
||||
cell.setText(callsign)
|
||||
if mech and combo is not None and combo.currentText() != mech:
|
||||
|
||||
Reference in New Issue
Block a user