From 8109db2f398ad16baf7dda884f50b5aa9a8c8a18 Mon Sep 17 00:00:00 2001 From: Dominik Roth Date: Thu, 13 Aug 2026 19:19:43 +0200 Subject: [PATCH] Accept a proposal using its detected marker id, not always a letter detected_id (map_vision.read_marker_id) was wired into ground-truth logging but never actually consumed when accepting a proposal -- every accepted target/ally silently got an auto-assigned A/B/C letter regardless of what number the game itself shows for that unit. _accept_proposal now prefers detected_id when present, falling back to auto-assign on a collision (two markers misread to the same id, or a real id that happens to match one already assigned) -- a duplicate id is worse than losing traceability to the game's own number for that one accept. Targets/allies keep their own separate id namespace, same as auto-assignment already does elsewhere. Co-Authored-By: Claude Sonnet 5 --- src/fenigma/app.py | 21 ++++++++-- tests/test_app_accept_proposal.py | 68 +++++++++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 3 deletions(-) create mode 100644 tests/test_app_accept_proposal.py diff --git a/src/fenigma/app.py b/src/fenigma/app.py index 8d17051..3ff067c 100644 --- a/src/fenigma/app.py +++ b/src/fenigma/app.py @@ -748,10 +748,25 @@ class MainWindow(Adw.ApplicationWindow): return if type_ is None: type_ = icons.target_type_from_icon(proposal.unit) or TargetType.UNKNOWN - if proposal.side == "friendly": - self.board.add_ally(type_, coord) + is_ally = proposal.side == "friendly" + # Prefer the marker's own detected "#" id (map_vision.read_marker_id, + # already past its own confidence floor by the time it's non-None) + # over an auto-assigned letter -- lets an accepted unit's id match + # what's actually on screen, rather than every accept silently + # becoming A/B/C regardless of what the game itself calls it. + # Falls back to auto-assign (id_=None) on a collision: two markers + # misread to the same id, or a real id that just happens to match + # one already auto-assigned, either way a duplicate id is worse + # than losing this one accept's traceability to the game's own + # number. + id_ = proposal.detected_id + existing = {a.id for a in self.board.allies} if is_ally else {t.id for t in self.board.targets} + if id_ in existing: + id_ = None + if is_ally: + self.board.add_ally(type_, coord, id_=id_) else: - self.board.add_target(type_, coord) + self.board.add_target(type_, coord, id_=id_) proposal.accepted = True proposal.confirmed_type = type_.name diff --git a/tests/test_app_accept_proposal.py b/tests/test_app_accept_proposal.py new file mode 100644 index 0000000..c2777a9 --- /dev/null +++ b/tests/test_app_accept_proposal.py @@ -0,0 +1,68 @@ +"""_accept_proposal: an accepted proposal's entity id should prefer the +marker's own detected "#" id (map_vision.read_marker_id, via +Proposal.detected_id) over an auto-assigned letter, so ids on the board +match what's actually on screen -- falling back to auto-assign only when +there's no detection, or it collides with an id already used in that +group (see _accept_proposal's own docstring). + +Needs a real Adw/Gtk init (MainWindow.__new__ skips __init__, so no +window/widgets are actually built, but Adw.init() is still required for +the module import chain), same pattern proven in this repo already for +headlessly exercising GTK-adjacent code. +""" +import gi + +gi.require_version("Gtk", "4.0") +gi.require_version("Gdk", "4.0") +gi.require_version("Adw", "1") +from gi.repository import Adw # noqa: E402 + +Adw.init() + +from fenigma.app import MainWindow # noqa: E402 +from fenigma.map_import import Proposal # noqa: E402 +from fenigma.models import Board # noqa: E402 + + +def _window() -> MainWindow: + win = MainWindow.__new__(MainWindow) # skip __init__: no widgets needed for this + win.board = Board() + return win + + +def _proposal(detected_id=None, side="hostile", sub_x=0, sub_y=0) -> Proposal: + return Proposal(side=side, label="G8", sub_x=sub_x, sub_y=sub_y, unit=None, + centre=(0, 0), box=(0, 0, 0, 0), detected_id=detected_id) + + +def test_accept_uses_the_detected_id_when_present(): + win = _window() + win._accept_proposal(_proposal(detected_id="8")) + assert win.board.targets[0].id == "8" + + +def test_accept_falls_back_to_auto_id_with_no_detection(): + win = _window() + win._accept_proposal(_proposal(detected_id=None)) + assert win.board.targets[0].id == "A" + + +def test_accept_falls_back_to_auto_id_on_a_detected_id_collision(): + win = _window() + win._accept_proposal(_proposal(detected_id="8", sub_x=1)) + win._accept_proposal(_proposal(detected_id="8", sub_x=2)) # same detected id, must not collide + ids = [t.id for t in win.board.targets] + assert ids[0] == "8" + assert ids[1] != "8" + + +def test_accept_keeps_target_and_ally_id_detection_in_separate_namespaces(): + """A detected id colliding with an existing ALLY id shouldn't force a + TARGET accept to fall back -- targets/allies are already a separate + id namespace everywhere else (Board.add_target/add_ally), detected-id + preference shouldn't quietly merge them.""" + win = _window() + win._accept_proposal(_proposal(detected_id="8", side="friendly")) + win._accept_proposal(_proposal(detected_id="8", side="hostile")) + assert win.board.allies[0].id == "8" + assert win.board.targets[0].id == "8"