Fix id-namespace regression, add StrikeRequest type, kill full-panel
rerender on assign/alive/shell, add Windows build tooling - Board.add_target/add_ally's id auto-assignment used a bare next(c for c in string.ascii_uppercase if c not in used), which raises StopIteration once 26 entities of a group exist -- a real crash confirmed via a live traceback, and a direct regression from moving that sequence from per-type to per-group. This was the actual cause of "Accept as"/"Accept all" silently doing nothing. Fixed with _next_free_id(), which rolls over to two-letter ids instead of raising. - New TargetType.STRIKE_REQUEST: the bearing/distance-offset "taking fire" fire-support request (see the earlier two-entity split) now creates this instead of reusing STRIKE, so a radioed-in request is never confused with a strike the player placed themselves. Same crosshair icon, excluded from type pickers/dedupe like STRIKE. - The "Accept as..." popover on a detected map marker now uses the same icon grid the entity-edit "Change type" popover does (was a plain unfiltered text list of every TargetType, which also wrongly offered STRIKE/STRIKE_REQUEST as pickable). - Firing panel: _cycle_assignment/_toggle_alive/_pick_shell no longer route through app.py's full solver+dedupe+canvas+panel refresh -- none of the three can affect the solver or dedupe, and none change which cards exist or their order (except _toggle_alive in hide/sort_later mode). New FiringPanel._rebuild_one() rebuilds just the one changed card; on_visual_change is a new, lighter callback (just a map redraw) for the two of these three that actually affect it. This was a real, confirmed lag source with many units on the board: every click on any of these was previously rebuilding every card of every target. - Map right-click entity menu: added "Mark destroyed"/"Mark alive", reusing the same cheap-refresh path (new FiringPanel.refresh_after_alive_change). - packaging/windows/: a from-scratch (untested against a real boot) MSYS2 + WiX .msi build pipeline for Windows, driven from Linux via dockur/windows (KVM-in-container), no Windows machine or GitHub required. See its own README for status/caveats. - New/updated tests: id-namespace sharing + the 26-entity overflow regression (tests/test_models.py), StrikeRequest split (tests/test_ocr.py), warp_to_map's img_scale param (tests/test_map_vision_warp.py). 44/44 passing. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -3,17 +3,26 @@
|
||||
Status legend: [x] fixed+tested, [~] partially addressed, [ ] open/needs input
|
||||
|
||||
- [x] Allies and enemies seem to share indices.
|
||||
Investigated: `Board.add_target`/`add_ally` already use fully separate
|
||||
id namespaces by design (see `models.py`'s `Ally`/`Target` docstrings),
|
||||
confirmed with a new regression test
|
||||
(`test_ally_and_target_ids_are_independent_namespaces`). What was
|
||||
probably actually seen: an ally and a hostile target of the same type
|
||||
display with the *same name* ("Tank#1") on the map with no visual
|
||||
"ally" cue beyond icon/side color — related to the next item, which
|
||||
fixes one concrete instance of that (TargetType.ENEMY's "Enemy" label
|
||||
on an Ally). If the symptom persists after that, it's a display/
|
||||
legibility issue, not an id collision — happy to take a screenshot of
|
||||
what's confusing.
|
||||
First pass on this was wrong: I only checked that targets and allies
|
||||
are separate id namespaces (they are, always were) and stopped there.
|
||||
The actual bug was one level down: `Board.add_target`/`add_ally`'s
|
||||
auto-id assignment (`used = {t.id for t in self.targets if t.type ==
|
||||
type_}`) was scoped **per type**, not per group — a Tank and an
|
||||
Infantry auto-added back to back both got id "A", each type getting
|
||||
its own independent A/B/C... sequence instead of sharing one across
|
||||
the whole group. Fixed: the id namespace split is targets-vs-allies
|
||||
ONLY, type never subdivides it further. New regression test
|
||||
(`test_auto_id_is_shared_across_types_within_targets_and_within_allies`).
|
||||
- [x] Regression FROM the fix above, caught via a real traceback: sharing
|
||||
one A/B/C... sequence across a whole group (instead of per-type)
|
||||
made it much easier to actually run out of the 26 letters --
|
||||
`next(c for c in string.ascii_uppercase if c not in used)` raises
|
||||
`StopIteration` the instant all 26 are taken, silently killing
|
||||
whatever button click triggered `add_target`/`add_ally` (this is
|
||||
what "Accept as"/"Accept all" doing nothing turned out to be, see
|
||||
below). Fixed with `_next_free_id()`: rolls over to two-letter ids
|
||||
("AA", "AB", ...) instead of raising, can't run out. New test
|
||||
(`test_auto_id_survives_past_26_entities_in_one_group`).
|
||||
- [x] Ally type 'ally' is called Enemy on map title.
|
||||
`icons._target_type_label` (now public `icons.target_type_label`)
|
||||
already special-cased this for the type picker, but the map's
|
||||
@@ -73,6 +82,16 @@ Status legend: [x] fixed+tested, [~] partially addressed, [ ] open/needs input
|
||||
`solver.point_from_bearing_distance` rather than through a Clue).
|
||||
New extractors in `ocr.py`, wired into `parse_intel_blocks`'s
|
||||
`flush()`. 3 new regression tests, all passing (`tests/test_ocr.py`).
|
||||
- [x] Follow-up bug in the above: the bearing/distance-offset variant
|
||||
names TWO different places (the reporting unit's own position, and
|
||||
a separate fire point offset from it), but only produced one Target
|
||||
entity, sitting at the offset point but still labeled with the
|
||||
unit's own type/id (e.g. "Infantry#11" at a spot no infantry is
|
||||
actually at). Math itself was right; the single-entity shape wasn't.
|
||||
Now produces two entries: the original (Infantry#N etc.) keeps its
|
||||
own reported position with no shell/deadline, and a new synthetic
|
||||
`Strike#<TypeWord><id>` entry (e.g. `Strike#Infantry11`) carries the
|
||||
shell/deadline at the computed offset coord. 2 more regression tests.
|
||||
|
||||
- [x] When the user deletes/replaces the map screenshot, capture whatever
|
||||
units they confirmed as ground truth for it.
|
||||
@@ -91,18 +110,18 @@ Status legend: [x] fixed+tested, [~] partially addressed, [ ] open/needs input
|
||||
places a screenshot stops being "the active one": explicit drop, a
|
||||
new screenshot pasted straight over it, and window close.
|
||||
|
||||
## Resolved via a real traceback (not guessed)
|
||||
|
||||
- [x] "Accept as" / "Accept all" on proposed targets doing nothing.
|
||||
A real traceback from the running app nailed it: `StopIteration`
|
||||
from `Board.add_ally`'s id auto-assignment once 26 allies existed
|
||||
already (see the id-namespace regression entry above) — every
|
||||
accept attempt after that silently died before the ally/target
|
||||
ever got added, popover already closed by the time it happened.
|
||||
Fixed there; not a separate bug.
|
||||
|
||||
## Needs more scope / your input before I keep going
|
||||
|
||||
- [ ] "Accept as" button on proposed targets doesn't work.
|
||||
Read through the whole path (`app.py`'s `_open_proposal_menu`/
|
||||
`_accept_proposal`, `map_import.py`'s `Proposal`/`ScreenshotImport`,
|
||||
`grid_widget.py`'s proposal hit-testing) end to end and couldn't find
|
||||
a static defect — `map_vision.GridSolution.cell_of` already clamps
|
||||
sub_x/sub_y into 0..9 before a Proposal is even built, so the obvious
|
||||
"coord fails to construct, accept silently no-ops" theory doesn't
|
||||
hold up either. I'd need a repro (which button exactly, screenshot of
|
||||
the popover, does *anything* happen — toast, marker staying put,
|
||||
wrong type applied) to chase this further rather than guess.
|
||||
- [ ] Enemy type detection needs to be more robust; read the entity id
|
||||
label so dedup is reliable; detect death from the log.
|
||||
All three are real computer-vision/OCR feature work (better marker
|
||||
|
||||
Reference in New Issue
Block a user