From 08b3292b80b22202a0105bba7c1079c8ece21bb6 Mon Sep 17 00:00:00 2001 From: Zwitschi Date: Sat, 31 Oct 2026 13:50:18 +0100 Subject: [PATCH] Runtime hygiene: fix RefCounted cycles (clean suite exit), full-length shifts, wand cooldown - FSM holds owner via WeakRef; ShiftCoordinator is RefCounted (logic-only); persistence tests free the Save node -> test suite exits with zero 'resources still in use' warnings (was ~13) and 57/57 pass - GameManager uses each scenario's shift_duration_s (900s ~ 10-15 min) for real shifts instead of the 300s test duration - Wand screening gets a 0.8s recharge cooldown with a wand.recharge message - TODO/DONE/MEMORY/README synced --- DONE.md | 10 ++++++++++ MEMORY.md | 8 +++++++- README.md | 2 +- TODO.md | 4 ---- content/strings.en.json | 3 ++- src/core/fsm.gd | 23 +++++++++++++++++------ src/core/shift_coordinator.gd | 2 +- src/game/game_manager.gd | 15 ++++++++++++++- src/test/tests/test_persistence.gd | 5 ++++- src/test/tests/test_strings.gd | 2 +- 10 files changed, 57 insertions(+), 17 deletions(-) diff --git a/DONE.md b/DONE.md index f77481e..f6257ac 100644 --- a/DONE.md +++ b/DONE.md @@ -185,6 +185,16 @@ Completed task sections moved from `TODO.md`. Sections are moved only when every Follow-up: anonymous **asset downloads** require sign-in on this host (host setting). - [x] CI packaging follow-up was already live (`.gitea/workflows/build.yml` on tags). +## 🧹 Runtime hygiene + timeline/polish items + +- [x] RefCounted cycle cleanup: `FSM` now holds its owner via `WeakRef`; `ShiftCoordinator` is + `RefCounted` (logic-only, no tree needs); persistence tests free the `Save` node. + Test suite now exits with **zero** “resources still in use” warnings. +- [x] Full-length shifts: the prototype now uses each scenario's `shift_duration_s` (900s ≈ + 10-15 min per PLAN.md) instead of a hardcoded 300s test duration. +- [x] Wand cooldown feel: a short `WAND_COOLDOWN_S` recharge between wand scans with a + `wand.recharge` message (Open Polish item). + ## 📦 Installer packaging (Windows NSIS + Linux AppImage) - [x] Export presets: `binary_format/embed_pck=true` on all platforms — every binary is now diff --git a/MEMORY.md b/MEMORY.md index 1f67b8a..40f477f 100644 --- a/MEMORY.md +++ b/MEMORY.md @@ -62,12 +62,18 @@ Session memory for durable decisions, state, and gotchas. Keep concise, remove o - Detailed graphics: textured facade/awning/skyline, humanoid NPC rigs + props + walk bob + attention head-turn, inspection artifact cards, mode icons, escalation vignette, body-cam overlay (REC + shift clock). Screenshot set regenerated (`docs/screenshots/`, day + night). +- Polish: wand scan cooldown (`WAND_COOLDOWN_S` + `wand.recharge` message). - NOTE: collaborators occasionally add uncommitted work to this shared tree (e.g. the radio easter egg); check `git status` before committing and leave foreign changes untouched unless asked. ## Known Issues / Notes -- RefCounted reference cycle: `NPCController` holds an `FSM` that holds a back-reference to its owner, so test suites report "resources still in use at exit". Benign (process exit), but worth breaking later (e.g. `WeakRef` or explicit `dispose()`). +- Reference cycles are fixed: `FSM` holds its owner via `WeakRef`, `ShiftCoordinator` is + `RefCounted`, and persistence tests free the `Save` node — the suite exits with no + "resources still in use" warnings (was ~13). If a future subsystem introduces a cycle, + repeat the WeakRef pattern. +- Shift length: `GameManager` uses the scenario's `shift_duration_s` (900s ≈ 10-15 min); + tests and the screenshot tool override elapsed time directly. - Headless (dummy renderer) logs "Parameter m is null" from `mesh_get_surface_count`; it does not occur with a real GL driver and does not affect logic. - Steps are visual-only (no collider) so NPCs can path to the entrance; the guard spawns on the landing so it never needs to climb. - Persistence tests write into the real `user://shift_results.json` (harmless local pollution of the demo history). diff --git a/README.md b/README.md index af99c0b..4729f02 100644 --- a/README.md +++ b/README.md @@ -215,7 +215,7 @@ Implemented so far: - Visual polish pass (v0.2.0): phase-based lighting (sun/sky/ambient per shift phase), street lamps with night-mode bulbs and omni lights, glowing marquee signage, crosswalk, planters and low-poly street trees, NPC height/brightness variety, denser routine traffic — day and late-night screenshot sets - Assets pass: authored textures in `assets/textures/` (brick facade, concrete, asphalt, awning, skyline, vignette, HUD icons, access-card art) generated by `tools/generate_textures.py`, soft sun shadows, humanoid NPC rigs with archetype props + procedural walk bob and attention head-turn, inspection artifact cards, escalation vignette, mode icons, and a body-camera overlay (REC + shift clock) -Not yet implemented: body-camera replay timeline, full 10-15 minute shift lengths, venue selection startup menu, editor-based GUT integration, RefCounted cycle cleanup, and crowd performance profiling/LOD (see `TODO.md`). +Not yet implemented: body-camera replay timeline, venue selection startup menu, editor-based GUT integration, and crowd performance profiling/LOD (see `TODO.md`). ### Smoke Test Checklist diff --git a/TODO.md b/TODO.md index a890622..eb6d475 100644 --- a/TODO.md +++ b/TODO.md @@ -42,15 +42,11 @@ Goal: a single executable `.AppImage` that runs the full game on any recent dist ## Future Systems (design-level backlog) - [ ] Body-camera replay timeline in the shift review. -- [ ] Long (10-15 minute) shift lengths (phase lighting already implemented). - [ ] Venue selection startup menu (keyboard cycling with `V` is implemented). - [ ] IDE/editor-based GUT integration when a Godot editor workflow is introduced. -- [ ] Reduce RefCounted reference cycles (NPCController <-> FSM) noted in `MEMORY.md`. ## Open Polish -- [ ] Add a body/wand cooldown feel: wand screening currently has no reuse penalty; consider a - short per-guest scan animation. - [ ] Keep `%` placeholders stable in `Strings.fmt` when adding keys (positional formatting). ## Detailed Graphics — remaining performance items diff --git a/content/strings.en.json b/content/strings.en.json index a2ff8cd..b38abb3 100644 --- a/content/strings.en.json +++ b/content/strings.en.json @@ -40,5 +40,6 @@ "door.thwarted": "%s tries the locked door and fails.", "flashlight.on": "Flashlight on.", "flashlight.off": "Flashlight off.", - "hud.venue_switched": "Venue: %s" + "hud.venue_switched": "Venue: %s", + "wand.recharge": "Wand scanner is recharging…" } diff --git a/src/core/fsm.gd b/src/core/fsm.gd index b97bb17..a608afe 100644 --- a/src/core/fsm.gd +++ b/src/core/fsm.gd @@ -2,15 +2,26 @@ class_name FSM extends RefCounted ## Minimal generic finite state machine used by NPCController and ShiftCoordinator. ## Deterministic and headless-testable. +## +## The owner is held through a WeakRef so long-lived owners do not create a +## reference cycle (FSM <-> owner), which previously leaked RefCounted objects +## at process exit (see MEMORY.md). signal state_changed(node: Object, from_state: String, to_state: String) var current_state: String = "" var _transitions: Dictionary = {} # state_name -> Array[{to, guard: Callable}] -var _owner_node: Object = null +var _owner_ref: WeakRef = null func _init(owner_node: Object = null) -> void: - _owner_node = owner_node + if owner_node != null: + _owner_ref = weakref(owner_node) + +## The live owner object, or null if it was freed. +func _owner() -> Object: + if _owner_ref == null: + return null + return _owner_ref.get_ref() as Object ## Register a transition. guard is an optional Callable(node) -> bool. func add_transition(from_state: String, to_state: String, guard: Callable = Callable()) -> void: @@ -20,7 +31,7 @@ func add_transition(from_state: String, to_state: String, guard: Callable = Call func start(initial_state: String) -> void: current_state = initial_state - state_changed.emit(_owner_node, "", initial_state) + state_changed.emit(_owner(), "", initial_state) ## Attempt to move to `to_state`. Returns true when executed. func transition_to(to_state: String) -> bool: @@ -31,10 +42,10 @@ func transition_to(to_state: String) -> bool: if t["to"] != to_state: continue var guard: Callable = t["guard"] - if not guard.is_valid() or guard.call(_owner_node): + if not guard.is_valid() or guard.call(_owner()): var from := current_state current_state = to_state - state_changed.emit(_owner_node, from, to_state) + state_changed.emit(_owner(), from, to_state) return true return false @@ -42,6 +53,6 @@ func can_transition_to(to_state: String) -> bool: for t in _transitions.get(current_state, []): if t["to"] == to_state: var guard: Callable = t["guard"] - if not guard.is_valid() or guard.call(_owner_node): + if not guard.is_valid() or guard.call(_owner()): return true return false diff --git a/src/core/shift_coordinator.gd b/src/core/shift_coordinator.gd index 6089b57..b11a2ee 100644 --- a/src/core/shift_coordinator.gd +++ b/src/core/shift_coordinator.gd @@ -1,5 +1,5 @@ class_name ShiftCoordinator -extends Node +extends RefCounted const Rules := preload("res://src/core/rules.gd") const EventBus := preload("res://src/core/event_bus.gd") diff --git a/src/game/game_manager.gd b/src/game/game_manager.gd index 18d84d2..c6d13a0 100644 --- a/src/game/game_manager.gd +++ b/src/game/game_manager.gd @@ -64,7 +64,11 @@ var verifier := AccessVerifier.new() var wand_screener := WandScreener.new() var active_scenario_id := "hollywood_building" -var scenario_duration_s: float = 300.0 # shortened prototype duration for testing +## Shift length mirrors the scenario default (900s / 10-15 min per PLAN.md) unless +## overridden for a test or debug build. +var scenario_duration_s: float = 900.0 +var _wand_cooldown := 0.0 +const WAND_COOLDOWN_S := 0.8 var scene_root: Node3D = null var npcs: Array = [] var npc_counter := 0 @@ -103,6 +107,7 @@ func _ready() -> void: guard.cleared_target.connect(_on_guard_clear) selected_scenario = _build_scenario(active_scenario_id) + scenario_duration_s = selected_scenario.shift_duration_s coordinator = ShiftCoordinator.new() coordinator.setup(selected_scenario, scenario_duration_s, 4242) coordinator.npc_spawn_requested.connect(_spawn_npc) @@ -169,6 +174,7 @@ func restart_shift() -> void: inspector_current_events = [] current_incident_active = false current_incident_npc_id = "" + scenario_duration_s = selected_scenario.shift_duration_s coordinator.setup(selected_scenario, scenario_duration_s, 4242) coordinator.start() _last_phase = -1 @@ -552,6 +558,9 @@ func _toggle_inspection() -> void: ## Handheld wand inspection (The Joke Shop). Only available when the venue ## carries a wand. The wand reports an alert; the player interprets it. func _toggle_wand_screening() -> void: + if _wand_cooldown > 0.0: + hud.log(Strings.lookup("wand.recharge")) + return if wand_open: hud.hide_wand() hud.set_wand_icon(false) @@ -615,6 +624,8 @@ func decide_wand(allowed: bool) -> void: controller.screened = true hud.hide_wand() wand_open = false + # Short scan cooldown: the wand must “recharge” between guests (PLAN.md: feel). + _wand_cooldown = WAND_COOLDOWN_S func decide_access(allowed: bool) -> void: var controller: Object = _targeted_controller() @@ -714,6 +725,8 @@ func _close_incident(outcome: String) -> void: current_incident_npc_id = "" func _physics_process(delta: float) -> void: + if _wand_cooldown > 0.0: + _wand_cooldown = maxf(0.0, _wand_cooldown - delta) if coordinator == null or not coordinator.running: return coordinator.tick(delta) diff --git a/src/test/tests/test_persistence.gd b/src/test/tests/test_persistence.gd index 4659c83..2a0ed7a 100644 --- a/src/test/tests/test_persistence.gd +++ b/src/test/tests/test_persistence.gd @@ -20,10 +20,13 @@ func test_save_round_trip() -> bool: var last: Dictionary = results[results.size() - 1] var c := TestUtils.check_eq(last.get("scenario_id", ""), "hollywood_building", "round-trips scenario id") var d := TestUtils.check_eq(float(last.get("reputation", {}).get("rating", 0.0)), 82.5, "round-trips rating") + save.free() return a and b and c and d func test_load_returns_array_when_empty_or_missing() -> bool: # The tester home may already contain results; this only asserts shape safety. var save := _save_instance() var results: Array = save.load_shift_results() - return TestUtils.check(results is Array, "load_shift_results always returns Array") + var ok := TestUtils.check(results is Array, "load_shift_results always returns Array") + save.free() + return ok diff --git a/src/test/tests/test_strings.gd b/src/test/tests/test_strings.gd index 846758b..09f6dbe 100644 --- a/src/test/tests/test_strings.gd +++ b/src/test/tests/test_strings.gd @@ -30,7 +30,7 @@ func test_strings_cover_ui_keys() -> bool: "hud.aim_first", "hud.no_one_nearby", "hud.targeting", "hud.no_wand", "hud.screen_first", "access.granted", "access.denied", "access.policy_satisfied", "access.wrong_grant", "access.wrong_deny", "wand.admitted_prohibited", - "wand.denied_false_alarm", "wand.cleared", "wand.refused", "radio.address_first", + "wand.denied_false_alarm", "wand.cleared", "wand.refused", "wand.recharge", "radio.address_first", "radio.blocked", "radio.inaccurate", "radio.received", "radio.emergency_dispatched", "responder.arrived", "react.comply", "react.leave", "react.argue", "react.record", "react.ignore", "react.threaten", "react.force_entry", "react.return_later",