From 9872cb993052baee7ea3347f0b8b400c79e98080 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 14:13:51 +0200 Subject: [PATCH 1/6] Draw the neon components in the editor, not just at runtime Nothing under ui/ or vfx/ was marked @tool, so every hand-drawn control rendered as an empty rect in the editor. BuildMenu.tscn positions its buttons with hand-tuned pixel offsets (offset_left = -204) around a glyph identified only as `icon = 8` - laying out invisible boxes by number. @tool on the three scene-placed components that are safe to run in the editor, covering 23 of the 34 placements: IconButton (20 scenes), TouchJoystick (2), NeonPanel (1). Two of them mutate persisted properties on _ready, which under @tool would write computed values back into the .tscn every time the scene was opened, so both are guarded with Engine.is_editor_hint(): NeonPanel._fit_to_screen() rewrites all four offsets TouchJoystick._relayout() overwrites max_radius, which is @export IconButton's _gui_input is guarded too, so clicking a button on the editor canvas does not emit `pressed`. The autoload-heavy drawers (ResourceBar, PriceTag, QuestPanel, ResourceTransferList, SkillTreeView, FormationEditor - 6 to 16 GameState./Quests. references each) are left alone; they need their call sites guarded first. Verified: 172/172 headless, and a --headless --editor pass loads the project with no script errors. That pass only proves the scripts parse under @tool - it never opens a scene, so it does not exercise the in-editor _draw. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019AJMdB7iuHuLFZM1aBu3GD --- ui/IconButton.gd | 6 ++++++ ui/NeonPanel.gd | 7 +++++++ ui/TouchJoystick.gd | 6 ++++++ 3 files changed, 19 insertions(+) diff --git a/ui/IconButton.gd b/ui/IconButton.gd index 7b9fdad..75ca542 100644 --- a/ui/IconButton.gd +++ b/ui/IconButton.gd @@ -1,3 +1,4 @@ +@tool class_name IconButton extends Control @@ -142,6 +143,11 @@ func _blocked_by_modal() -> bool: func _gui_input(event: InputEvent) -> void: + # @tool runs this in the editor too, where a click on the canvas would + # emit `pressed` at whatever it is wired to. The glyph is what we want + # visible there, not the behaviour. + if Engine.is_editor_hint(): + return if _blocked_by_modal(): return if event is InputEventScreenTouch: diff --git a/ui/NeonPanel.gd b/ui/NeonPanel.gd index 55803ab..1fb1e36 100644 --- a/ui/NeonPanel.gd +++ b/ui/NeonPanel.gd @@ -1,3 +1,4 @@ +@tool class_name NeonPanel extends Control @@ -39,6 +40,12 @@ var design_centre: Vector2 = Vector2.ZERO func _ready() -> void: + # _fit_to_screen() below rewrites offset_left/right/top/bottom. Under + # @tool that runs against the editor viewport, overwriting the authored + # layout and dirtying the scene, so the card previews at its authored + # size and is only ever fitted at runtime. + if Engine.is_editor_hint(): + return # Fitting adjusts the four offsets around a fixed centre, which is only # meaningful for a card pinned to a point - both anchor pairs collapsed, # as a centred dialog's are. A panel stretched between different anchors diff --git a/ui/TouchJoystick.gd b/ui/TouchJoystick.gd index 78056a6..a16c48e 100644 --- a/ui/TouchJoystick.gd +++ b/ui/TouchJoystick.gd @@ -1,3 +1,4 @@ +@tool class_name TouchJoystick extends Control @@ -40,6 +41,11 @@ var knob_pos: Vector2 = Vector2.ZERO func _ready() -> void: + # _relayout() writes max_radius, which is @export - under @tool that + # would persist a computed value into the scene. The preview uses the + # authored radius instead; only a running game refits. + if Engine.is_editor_hint(): + return # Dialogs release every spring-loaded stick through one compositional # contract, without NarrationBanner knowing scene paths or joystick roles. add_to_group("touch_joysticks") From 693919dadbe2b8daa9600eccef5f18a13e92a51d Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 22:31:56 +0200 Subject: [PATCH 2/6] Let a dialog opened over another dialog be closed The "no research data" explainer is the only dialog in the game that opens ON TOP of another one - the skill tree or the ship lab, from the button that was just refused. IconButton._blocked_by_modal() refuses input to any button sitting behind an open panel, and it had no notion of stacking order: it saw the lab still visible, saw it was not an ancestor of the popup's close button, and killed the touch. Every button in the explainer was dead, including the only way out of it. Reported from the game. The check now compares draw order. A panel that this button's own panel is drawn on top of is behind it, not over it, and is ignored; a panel drawn above it still blocks, so the lab's own buttons stay dead while the popup covers them. Buttons belonging to no panel - the HUD - are unchanged and still blocked by anything open. The existing test missed this because it dismissed the popup by calling close_menu() directly, which is not what a player can do. Case D presses the button, and asserts the reverse too. Verified in both directions: with the fix reverted it fails with "the explainer close button is blocked by the lab behind it - the dialog is inescapable". 172/172. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019AJMdB7iuHuLFZM1aBu3GD --- tests/test_research_points_explainer.gd | 28 +++++++++++++++++++ ui/IconButton.gd | 36 +++++++++++++++++++++++-- 2 files changed, 62 insertions(+), 2 deletions(-) diff --git a/tests/test_research_points_explainer.gd b/tests/test_research_points_explainer.gd index aaef71a..6fc638e 100644 --- a/tests/test_research_points_explainer.gd +++ b/tests/test_research_points_explainer.gd @@ -113,6 +113,10 @@ func _process(delta: float) -> bool: break if game_state.research_point_cost(String(options[controller.lab_choice].id)) <= 0: return _fail("no lab option costs research data, so this proves nothing") + # The lab panel is open when the player presses buy - that is what + # puts the explainer on top of another dialog, which is the whole + # point of case D below. + controller.lab_menu.open_menu() controller._on_lab_buy() if not popup.visible: return _fail("the lab refused for want of data with no explanation") @@ -120,6 +124,30 @@ func _process(delta: float) -> bool: return _fail("the lab still blames the queue: '%s'" % controller.lab_menu.status.text) print("ship lab: refused and explained, not blamed on the queue") + # --- D. and the player can get back out of it ---------------- + # The explainer is the only dialog that opens ON TOP of another + # one, and IconButton refuses input to any button sitting behind + # an open panel. That used to include this popup's own close + # button, so the dialog could not be dismissed at all. Case B + # above left the ship lab open underneath, which is the real + # situation - assert that rather than arranging it. + if not controller.lab_menu.visible: + return _fail("the lab is not open underneath, so this proves nothing") + var close: Control = popup.get_node("CloseButton") + if close._blocked_by_modal(): + return _fail("the explainer close button is blocked by the lab behind it - the dialog is inescapable") + close.pressed.emit() + if popup.visible: + return _fail("pressing close left the explainer open") + # The reverse must still hold: the lab's own buttons stay dead + # while a dialog covers them. + popup.open_for_shortfall(10, "anything") + var lab_close: Control = controller.lab_menu.get_node("CloseButton") + if not lab_close._blocked_by_modal(): + return _fail("the lab buttons are live underneath an open dialog") + popup.close_menu() + print("close button works over the lab, and the lab stays blocked underneath") + print("TEST_RESEARCH_POINTS_EXPLAINER: PASS") quit(0) return true diff --git a/ui/IconButton.gd b/ui/IconButton.gd index 75ca542..c6af89c 100644 --- a/ui/IconButton.gd +++ b/ui/IconButton.gd @@ -130,6 +130,7 @@ func _blocked_by_modal() -> bool: # touches through the GUI system swallows them before they ever get # there. So the panels stay transparent to input and the buttons check # instead. + var own_modal: Node = _own_modal() for menu in get_tree().get_nodes_in_group("modal_menus"): # is_visible_in_tree(), NOT visible. Hiding a parent leaves the # child's own `visible` flag true, and the build menu's info popup is @@ -137,11 +138,42 @@ func _blocked_by_modal() -> bool: # invisible popup still claiming to be open, and every button in the # game was blocked from then on. That is not a state a player can # get out of. - if menu.is_visible_in_tree() and not menu.is_ancestor_of(self): - return true + if not menu.is_visible_in_tree(): + continue + if menu.is_ancestor_of(self): + continue + # Stacking order matters, and used not to be considered at all. A + # panel that this button's own panel is drawn ON TOP OF sits behind + # it, not over it, so it must not swallow the input. The research + # shortfall popup opens over the skill tree and the ship lab, and + # without this every button in it - its close button included - was + # dead, leaving the player no way out of the dialog. + if own_modal != null and not _covers(menu, own_modal): + continue + return true return false +# The modal panel this button lives inside, or null for a HUD button that +# belongs to no panel (those stay blocked by any open panel, as before). +func _own_modal() -> Node: + var node: Node = get_parent() + while node != null: + if node.is_in_group("modal_menus"): + return node + node = node.get_parent() + return null + + +func _covers(other: Node, own: Node) -> bool: + # Later siblings draw on top. Different parents means one of them is + # nested inside something else - the build menu's info popup is a child + # of the build menu - and there the conservative answer is "covered". + if other.get_parent() != own.get_parent(): + return true + return other.get_index() > own.get_index() + + func _gui_input(event: InputEvent) -> void: # @tool runs this in the editor too, where a click on the canvas would # emit `pressed` at whatever it is wired to. The glyph is what we want From aaf8b400efcb9ca06d3437fe7ab7c74b04c33926 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 22:35:41 +0200 Subject: [PATCH 3/6] Build the accretion disc once per spawn, not twice Planet.gd calls visual.configure() BEFORE add_child(), and the SceneTree defers _ready() to the first frame - so configure() built the five rings, and _ready() then threw them away and built five more. Eighty antialiased polylines tessellated per black hole instead of forty, plus five nodes queued for free. Confirmed by instrumenting _build(): _build() called, existing children=0 <- configure() _build() called, existing children=5 <- _ready() _ready() now uses the same is-it-built guard configure() already had. Note for anyone chasing the same thing: this is NOT the disc's steady-state cost. Measured, the geometry rasterises zero times over sixty frames (the NeonPiece refactor works), and rendering the disc costs 0.1 ms against a 133 ms softpipe floor. The disc is cheap once it exists. 172/172. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019AJMdB7iuHuLFZM1aBu3GD --- planets/BlackHoleVisual.gd | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/planets/BlackHoleVisual.gd b/planets/BlackHoleVisual.gd index ab0cb0e..ea664ac 100644 --- a/planets/BlackHoleVisual.gd +++ b/planets/BlackHoleVisual.gd @@ -26,7 +26,12 @@ var _rings: Array = [] func _ready() -> void: - _build() + # Planet.gd calls configure() BEFORE add_child(), so the disc is usually + # already built by the time _ready fires (which the SceneTree defers to + # the first frame). Without this guard the ring set was thrown away and + # re-tessellated - eighty antialiased polylines per spawn instead of forty. + if _rings.is_empty(): + _build() func configure(_shape_seed: int) -> void: From 37e2e89cd326108ea800b12f981cb89cd27fd3dd Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 9 Sep 2026 11:40:28 +0200 Subject: [PATCH 4/6] Stop the first play of each sound paying for its own load Reported from the game: sounds sometimes start a bit late. The samples were the suspect, so they were measured first - and they are clean. True digital silence at the head of each file: seven of nine start on sample 1 sfx_gas_blast 0.2ms sfx_black_hole 1.5ms, then a deliberate swell The black hole's slow attack is the sound it was written to be - make_sfx.py says it "swells across most of its length instead of decaying from the first sample" - so there is nothing to trim without breaking it. The delay was in the playback path, and "sometimes" was the clue: play() called load() every time. A cache hit after the first, but the FIRST play of each sound was a synchronous read and decode on the frame the sound was asked for. Ten sounds, ten occasional late starts. _pool() built its twelve AudioStreamPlayer nodes lazily, on the first play() call - one bigger stall, every session. The catalog is preloaded now, so play() never touches the disk, and Sfx.warm() builds the voices next to Music.ensure() at startup. Case F pins the thing that was originally suspected, since make_sfx.py regenerates these and a lazy envelope would hide the delay somewhere nobody would look. It reads the source .wav's RIFF chunks rather than the imported stream: these import as QOA (compress/mode=2), so AudioStreamWAV.data is not PCM. The first version of that check was vacuous for exactly that reason and passed at a 0.5ms threshold; it now reports "9 samples checked, worst attack sfx_black_hole.wav at 1.5ms" and fails when tightened. Not touched: project.godot has no [audio] section, so output_latency is at its default. On Android that buffer is usually the dominant latency, but lowering it risks crackle on real hardware - worth doing with someone listening, not blind. 172/172. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019AJMdB7iuHuLFZM1aBu3GD --- audio/Sfx.gd | 38 +++++++++---- controllers/GameController.gd | 3 + tests/test_sfx.gd | 100 +++++++++++++++++++++++++++++++++- ui/MainMenu.gd | 3 + 4 files changed, 130 insertions(+), 14 deletions(-) diff --git a/audio/Sfx.gd b/audio/Sfx.gd index 04be32c..a748599 100644 --- a/audio/Sfx.gd +++ b/audio/Sfx.gd @@ -34,17 +34,23 @@ const ASTEROID_BREAK: String = "asteroid_break" const GAS_BLAST: String = "gas_blast" const BLACK_HOLE: String = "black_hole" +# PRELOADED, not paths. play() used to load() on every call, which is a +# cache hit after the first time but a synchronous disk read and decode the +# first time - on the frame the sound was asked for. Ten sounds meant ten +# occasional late starts, one per sound, which is what "sometimes starting a +# bit late" was. Swapping in a different file is still just replacing the +# .wav; nothing here knows or cares which it is. const SOUNDS: Dictionary = { - SHOT: "res://audio/sfx_shot.wav", - SHIP_DEATH: "res://audio/sfx_ship_death.wav", - ENEMY_DEATH: "res://audio/sfx_ship_death.wav", - BUILDING_DEATH: "res://audio/sfx_building_death.wav", - DIALOG: "res://audio/sfx_dialog.wav", - BOSS_SCREAM: "res://audio/sfx_boss_scream.wav", - PICKUP: "res://audio/sfx_pickup.wav", - ASTEROID_BREAK: "res://audio/sfx_asteroid_break.wav", - GAS_BLAST: "res://audio/sfx_gas_blast.wav", - BLACK_HOLE: "res://audio/sfx_black_hole.wav", + SHOT: preload("res://audio/sfx_shot.wav"), + SHIP_DEATH: preload("res://audio/sfx_ship_death.wav"), + ENEMY_DEATH: preload("res://audio/sfx_ship_death.wav"), + BUILDING_DEATH: preload("res://audio/sfx_building_death.wav"), + DIALOG: preload("res://audio/sfx_dialog.wav"), + BOSS_SCREAM: preload("res://audio/sfx_boss_scream.wav"), + PICKUP: preload("res://audio/sfx_pickup.wav"), + ASTEROID_BREAK: preload("res://audio/sfx_asteroid_break.wav"), + GAS_BLAST: preload("res://audio/sfx_gas_blast.wav"), + BLACK_HOLE: preload("res://audio/sfx_black_hole.wav"), } # Per-sound level. A shot is fired constantly and a death happens once, so @@ -81,6 +87,16 @@ const PITCH_SPREAD: Dictionary = { } +# Builds the voice pool before anything needs it. _pool() is lazy, so without +# this the first sound in a session paid for twelve AudioStreamPlayer nodes +# being created on the frame it was asked for - the worst late start of all, +# and the one that happens every session. +static func warm(tree: SceneTree) -> void: + if tree == null: + return + _pool(tree) + + static func _pool(tree: SceneTree) -> Node: var existing: Node = tree.root.get_node_or_null(NODE_NAME) if existing != null: @@ -103,7 +119,7 @@ static func _pool(tree: SceneTree) -> Node: static func play(tree: SceneTree, id: String) -> void: if tree == null or not SOUNDS.has(id): return - var stream: AudioStream = load(String(SOUNDS[id])) + var stream: AudioStream = SOUNDS[id] as AudioStream if stream == null: return var pool: Node = _pool(tree) diff --git a/controllers/GameController.gd b/controllers/GameController.gd index 6eefed9..b15b5f5 100644 --- a/controllers/GameController.gd +++ b/controllers/GameController.gd @@ -114,6 +114,9 @@ func _ready() -> void: # This script is the root of BOTH the space scene and the planet # surface, so one call here covers the whole game. Music.ensure(get_tree()) + # Same moment, same reason: build the sfx voices before the first + # sound needs them rather than during it. + Sfx.warm(get_tree()) if joystick != null: joystick.direction_changed.connect(_on_joystick_direction) diff --git a/tests/test_sfx.gd b/tests/test_sfx.gd index ea90a10..fac9813 100644 --- a/tests/test_sfx.gd +++ b/tests/test_sfx.gd @@ -22,6 +22,10 @@ extends SceneTree # once, so a shot must be the quieter of the two # F. sound keeps working while a dialog has the game stopped (#128) +# 20ms is well clear of what the generator actually produces (1.5ms at +# worst, on the black hole) while still catching a real lazy attack. +const MAX_LEAD_SILENCE_MS: float = 20.0 + var game_state: Node var scene: Node var fleet: Node2D @@ -60,16 +64,76 @@ func _sounding() -> int: return n +# Every sample committed in audio/, not just the ones the catalog names. +func _wav_file_names() -> Array: + var names: Array = [] + var dir: DirAccess = DirAccess.open("res://audio") + if dir == null: + return names + for file_name in dir.get_files(): + # The editor leaves .import siblings; take the real files only. + if file_name.ends_with(".wav"): + names.append(file_name) + names.sort() + return names + + +# Leading samples that are exactly zero, in milliseconds. Negative if the +# file could not be parsed, which the caller treats as a failure rather than +# quietly passing. +func _leading_silence_ms(path: String) -> float: + var f: FileAccess = FileAccess.open(path, FileAccess.READ) + if f == null: + return -1.0 + if f.get_buffer(4).get_string_from_ascii() != "RIFF": + return -1.0 + f.get_32() + if f.get_buffer(4).get_string_from_ascii() != "WAVE": + return -1.0 + var channels: int = 0 + var rate: int = 0 + var bits: int = 0 + while f.get_position() + 8 <= f.get_length(): + var chunk_id: String = f.get_buffer(4).get_string_from_ascii() + var chunk_size: int = f.get_32() + var next: int = f.get_position() + chunk_size + (chunk_size & 1) + if chunk_id == "fmt ": + f.get_16() + channels = f.get_16() + rate = f.get_32() + f.get_32() + f.get_16() + bits = f.get_16() + elif chunk_id == "data": + if bits != 16 or channels <= 0 or rate <= 0: + return -1.0 + var silent_frames: int = 0 + var data_end: int = f.get_position() + chunk_size + while f.get_position() + 2 * channels <= data_end: + var quiet: bool = true + for c in range(channels): + if f.get_16() != 0: + quiet = false + if not quiet: + break + silent_frames += 1 + return float(silent_frames) / float(rate) * 1000.0 + f.seek(next) + return -1.0 + + func _process(delta: float) -> bool: elapsed += delta match stage: 0: # --- A. the catalog is real ----------------------------- for id in Sfx.SOUNDS: - var path: String = String(Sfx.SOUNDS[id]) - var stream: AudioStream = load(path) + # Preloaded, so this asserts the catalog is already resident + # rather than merely loadable - play() must never touch the + # disk on the frame a sound is asked for. + var stream: AudioStream = Sfx.SOUNDS[id] as AudioStream if stream == null: - return _fail("the \"%s\" sound (%s) does not load" % [id, path]) + return _fail("the \"%s\" sound is not a preloaded stream" % id) if stream.get_length() <= 0.02: return _fail("the \"%s\" sound is %.3fs long - that is silence" \ % [id, stream.get_length()]) @@ -77,6 +141,36 @@ func _process(delta: float) -> bool: return _fail("the \"%s\" sound has no level set" % id) print("%d sounds, all loading, all with a level" % Sfx.SOUNDS.size()) + # --- F. and none of them opens on silence --------------- + # Reported from the game as sounds "sometimes starting a bit + # late". Measured, the samples were clean and the cause was + # elsewhere - play() used to load() on the frame the sound was + # asked for. But make_sfx.py regenerates these, and an envelope + # that eased in from zero would put the delay back somewhere + # nobody would think to look. + # + # Reads the source .wav rather than the imported stream: these + # import as QOA (compress/mode=2), so AudioStreamWAV.data is + # not PCM and cannot be inspected sample by sample. True digital + # silence only - the black hole deliberately SWELLS (see its + # docstring in make_sfx.py) and a soft attack is a choice. + var checked: int = 0 + var worst_id: String = "" + var worst_ms: float = 0.0 + for file_name in _wav_file_names(): + var lead: float = _leading_silence_ms("res://audio/" + file_name) + if lead < 0.0: + return _fail("could not read %s to check its attack" % file_name) + checked += 1 + if lead > worst_ms: + worst_ms = lead + worst_id = file_name + if lead > MAX_LEAD_SILENCE_MS: + return _fail("%s opens with %.1fms of dead air" % [file_name, lead]) + if checked < Sfx.SOUNDS.size() - 1: + return _fail("only %d wav files checked for dead air - that proves nothing" % checked) + print("%d samples checked, worst attack %s at %.1fms" % [checked, worst_id, worst_ms]) + # --- E. and in proportion ------------------------------- if float(Sfx.LEVELS[Sfx.SHOT]) >= float(Sfx.LEVELS[Sfx.SHIP_DEATH]): return _fail("a shot is as loud as a death - one happens constantly, the other once") diff --git a/ui/MainMenu.gd b/ui/MainMenu.gd index 5b99822..ef0eaaf 100644 --- a/ui/MainMenu.gd +++ b/ui/MainMenu.gd @@ -56,6 +56,9 @@ func _ready() -> void: # #148: the theme starts here, at the first screen, and keeps playing # through every scene after it. Music.ensure(get_tree()) + # Same moment, same reason: build the sfx voices before the first + # sound needs them rather than during it. + Sfx.warm(get_tree()) play_button.pressed.connect(_on_play_pressed) editor_button.pressed.connect(_on_editor_pressed) editor_button.enabled = true From 840f9aa9a0bbd623d4bfe46f17ec7ba5574dfe16 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 9 Sep 2026 12:57:28 +0200 Subject: [PATCH 5/6] #224: refuse to build a structure on top of another BuildingField.place() dropped a structure at the fleet centroid with no overlap check of any kind, so they could be stacked. It reads worse than it looks. BuildingField decides which structure the fleet is standing on by testing the centroid against interact_radius and returning the FIRST match, so two structures inside one another make that a question of list order rather than of where the player flew - you could fly to your workshop and open the crafter. The separation is interact_radius itself (150) rather than a new number, because that is exactly the circle the ambiguity comes from. Buildings draw to a radius of roughly 60, so this leaves a visible gap rather than letting them touch. Gated in three places: the info popup (the button greys out and says which structure is in the way), _confirm_build, and _try_place - the preview goes stale as the fleet keeps moving. Ordering matters in one case. A unique structure replaces itself by demolishing the old one BEFORE placing the new one, so a placement refused afterwards would leave the player with neither. The check runs before the demolition and excludes the structure being replaced. Case D pins it. Two existing tests encoded the old permissive behaviour and now fly clear first; neither is about placement: test_build_info_popup builds every menu option from one position, so each structure blocked the next test_payments_use_fleet_hold builds a mothership on the ship lab that an earlier case injects at the origin Also fixes a bug from 37e2e89: Sfx.warm() called add_child() from _ready(), which Godot refuses outright - it printed an error and never pre-built the pool, defeating the point of warming it. Deferred, like Music.ensure(). 173/173. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019AJMdB7iuHuLFZM1aBu3GD --- audio/Sfx.gd | 8 +- controllers/BuildController.gd | 36 ++++++- core/GameState.gd | 22 +++++ tests/test_build_info_popup.gd | 11 ++- tests/test_building_separation.gd | 135 ++++++++++++++++++++++++++ tests/test_payments_use_fleet_hold.gd | 6 ++ 6 files changed, 214 insertions(+), 4 deletions(-) create mode 100644 tests/test_building_separation.gd diff --git a/audio/Sfx.gd b/audio/Sfx.gd index a748599..e8dac10 100644 --- a/audio/Sfx.gd +++ b/audio/Sfx.gd @@ -94,7 +94,13 @@ const PITCH_SPREAD: Dictionary = { static func warm(tree: SceneTree) -> void: if tree == null: return - _pool(tree) + # Deferred, like Music.ensure(): this is called from _ready(), where the + # parent is still setting up children and a direct add_child() is + # refused outright. The lazy path in play() runs during an ordinary + # frame and does not need it. + var build: Callable = func() -> void: + _pool(tree) + build.call_deferred() static func _pool(tree: SceneTree) -> Node: diff --git a/controllers/BuildController.gd b/controllers/BuildController.gd index 915fe12..c765963 100644 --- a/controllers/BuildController.gd +++ b/controllers/BuildController.gd @@ -426,8 +426,17 @@ func _show_building_info(type: String) -> void: else: lines.append("Will mine %s from %s." % [ GameState.resource_name(planet.resource_type), planet.planet_name]) - elif can_build and definition.unique and GameState.has_building_of_type(type): - lines.append("WARNING: only one may exist - this destroys your current one.") + elif can_build: + if definition.unique and GameState.has_building_of_type(type): + lines.append("WARNING: only one may exist - this destroys your current one.") + # A unique structure replaces itself, so the one being demolished is + # not in the way. + var blocker: Dictionary = GameState.building_blocking(fleet.get_centroid(), + type if definition.unique else "") + if not blocker.is_empty(): + lines.append("Too close to your %s - fly clear and try again." \ + % _building_name(String(blocker.get("type", "")))) + can_build = false if can_build and not GameState.can_afford_resources(costs, fleet.get_centroid()): can_build = false pending_build_type = type if can_build else "" @@ -468,6 +477,16 @@ func _confirm_build() -> void: planet.resource_type, planet.planet_id) build_menu.close_menu() return + # BEFORE the demolition below, not after: a unique structure replaces + # itself by removing the old one first, so a blocked placement discovered + # afterwards would leave the player with neither. The old one is excluded + # because it is the thing being replaced. + var blocker: Dictionary = GameState.building_blocking(fleet.get_centroid(), + type if definition.unique else "") + if not blocker.is_empty(): + build_menu.set_status("Too close to your %s" \ + % _building_name(String(blocker.get("type", "")))) + return if definition.unique and GameState.has_building_of_type(type): if not GameState.can_afford_resources(GameState.building_costs(type), fleet.get_centroid()): build_menu.set_status(_cannot_afford(type)) @@ -477,6 +496,11 @@ func _confirm_build() -> void: _try_place(type) +func _building_name(type: String) -> String: + var definition: Resource = BUILDING_DEFINITIONS.get_definition(type) + return String(definition.display_name) if definition != null else type.capitalize() + + func _cannot_afford(type: String) -> String: var missing: Array = [] for resource_type in GameState.building_costs(type): @@ -489,6 +513,14 @@ func _cannot_afford(type: String) -> String: func _try_place(type: String) -> void: + # Re-checked here and not only in the preview: the fleet keeps moving + # between opening the menu and confirming, and this is the one place + # every space structure is actually placed. + var blocker: Dictionary = GameState.building_blocking(fleet.get_centroid()) + if not blocker.is_empty(): + build_menu.set_status("Too close to your %s" \ + % _building_name(String(blocker.get("type", "")))) + return if not GameState.spend_resources(GameState.building_costs(type), fleet.get_centroid()): build_menu.set_status(_cannot_afford(type)) return diff --git a/core/GameState.gd b/core/GameState.gd index fcab54e..b8a8e5a 100644 --- a/core/GameState.gd +++ b/core/GameState.gd @@ -2707,6 +2707,28 @@ func has_mothership() -> bool: return get_mothership_position() != null +# Space structures may not be stacked (#224). The distance is Building's own +# interact_radius, rather than a new number: that is the circle BuildingField +# tests the fleet centroid against to decide which structure you are standing +# on, and it returns the FIRST match - so two structures inside one another +# make that a question of list order rather than of where you flew. +const MIN_BUILDING_SEPARATION: float = 150.0 + + +# The structure a new one at this position would sit on top of, or an empty +# dictionary when the spot is clear. ignore_type skips a type that is about +# to be demolished, which is how the unique structures replace themselves - +# without it a mothership could never be rebuilt where the old one stood. +func building_blocking(at_position: Vector2, ignore_type: String = "") -> Dictionary: + for building in buildings: + if ignore_type != "" and String(building.get("type", "")) == ignore_type: + continue + var at: Vector2 = building.get("position", Vector2.ZERO) + if at.distance_to(at_position) < MIN_BUILDING_SEPARATION: + return building + return {} + + func has_building_of_type(type: String) -> bool: for building in buildings: if building.type == type: diff --git a/tests/test_build_info_popup.gd b/tests/test_build_info_popup.gd index 43988be..2ea2e7b 100644 --- a/tests/test_build_info_popup.gd +++ b/tests/test_build_info_popup.gd @@ -17,6 +17,7 @@ extends SceneTree var scene: Node var game_state: Node var controller: Node +var fleet: Node2D var popup: Control var title: Label var message: Label @@ -55,6 +56,7 @@ func _initialize() -> void: scene = load("res://world/Main.tscn").instantiate() root.add_child(scene) controller = scene.get_node("BuildController") + fleet = scene.get_node("Fleet") popup = scene.get_node("UI/BuildMenu/InfoPopup") title = popup.get_node("Panel/TitleLabel") message = popup.get_node("Panel/MessageLabel") @@ -83,8 +85,15 @@ func _process(delta: float) -> bool: if elapsed < 0.2: return false - for option in OPTIONS: + for i in range(OPTIONS.size()): + var option: Dictionary = OPTIONS[i] var type: String = option.type + # #224: structures may not be stacked, and this loop BUILDS + # each option in turn. Without moving between them the second + # one is refused for sitting on the first, which is the rule + # working - but this test is about what the popup says, not + # about where things may go. + fleet.teleport_to(Vector2(float(i + 2) * GameState.MIN_BUILDING_SEPARATION * 2.0, 0.0)) var expected_cost: int = controller.cost_of(type) var rocks_before: int = game_state.rocks var built_before: int = _count(type) diff --git a/tests/test_building_separation.gd b/tests/test_building_separation.gd new file mode 100644 index 0000000..91cddce --- /dev/null +++ b/tests/test_building_separation.gd @@ -0,0 +1,135 @@ +extends SceneTree + +# Forgejo #224: a structure may not be built on top of one that is already +# there. +# +# It matters beyond the look of it. BuildingField decides which structure the +# fleet is standing on by testing the centroid against interact_radius and +# returning the FIRST match, so two structures inside one another make that a +# question of list order rather than of where the player flew - you would fly +# to your workshop and open the crafter. +# +# Covers: +# A. the spot a structure already occupies is refused +# B. clear of it, the same build goes through +# C. a unique structure can still be rebuilt where the old one stands, +# because the old one is what is being replaced +# D. and a REFUSED rebuild does not demolish the existing one first + +var game_state: Node +var scene: Node +var controller: Node +var fleet: Node2D +var elapsed: float = 0.0 +var stage: int = 0 + + +func _initialize() -> void: + game_state = root.get_node("GameState") + game_state.buildings.clear() + game_state.extractors.clear() + game_state.reset_progression() + game_state.reset_new_game() + game_state.intro_active = false + game_state.reset_quests() + game_state.rocks = 100000 + game_state.bank_inventory.set_amount(GameState.RESOURCE_GAS, 100000) + game_state.bank_inventory.set_amount(GameState.RESOURCE_ENERGY, 100000) + scene = load("res://world/Main.tscn").instantiate() + root.add_child(scene) + current_scene = scene + controller = scene.get_node("BuildController") + fleet = scene.get_node("Fleet") + + +func _process(delta: float) -> bool: + elapsed += delta + if elapsed < 0.4: + return false + + var sep: float = GameState.MIN_BUILDING_SEPARATION + + # _confirm_build() returns SILENTLY on a locked structure, which would + # make every refusal below look like the separation rule working when it + # was only the unlock gate. Unlock what this test builds, and assert it. + for type in [GameState.BUILDING_COMMAND_CENTER, GameState.BUILDING_MOTHERSHIP, + GameState.BUILDING_SHIP_CRAFTER]: + var node_id: String = game_state.research_node_for_building(type) + if node_id != "" and not game_state.is_unlocked(node_id): + game_state.unlocked.append(node_id) + if not game_state.is_building_unlocked(type): + return _fail("'%s' is still locked, so a refusal would prove nothing" % type) + + # --- A. the occupied spot is refused -------------------------------- + game_state.buildings.clear() + game_state.add_building(GameState.BUILDING_SHIP_CRAFTER, Vector2.ZERO) + + if game_state.building_blocking(Vector2.ZERO).is_empty(): + return _fail("a structure's own position reads as a clear spot") + # Just inside the separation, and just outside it. + if game_state.building_blocking(Vector2(sep - 1.0, 0.0)).is_empty(): + return _fail("a spot %.0f units away is not blocked, but the rule is %.0f" % [sep - 1.0, sep]) + if not game_state.building_blocking(Vector2(sep + 1.0, 0.0)).is_empty(): + return _fail("a spot %.0f units away is blocked, but the rule is %.0f" % [sep + 1.0, sep]) + var blocker: Dictionary = game_state.building_blocking(Vector2.ZERO) + if String(blocker.get("type", "")) != GameState.BUILDING_SHIP_CRAFTER: + return _fail("the blocking structure is reported as '%s'" % blocker.get("type", "")) + print("a spot inside %.0f units of a structure is refused, outside it is clear" % sep) + + # --- B. and the whole build path agrees ----------------------------- + # On top of the crafter: refused, and nothing is built or spent. + fleet.teleport_to(Vector2.ZERO) + var before: int = game_state.buildings.size() + controller.pending_build_type = GameState.BUILDING_COMMAND_CENTER + controller._confirm_build() + if game_state.buildings.size() != before: + return _fail("a structure was built on top of the crafter") + print("the build path refuses it too: %s" % controller.build_menu.status.text) + + # Clear of it: the same build goes through. + fleet.teleport_to(Vector2(sep * 3.0, 0.0)) + controller.pending_build_type = GameState.BUILDING_COMMAND_CENTER + controller._confirm_build() + if game_state.buildings.size() != before + 1: + return _fail("the same build was refused on a clear spot") + print("clear of it, the same build goes through") + + # --- C. a unique structure replaces itself in place ----------------- + game_state.buildings.clear() + game_state.add_building(GameState.BUILDING_MOTHERSHIP, Vector2.ZERO) + fleet.teleport_to(Vector2.ZERO) + if not game_state.building_blocking(Vector2.ZERO, GameState.BUILDING_MOTHERSHIP).is_empty(): + return _fail("a unique structure blocks its own replacement") + controller.pending_build_type = GameState.BUILDING_MOTHERSHIP + controller._confirm_build() + if game_state.buildings.size() != 1: + return _fail("rebuilding the mothership in place left %d structures" % game_state.buildings.size()) + if String(game_state.buildings[0].type) != GameState.BUILDING_MOTHERSHIP: + return _fail("the rebuilt structure is not a mothership") + print("a unique structure can still be rebuilt where the old one stood") + + # --- D. a refused rebuild demolishes nothing ------------------------ + # The mothership is unique and is removed before the new one is placed. + # Something ELSE in the way has to be caught before that demolition, or + # the player is left with neither. + game_state.buildings.clear() + var keep: int = game_state.add_building(GameState.BUILDING_MOTHERSHIP, Vector2.ZERO) + game_state.add_building(GameState.BUILDING_SHIP_CRAFTER, Vector2(sep * 4.0, 0.0)) + fleet.teleport_to(Vector2(sep * 4.0, 0.0)) + controller.pending_build_type = GameState.BUILDING_MOTHERSHIP + controller._confirm_build() + if game_state.find_building(keep).is_empty(): + return _fail("a refused rebuild demolished the existing mothership - the player is left with neither") + if game_state.buildings.size() != 2: + return _fail("a refused rebuild changed the structure count to %d" % game_state.buildings.size()) + print("a refused rebuild leaves the existing structure standing") + + print("TEST_BUILDING_SEPARATION: PASS") + quit(0) + return true + + +func _fail(message: String) -> bool: + print("TEST_BUILDING_SEPARATION: FAIL (%s)" % message) + quit(1) + return true diff --git a/tests/test_payments_use_fleet_hold.gd b/tests/test_payments_use_fleet_hold.gd index 690c9ad..24fcaf5 100644 --- a/tests/test_payments_use_fleet_hold.gd +++ b/tests/test_payments_use_fleet_hold.gd @@ -270,6 +270,12 @@ func _process(delta: float) -> bool: game_state.fleet_inventory.set_amount(GameState.RESOURCE_ORE, HOLD_ORE) var before: int = game_state.buildings.size() + # #224: structures may not be stacked, and _check_ship_lab left a + # lab standing at the origin as a prerequisite for an earlier + # case. Fly clear of it - the subject here is which purse the ore + # comes out of, not where a mothership may stand. + scene.get_node("Fleet").teleport_to( + Vector2(GameState.MIN_BUILDING_SEPARATION * 4.0, 0.0)) build_controller.pending_build_type = GameState.BUILDING_MOTHERSHIP build_controller._confirm_build() From fb3587173d86e0a243e12f27a307ca1740cf9a23 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 9 Sep 2026 21:32:23 +0200 Subject: [PATCH 6/6] #225: save the run, and offer to resume it JSON at user://savegame.json rather than a binary resource: a save that can be opened in a text editor is diagnosable from a bug report, and ResourceSaver would tie the file's shape to GDScript class layout, which is the thing that keeps changing. The run is not plain data, which is the actual work here. A tagged codec carries Vector2, Vector2i, PackedStringArray and the Inventory objects nested inside `buildings`. explored_chunks is keyed by Vector2i and JSON has no such key, so every key carries its own type prefix ("v2i:3,-4") - without that a planet id could come back as a chunk coordinate. WHAT is saved is an explicit list of 30 fields, not a dump of GameState's 58 vars. A dump would carry transient junk - which pointer last touched the screen, whether a dialog holds the camera - and would break silently on every rename. The cost is that new progression must be added to the list; the benefit is that forgetting is a missing feature, not a corrupt save. Versioning, as the ticket asked. MIGRATIONS walks a save up one version at a time; MIN_SUPPORTED_VERSION is how a break too messy to migrate gets DECLARED rather than bodged. A save that cannot be walked forward is refused with a player-readable reason, because a save that loads subtly WRONG is worse than one that refuses and says so. Adding a field needs no bump: it arrives missing from old saves and keeps its new-game value (case F). The menu offers Continue only when there is something to offer. A save this build CANNOT open is still shown, disabled, with the reason underneath - silently hiding someone's run reads as lost rather than incompatible. Autosave every 30s, plus on close and on Android's app-pause, since the phone kills backgrounded apps with no warning. Skipped during the prologue. That autosave writes to the same user:// path every test that loads Main.tscn shares, so it could make Continue appear and shift the menu stack under test_main_menu depending on what ran first. That test clears the save before building the menu. Verified by breaking it: neutering the key decoder fails with "chunk keys came back as [v2i:-11,0, v2i:3,-4]". Still absent, deliberately: no save-on-demand button, and no route back to the menu from in-game, so a resume currently needs a relaunch. 175/175. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019AJMdB7iuHuLFZM1aBu3GD --- controllers/GameController.gd | 29 ++++ core/SaveGame.gd | 243 +++++++++++++++++++++++++++++++ tests/test_main_menu.gd | 5 + tests/test_main_menu_continue.gd | 117 +++++++++++++++ tests/test_save_game.gd | 179 +++++++++++++++++++++++ ui/MainMenu.gd | 31 +++- ui/MainMenu.tscn | 31 ++++ 7 files changed, 634 insertions(+), 1 deletion(-) create mode 100644 core/SaveGame.gd create mode 100644 tests/test_main_menu_continue.gd create mode 100644 tests/test_save_game.gd diff --git a/controllers/GameController.gd b/controllers/GameController.gd index b15b5f5..6e2aa03 100644 --- a/controllers/GameController.gd +++ b/controllers/GameController.gd @@ -102,6 +102,14 @@ var keyboard_seen: bool = false var keyboard_steering: bool = false +# Seconds between automatic saves. Long enough that writing the file is +# never felt, short enough that a lost run is an annoyance rather than an +# evening. +const AUTOSAVE_INTERVAL: float = 30.0 + +var autosave_countdown: float = AUTOSAVE_INTERVAL + + func _ready() -> void: # Most visuals still animate per rendered frame. The main/surface scenes opt # their roots out of global interpolation, then opt their physics-follow @@ -150,6 +158,12 @@ func _held_pivot_direction() -> float: func _notification(what: int) -> void: if what == NOTIFICATION_UNPAUSED: _forget_released_keys() + # #225: the phone is where this is played, and Android kills an app in + # the background without warning - so the run is written on the way out + # rather than only on a timer. + elif what == NOTIFICATION_WM_CLOSE_REQUEST \ + or what == NOTIFICATION_APPLICATION_PAUSED: + autosave() func _forget_released_keys() -> void: @@ -296,7 +310,22 @@ func _on_joystick_direction(direction: Vector2) -> void: fleet.stop() +# Written on a timer as well as on the way out: a crash or a battery pull +# gives no notification at all, and losing half an hour is what makes a save +# system feel absent even when it is there. +func autosave() -> void: + if GameState.intro_active: + # Nothing worth keeping yet, and saving mid-prologue would resume + # into a run that has not been set up. + return + SaveGame.save(GameState) + autosave_countdown = AUTOSAVE_INTERVAL + + func _process(delta: float) -> void: + autosave_countdown -= delta + if autosave_countdown <= 0.0: + autosave() pivot_cw_tap_cooldown_left = maxf(pivot_cw_tap_cooldown_left - delta, 0.0) pivot_ccw_tap_cooldown_left = maxf(pivot_ccw_tap_cooldown_left - delta, 0.0) _update_pivot(delta) diff --git a/core/SaveGame.gd b/core/SaveGame.gd new file mode 100644 index 0000000..c1abc5d --- /dev/null +++ b/core/SaveGame.gd @@ -0,0 +1,243 @@ +class_name SaveGame +extends RefCounted + +# Forgejo #225. Writing the run to disk and reading it back. +# +# Saves are JSON rather than a binary resource: a save that can be opened in +# a text editor can be diagnosed from a bug report, and ResourceSaver would +# tie the file's shape to GDScript class layout, which is exactly the thing +# that keeps changing. +# +# WHAT IS SAVED is an explicit list (FIELDS), not every var on GameState. +# A blind dump would carry transient junk - which pointer last touched the +# screen, whether a dialog is currently holding the camera - and would break +# silently every time a field was renamed. The cost is that a new piece of +# progression has to be added to the list; the benefit is that forgetting to +# is a missing feature rather than a corrupt save. + +const SAVE_PATH: String = "user://savegame.json" + +# Bumped whenever the SHAPE of the saved data changes - a field renamed, a +# structure reorganised. Adding a brand new field does not need a bump: it +# simply arrives missing from old saves and keeps whatever GameState starts +# it at. +const SAVE_VERSION: int = 1 + +# The oldest version this build can still open. Raising it is how a format +# change too messy to migrate is DECLARED rather than bodged: #225 is +# explicit that breaking compatibility is the right call when the transition +# is too complex, and a save that loads subtly WRONG is far worse than one +# that refuses to load and says so. +const MIN_SUPPORTED_VERSION: int = 1 + +# version N -> N+1, keyed by the version being migrated FROM. A save is +# walked up one step at a time until it reaches SAVE_VERSION. A gap here is +# not an oversight - it is the declaration that saves below it are +# incompatible, and load() refuses them by name rather than guessing. +const MIGRATIONS: Dictionary = {} + +# The run, field by field. Grouped by what they mean rather than by type, so +# it is obvious what a new piece of progression sits next to. +const FIELDS: Array = [ + # The purse and the hold. + "bank_inventory", "fleet_inventory", + # What has been built, and what has been taken. + "buildings", "next_building_id", "extractors", "planet_capture", + # What has been learned. + "unlocked", "research_points", "vessel_levels", + # The fleet itself. + "fleet_position", "fleet_vessel_types", "surviving_ship_count", + "surviving_vessel_types", "formation", "stored_vessels", + "vessel_targets", "vessel_deliveries", "repair_drones", + "max_fleet_speed_multiplier", "fleet_speed_multiplier", "build_time_scale", + # The world as the player has seen it. + "explored_chunks", "scanner_target", + # The siege. + "wave_number", "wave_phase", "wave_timer", + # The campaign. + "quests_armed", "quest_index", "quest_announced", "quest_messages", + "visited_planets", "visited_planet_kinds", +] + + +static func has_save(path: String = SAVE_PATH) -> bool: + return FileAccess.file_exists(path) + + +# What the "load your last run?" prompt needs, without committing to loading +# it. Returns an empty dictionary when there is nothing to offer. +static func peek(path: String = SAVE_PATH) -> Dictionary: + if not FileAccess.file_exists(path): + return {} + var raw: Dictionary = _read(path) + if raw.is_empty(): + return {} + return { + "version": int(raw.get("version", 0)), + "saved_at": String(raw.get("saved_at", "")), + "loadable": _refusal(int(raw.get("version", 0))) == "", + "refusal": _refusal(int(raw.get("version", 0))), + } + + +static func save(state: Node, path: String = SAVE_PATH) -> bool: + if state == null: + return false + var fields: Dictionary = {} + for name in FIELDS: + fields[name] = _encode(state.get(name)) + var payload: Dictionary = { + "version": SAVE_VERSION, + "saved_at": Time.get_datetime_string_from_system(true), + "fields": fields, + } + var file: FileAccess = FileAccess.open(path, FileAccess.WRITE) + if file == null: + return false + file.store_string(JSON.stringify(payload, "\t")) + file.close() + return true + + +# Applies a save to GameState. Returns {ok, reason} - reason is empty on +# success and player-readable on failure, because "it did not load" with no +# explanation is the thing that makes people delete their save folder. +static func load_into(state: Node, path: String = SAVE_PATH) -> Dictionary: + if state == null: + return {"ok": false, "reason": "No game to load into."} + if not FileAccess.file_exists(path): + return {"ok": false, "reason": "There is no saved run."} + var raw: Dictionary = _read(path) + if raw.is_empty(): + return {"ok": false, "reason": "The saved run could not be read - the file is damaged."} + var version: int = int(raw.get("version", 0)) + var refusal: String = _refusal(version) + if refusal != "": + return {"ok": false, "reason": refusal} + var fields: Dictionary = raw.get("fields", {}) + if not (fields is Dictionary) or fields.is_empty(): + return {"ok": false, "reason": "The saved run has no contents."} + while version < SAVE_VERSION: + fields = (MIGRATIONS[version] as Callable).call(fields) + version += 1 + for name in FIELDS: + # A field the save predates keeps whatever a new game starts it at, + # which is what makes ADDING progression a non-breaking change. + if fields.has(name): + state.set(name, _decode(fields[name])) + return {"ok": true, "reason": ""} + + +static func clear(path: String = SAVE_PATH) -> void: + if FileAccess.file_exists(path): + DirAccess.remove_absolute(ProjectSettings.globalize_path(path)) + + +# Why this version cannot be opened, or "" if it can. +static func _refusal(version: int) -> String: + if version <= 0: + return "The saved run does not say what version it is." + if version > SAVE_VERSION: + return "That run was saved by a NEWER version of the game (save %d, this build reads %d)." \ + % [version, SAVE_VERSION] + if version < MIN_SUPPORTED_VERSION: + return "That run was saved by an older version of the game (save %d) and cannot be opened by this one." \ + % version + var at: int = version + while at < SAVE_VERSION: + if not MIGRATIONS.has(at): + return "That run was saved by an older version of the game (save %d) and there is no way to bring it forward." \ + % version + at += 1 + return "" + + +static func _read(path: String) -> Dictionary: + var file: FileAccess = FileAccess.open(path, FileAccess.READ) + if file == null: + return {} + var text: String = file.get_as_text() + file.close() + var parsed: Variant = JSON.parse_string(text) + return parsed if parsed is Dictionary else {} + + +# --- the type-tagged codec --------------------------------------------- +# JSON has numbers, strings, arrays and string-keyed objects. The run has +# Vector2s, chunk coordinates used AS dictionary keys, and Inventory +# objects, so each of those is written with a tag saying what to rebuild. + +static func _encode(value: Variant) -> Variant: + if value is Vector2: + return {"__t": "v2", "x": value.x, "y": value.y} + if value is Vector2i: + return {"__t": "v2i", "x": value.x, "y": value.y} + if value is PackedStringArray: + return {"__t": "psa", "v": Array(value)} + if value is Inventory: + return {"__t": "inv", "a": value.snapshot(), + "cap": value.total_capacity, + "caps": value.resource_capacities.duplicate()} + if value is Array: + var out: Array = [] + for item in value: + out.append(_encode(item)) + return out + if value is Dictionary: + var out_d: Dictionary = {} + for key in value: + out_d[_encode_key(key)] = _encode(value[key]) + return out_d + return value + + +static func _decode(value: Variant) -> Variant: + if value is Array: + var out: Array = [] + for item in value: + out.append(_decode(item)) + return out + if value is Dictionary: + match String(value.get("__t", "")): + "v2": + return Vector2(float(value.get("x", 0.0)), float(value.get("y", 0.0))) + "v2i": + return Vector2i(int(value.get("x", 0)), int(value.get("y", 0))) + "psa": + return PackedStringArray(value.get("v", [])) + "inv": + var inv: Inventory = Inventory.new() + inv.total_capacity = int(value.get("cap", -1)) + for id in Dictionary(value.get("caps", {})): + inv.resource_capacities[String(id)] = int(value["caps"][id]) + for id in Dictionary(value.get("a", {})): + inv.set_amount(String(id), int(value["a"][id])) + return inv + var out_d: Dictionary = {} + for key in value: + out_d[_decode_key(String(key))] = _decode(value[key]) + return out_d + return value + + +# Every key carries its type, so a planet id that happens to look like a +# chunk coordinate cannot be mistaken for one on the way back in. +static func _encode_key(key: Variant) -> String: + if key is Vector2i: + return "v2i:%d,%d" % [key.x, key.y] + if key is int: + return "i:%d" % key + return "s:%s" % String(key) + + +static func _decode_key(key: String) -> Variant: + if key.begins_with("v2i:"): + var parts: PackedStringArray = key.substr(4).split(",") + if parts.size() == 2: + return Vector2i(int(parts[0]), int(parts[1])) + return key + if key.begins_with("i:"): + return int(key.substr(2)) + if key.begins_with("s:"): + return key.substr(2) + return key diff --git a/tests/test_main_menu.gd b/tests/test_main_menu.gd index f7f2aea..37ef894 100644 --- a/tests/test_main_menu.gd +++ b/tests/test_main_menu.gd @@ -26,6 +26,11 @@ var stage: int = 0 func _initialize() -> void: game_state = root.get_node("GameState") + # #225 put a Continue entry in this stack, shown only when a save + # exists - and the autosave writes to the same user:// path every test + # that runs Main.tscn for long enough. Clear it, or which entries this + # menu has would depend on what ran before it. + SaveGame.clear() menu = load("res://ui/MainMenu.tscn").instantiate() root.add_child(menu) current_scene = menu diff --git a/tests/test_main_menu_continue.gd b/tests/test_main_menu_continue.gd new file mode 100644 index 0000000..c999886 --- /dev/null +++ b/tests/test_main_menu_continue.gd @@ -0,0 +1,117 @@ +extends SceneTree + +# Forgejo #225: the menu offers to resume a run, and only when there is one. +# +# Covers: +# A. with no save on disk there is no Continue entry at all - a first-time +# player must not meet a button that does nothing +# B. with a save, Continue appears and is offered +# C. pressing it loads the run and starts the game, rather than resetting +# to a new one the way Play does +# D. a save this build cannot open is still SHOWN, disabled, with the +# reason - a run that silently vanishes reads as lost, not incompatible + +var game_state: Node +var menu: Node +var elapsed: float = 0.0 +var stage: int = 0 + + +func _initialize() -> void: + game_state = root.get_node("GameState") + SaveGame.clear() + + +func _rebuild_menu() -> void: + if menu != null and is_instance_valid(menu): + menu.free() + menu = load("res://ui/MainMenu.tscn").instantiate() + root.add_child(menu) + + +func _process(delta: float) -> bool: + elapsed += delta + match stage: + 0: + if elapsed < 0.3: + return false + # --- A. nothing saved, nothing offered ---------------------- + _rebuild_menu() + elapsed = 0.0 + stage = 1 + return false + 1: + if elapsed < 0.3: + return false + if menu.continue_button.visible: + return _fail("a Continue entry is offered with no save on disk") + print("no save: no Continue entry") + + # --- B. a saved run is offered ------------------------------ + game_state.reset_progression() + game_state.reset_new_game() + game_state.research_points = 31 + game_state.intro_active = false + if not SaveGame.save(game_state): + return _fail("could not write a save to offer") + _rebuild_menu() + elapsed = 0.0 + stage = 2 + return false + 2: + if elapsed < 0.3: + return false + if not menu.continue_button.visible: + return _fail("a save exists but no Continue entry is offered") + if not menu.continue_button.enabled: + return _fail("Continue is offered but disabled: '%s'" % menu.status_label.text) + print("a saved run is offered") + + # --- C. and pressing it resumes rather than resets ----------- + game_state.research_points = 0 + menu._on_continue_pressed() + if game_state.research_points != 31: + return _fail("Continue did not restore the run (research %d, expected 31)" \ + % game_state.research_points) + if menu.loading_path == "": + return _fail("Continue restored the run but never started the game") + print("Continue restored the run and started the game") + + # --- D. an unopenable save is shown, not hidden -------------- + _corrupt_version() + _rebuild_menu() + elapsed = 0.0 + stage = 3 + return false + 3: + if elapsed < 0.3: + return false + if not menu.continue_button.visible: + return _fail("a save this build cannot open was hidden, so the run looks lost") + if menu.continue_button.enabled: + return _fail("a save this build cannot open is still offered as loadable") + if menu.status_label.text.strip_edges() == "": + return _fail("an unopenable save is disabled with no reason given") + print("an unopenable save is shown, disabled: %s" % menu.status_label.text) + + SaveGame.clear() + print("TEST_MAIN_MENU_CONTINUE: PASS") + quit(0) + return true + return false + + +func _corrupt_version() -> void: + var f: FileAccess = FileAccess.open("user://savegame.json", FileAccess.READ) + var data: Dictionary = JSON.parse_string(f.get_as_text()) + f.close() + data["version"] = 999 + f = FileAccess.open("user://savegame.json", FileAccess.WRITE) + f.store_string(JSON.stringify(data)) + f.close() + + +func _fail(message: String) -> bool: + print("TEST_MAIN_MENU_CONTINUE: FAIL (%s)" % message) + quit(1) + return true diff --git a/tests/test_save_game.gd b/tests/test_save_game.gd new file mode 100644 index 0000000..c29b2f7 --- /dev/null +++ b/tests/test_save_game.gd @@ -0,0 +1,179 @@ +extends SceneTree + +# Forgejo #225: the run survives being written to disk and read back. +# +# The interesting part is not "does it write a file" but whether the things +# JSON cannot hold survive: Vector2 positions, chunk coordinates used as +# dictionary KEYS, and Inventory objects nested inside the buildings array. +# Those are what a naive save silently flattens. +# +# Covers: +# A. a rich run round-trips field for field +# B. Vector2i dictionary keys come back as Vector2i, not as strings +# C. inventories come back with their contents AND their capacity +# D. a save from a newer build is refused, with a reason +# E. a damaged file is refused rather than half-applied +# F. a field the save predates keeps its new-game value + +# Mirrored rather than read off SaveGame: a bare ClassName.CONST in a +# SceneTree script has broken method resolution for a whole file here before. +const TEST_PATH: String = "user://test_savegame.json" + +var game_state: Node + + +func _initialize() -> void: + game_state = root.get_node("GameState") + + +func _process(_delta: float) -> bool: + game_state.buildings.clear() + game_state.extractors.clear() + game_state.reset_progression() + game_state.reset_new_game() + game_state.reset_quests() + + # --- a run with something in every awkward corner ------------------- + game_state.research_points = 42 + game_state.unlocked = ["shipyards", "coordination"] + game_state.rocks = 1234 + game_state.fleet_position = Vector2(987.5, -321.25) + game_state.wave_number = 7 + game_state.quest_index = 5 + game_state.explored_chunks[Vector2i(3, -4)] = true + game_state.explored_chunks[Vector2i(-11, 0)] = true + game_state.visited_planets["some-planet-id"] = true + var b_id: int = game_state.add_building(GameState.BUILDING_TRANSPORT_HUB, Vector2(600.0, -150.0)) + var record: Dictionary = game_state.find_building(b_id) + record.inventory.total_capacity = 88 + record.inventory.set_amount(GameState.RESOURCE_ORE, 17) + game_state.fleet_inventory.set_amount(GameState.RESOURCE_GAS, 9) + + var before: Dictionary = _snapshot() + + if not SaveGame.save(game_state, TEST_PATH): + return _fail("the run would not save") + if not SaveGame.has_save(TEST_PATH): + return _fail("saved, but has_save() cannot see the file") + + # Wipe it back to a fresh game, so a load that does nothing cannot pass. + game_state.buildings.clear() + game_state.extractors.clear() + game_state.reset_progression() + game_state.reset_new_game() + game_state.reset_quests() + game_state.explored_chunks.clear() + if _snapshot() == before: + return _fail("wiping the run changed nothing, so a load would prove nothing") + + var result: Dictionary = SaveGame.load_into(game_state, TEST_PATH) + if not bool(result.get("ok", false)): + return _fail("the run would not load back: %s" % result.get("reason", "")) + + # --- A. field for field --------------------------------------------- + var after: Dictionary = _snapshot() + for key in before: + if after.get(key) != before[key]: + return _fail("'%s' did not survive the round trip: %s -> %s" \ + % [key, before[key], after.get(key)]) + print("a run with %d fields round-tripped intact" % before.size()) + + # --- B. chunk coordinates are still coordinates ---------------------- + if not game_state.explored_chunks.has(Vector2i(3, -4)): + return _fail("chunk keys came back as something other than Vector2i: %s" \ + % str(game_state.explored_chunks.keys())) + print("explored chunks came back keyed by Vector2i") + + # --- C. inventories keep contents and capacity ----------------------- + var loaded: Dictionary = game_state.find_building(b_id) + if loaded.is_empty(): + return _fail("the transport hub did not come back") + if int(loaded.inventory.amount(GameState.RESOURCE_ORE)) != 17: + return _fail("the hub's ore did not survive: %d" % loaded.inventory.amount(GameState.RESOURCE_ORE)) + if int(loaded.inventory.total_capacity) != 88: + return _fail("the hub's capacity did not survive: %d" % loaded.inventory.total_capacity) + if int(game_state.fleet_inventory.amount(GameState.RESOURCE_GAS)) != 9: + return _fail("the fleet hold's gas did not survive") + print("inventories came back with their contents and their capacity") + + # --- D. a save from a newer build is refused -------------------------- + _rewrite_version(999) + var newer: Dictionary = SaveGame.load_into(game_state, TEST_PATH) + if bool(newer.get("ok", false)): + return _fail("a save from a newer build loaded anyway") + if not String(newer.get("reason", "")).to_lower().contains("newer"): + return _fail("a newer save was refused without saying why: '%s'" % newer.get("reason", "")) + print("a newer save is refused: %s" % newer.get("reason", "")) + + # --- E. a damaged file is refused, not half-applied ------------------- + var f: FileAccess = FileAccess.open(TEST_PATH, FileAccess.WRITE) + f.store_string("{ this is not json") + f.close() + var marker: int = game_state.research_points + var damaged: Dictionary = SaveGame.load_into(game_state, TEST_PATH) + if bool(damaged.get("ok", false)): + return _fail("a damaged save reported success") + if game_state.research_points != marker: + return _fail("a damaged save was partly applied before it failed") + print("a damaged save is refused: %s" % damaged.get("reason", "")) + + # --- F. a field the save predates keeps its new-game value ------------ + # Adding progression must not invalidate every existing save, so a + # missing field is left alone rather than cleared. + SaveGame.save(game_state, TEST_PATH) + _drop_field("research_points") + game_state.research_points = 77 + var partial: Dictionary = SaveGame.load_into(game_state, TEST_PATH) + if not bool(partial.get("ok", false)): + return _fail("a save missing one field was refused entirely: %s" % partial.get("reason", "")) + if game_state.research_points != 77: + return _fail("a missing field was cleared instead of left alone (%d)" % game_state.research_points) + print("a field the save predates keeps its current value") + + SaveGame.clear(TEST_PATH) + print("TEST_SAVE_GAME: PASS") + quit(0) + return true + + +# The fields this test compares. Deliberately a hand-picked spread rather +# than SaveGame.FIELDS: comparing the list against itself would pass even if +# the codec dropped everything. +func _snapshot() -> Dictionary: + return { + "research_points": game_state.research_points, + "unlocked": game_state.unlocked.duplicate(), + "rocks": game_state.rocks, + "fleet_position": game_state.fleet_position, + "wave_number": game_state.wave_number, + "quest_index": game_state.quest_index, + "building_count": game_state.buildings.size(), + "chunk_count": game_state.explored_chunks.size(), + "visited": game_state.visited_planets.size(), + } + + +func _rewrite_version(to: int) -> void: + var file: FileAccess = FileAccess.open(TEST_PATH, FileAccess.READ) + var data: Dictionary = JSON.parse_string(file.get_as_text()) + file.close() + data["version"] = to + file = FileAccess.open(TEST_PATH, FileAccess.WRITE) + file.store_string(JSON.stringify(data)) + file.close() + + +func _drop_field(name: String) -> void: + var file: FileAccess = FileAccess.open(TEST_PATH, FileAccess.READ) + var data: Dictionary = JSON.parse_string(file.get_as_text()) + file.close() + data["fields"].erase(name) + file = FileAccess.open(TEST_PATH, FileAccess.WRITE) + file.store_string(JSON.stringify(data)) + file.close() + + +func _fail(message: String) -> bool: + print("TEST_SAVE_GAME: FAIL (%s)" % message) + quit(1) + return true diff --git a/ui/MainMenu.gd b/ui/MainMenu.gd index ef0eaaf..3d639c5 100644 --- a/ui/MainMenu.gd +++ b/ui/MainMenu.gd @@ -26,6 +26,8 @@ const STATUS_HEIGHT: float = 70.0 @onready var play_button: IconButton = $UI/PlayButton @onready var play_label: Label = $UI/PlayLabel +@onready var continue_button: IconButton = $UI/ContinueButton +@onready var continue_label: Label = $UI/ContinueLabel @onready var editor_button: IconButton = $UI/EditorButton @onready var editor_label: Label = $UI/EditorLabel @onready var dev_button: IconButton = $UI/DevModeButton @@ -60,6 +62,18 @@ func _ready() -> void: # sound needs them rather than during it. Sfx.warm(get_tree()) play_button.pressed.connect(_on_play_pressed) + # Offered only when there is something to offer, so a first-time player + # never sees a button that does nothing. #225. + continue_button.pressed.connect(_on_continue_pressed) + var save: Dictionary = SaveGame.peek() + continue_button.visible = not save.is_empty() + continue_label.visible = continue_button.visible + if not save.is_empty() and not bool(save.get("loadable", false)): + # A save that cannot be opened is still shown, greyed, with the + # reason under it - silently hiding someone's run makes it look + # lost rather than incompatible. + continue_button.enabled = false + status_label.text = String(save.get("refusal", "")) editor_button.pressed.connect(_on_editor_pressed) editor_button.enabled = true dev_button.pressed.connect(_on_dev_mode_pressed) @@ -87,7 +101,8 @@ func is_developer_build() -> bool: func _layout_stack() -> void: var rows: Array = [] - for row in [[play_button, play_label], [editor_button, editor_label], + for row in [[continue_button, continue_label], [play_button, play_label], + [editor_button, editor_label], [dev_button, dev_label]]: if row[0].visible: rows.append(row) @@ -114,6 +129,20 @@ func _layout_stack() -> void: status_label.offset_bottom = status_top + STATUS_HEIGHT +func _on_continue_pressed() -> void: + if loading_path != "": + return + var result: Dictionary = SaveGame.load_into(GameState) + if not bool(result.get("ok", false)): + status_label.text = String(result.get("reason", "That run could not be opened.")) + continue_button.enabled = false + return + # A resumed run never replays the prologue, whatever it was doing when + # it was written. + GameState.intro_active = false + _start("res://world/Main.tscn") + + func _on_play_pressed() -> void: # Checked here as well as in _start: this one has a side effect, and a # second press while the first is still loading must not wipe the game diff --git a/ui/MainMenu.tscn b/ui/MainMenu.tscn index 336865d..60ba5d8 100644 --- a/ui/MainMenu.tscn +++ b/ui/MainMenu.tscn @@ -68,6 +68,37 @@ theme_override_font_sizes/font_size = 32 text = "Play" horizontal_alignment = 1 +[node name="ContinueButton" type="Control" parent="UI"] +layout_mode = 3 +anchors_preset = 8 +anchor_left = 0.5 +anchor_top = 0.5 +anchor_right = 0.5 +anchor_bottom = 0.5 +offset_left = -64.0 +offset_top = -300.0 +offset_right = 64.0 +offset_bottom = -172.0 +grow_horizontal = 2 +grow_vertical = 2 +script = ExtResource("2") +icon = 2 +glow_color = Color(0.45, 0.78, 1, 1) + +[node name="ContinueLabel" type="Label" parent="UI"] +anchors_preset = 8 +anchor_left = 0.5 +anchor_top = 0.5 +anchor_right = 0.5 +anchor_bottom = 0.5 +offset_left = -160.0 +offset_top = -160.0 +offset_right = 160.0 +offset_bottom = -94.0 +theme_override_font_sizes/font_size = 32 +text = "Continue" +horizontal_alignment = 1 + [node name="EditorButton" type="Control" parent="UI" unique_id=1570704159] layout_mode = 3 anchors_preset = 8