J. Rogers, SE Ohio
What We Were Doing and Why
The core architectural goal was to replace a sprawling on_map_event(event, wx, wy, cam_x, cam_y, zoom, ...) plugin hook — which passed raw pygame events and pre-computed world coordinates into controller subclasses — with a clean six-method handle interface. The old system had two fundamental problems: controllers received raw input events (violating separation of concerns), and world coordinate computation was duplicated and inconsistent across every caller.
The new interface is:
get_handle_at(sx, sy)
update_handle(handle_id, sx, sy)
end_handle_drag(handle_id, sx, sy)
activate_handle(handle_id, sx, sy)
context_action(handle_id, sx, sy)
delete_active_handle()CampaignManager owns all events. Controllers own only their data and rendering. Every method works in screen space; controllers convert to world space internally using their own stored camera.
Bugs We Introduced in the Refactor (and Fixed)
1. Map image frozen — pan and zoom moved markers but not the map
Root cause: MapViewer.draw() passes its own self.cam_x/cam_y/zoom to draw_map() and draw_overlays(). In the old architecture MapViewer owned input and updated those fields directly. In the new architecture CampaignManager owns input and updated only controller.set_camera() — so MapViewer's fields were never touched after initial load. Markers moved (they used the controller's stored camera) but the map image stayed frozen at the load position.
Fix: In _apply_zoom and _pan_to in CampaignManager, after computing the new camera, also write self.map_viewer.cam_x/cam_y/zoom before calling set_camera(). This also fixed save_current_state() which was saving the stale initial values.
2. Dungeon level browser (StructureBrowser) stopped switching maps
Root cause: StructureBrowser was drawn in the LOC tab but never added to self.widgets. In the old code it received events directly through on_map_event. In the new architecture CampaignManager's widget loop calls handle_event() on everything in self.widgets. Since structure_browser was missing from that list, clicks on it were ignored.
Fix: One line — add self.structure_browser to self.widgets when active_tab == "LOC" in TacticalController.update().
3. _screen_to_world coordinate system mismatch in TacticalController
Root cause: The old _world_to_screen(wx, wy, cam_x, cam_y, zoom) took explicit camera parameters. The new version uses stored self.cam_x/cam_y/zoom. TacticalController world units are grid cells, not pixels — so the conversion must multiply by cell_size * zoom, not just zoom. The geo controller uses zoom alone.
Fix: Each controller implements its own _world_to_screen / _screen_to_world pair. GeoController: sc = zoom. TacticalController: sc = cell_size * zoom. The coord_scale property carries this into CampaignManager's pan math so _pan_to divides pixel delta by zoom * coord_scale to get the correct world delta regardless of map type.
4. Stairs markers not activating (marker_type is None)
Root cause: Old DB records for stairs were created with symbol='stairs_up' but no marker_type. The new _handle_marker_click only checked props.get('marker_type'), which returned None for all legacy markers, falling through to the "no action" log.
Fix: One line — m_type = props.get('marker_type') or props.get('symbol'). New markers are found by marker_type; legacy markers fall back to symbol.
5. get_property_updates() never called — settings not saved
Root cause: The refactor named the save method get_property_updates() in controllers, but both call sites in CampaignManager were delegating to map_viewer.save_current_state() — which calls get_metadata_updates(), a name that no longer exists in the program. Settings, grid size, slider values, and camera position were silently not persisted.
Fix: Both save_current_state() calls in CampaignManager replaced with inline saves that call controller.get_property_updates() directly, merge the result with cam state, and write to the DB via update_node().
Bugs That Existed Before the Refactor (Fixed Now)
6. Zoom was accumulating trackpad deltas and jumping to minimum
Root cause: pygame.MOUSEWHEEL event.y can be a large integer on trackpads — values of 5, 8, even 15 per event. The zoom formula self.zoom + scroll_y * ZOOM_SPEED * self.zoom with scroll_y=8 collapsed the zoom to minimum in a single swipe. This was always broken on trackpad hardware; it just wasn't noticed while the map had other problems.
Fix: step = 1 if scroll_y > 0 else -1. One step per event regardless of delta magnitude. Also removed cursor-centred zoom math entirely — zoom now happens from screen centre, which is simpler and avoids the camera drift that cursor-centred zoom introduced when the cursor was near a sidebar edge.
7. Geo overlay cells were 1×1 pixels — effectively invisible
Root cause: OverlayManager.draw() takes a cell_size argument. The geo controller always passed 1, meaning each painted cell rendered as 1 * zoom pixels wide — invisible at normal zoom levels. Political and climate overlay painting had never actually worked visually on geo maps.
Fix: GeoController now owns its own overlay renderer _draw_geo_overlays() which draws filled hex polygons or filled squares using the same geometry as the visible grid lines. Paint coordinates are stored in grid-cell space (int(wx / grid_size), int(wy / grid_size)) so one brush stroke fills one visible hex or square. Zone color is looked up live from the zone dict at draw time — recoloring a zone is instantly reflected everywhere it is painted without any repainting.
8. Middle-click context query was wired to nothing
Root cause: The action "query_point" was being fired by CampaignManager but had no entry in _action_handlers, so it was logged as unknown and discarded. The old MapViewer.handle_input() handled this directly; the refactor moved input ownership to CampaignManager but never added the handler.
Fix: _on_query_point added to the dispatch table. Converts screen coords to world coords via controller._screen_to_world(), then calls map_viewer._show_context_popup(wx, wy) which runs ContextQueryEngine and opens the blocking ContextPopup.
9. Overlay query_point broke when grid_size changed
Root cause: OverlayManager.query_point(world_x, world_y) does int(world_x), int(world_y) as the key lookup. But geo paint data is now stored at int(wx / grid_size), int(wy / grid_size). When grid_size is anything other than 1, the query key never matches any paint key. This was a direct consequence of fix #7 — the coordinate spaces were consistent within paint and draw but the query was still operating in raw pixel space.
Fix: In GeoController.__init__, wrap overlay_manager.query_point with a closure that divides incoming world coords by self.grid_size before delegating to the original method. ContextQueryEngine calls query_point directly — the wrap intercepts it transparently without touching any shared code.
10. Stairs with no portal_to link did nothing
Root cause: transition_tactical_map() checked for portal_to and silently returned if it was absent. Stairs placed by the AI generator or manually created were never automatically linked to the level they should lead to. This was a pre-existing gap — the routing existed, the generation didn't.
Fix: When portal_to is missing, transition_tactical_map now generates a new sibling dungeon_level as a child of the same parent, links the stairs marker forward to the new level, and transitions. The existing back-link repair in _set_controller wires the return staircase on arrival.
Summary
| # | Bug | Where | Category |
|---|---|---|---|
| 1 | Map image frozen on pan/zoom | CampaignManager | Introduced by refactor |
| 2 | StructureBrowser not receiving events | TacticalController | Introduced by refactor |
| 3 | Wrong coordinate scale in tactical | TacticalController | Introduced by refactor |
| 4 | Stairs marker_type None on legacy data | TacticalController | Introduced by refactor |
| 5 | get_property_updates never called | CampaignManager | Introduced by refactor |
| 6 | Trackpad zoom jumping to minimum | CampaignManager | Pre-existing |
| 7 | Geo overlay cells 1px, invisible | GeoController | Pre-existing |
| 8 | query_point action unhandled | CampaignManager | Introduced by refactor |
| 9 | Overlay query broken after grid_size change | GeoController | Introduced by fix #7 |
| 10 | Stairs with no portal_to do nothing | CampaignManager | Pre-existing |
Five bugs introduced by the refactor, five pre-existing. The refactor created the conditions to find and fix the pre-existing ones precisely because the clean separation of concerns made the broken assumptions visible.
No comments:
Post a Comment