From 50aba3adf65348d74a92c6a9e1aa183e1c4d96e3 Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Sat, 28 Feb 2026 22:11:17 +0100 Subject: [PATCH] fix(client): address PR #78 review comments - Widen world_seed entropy from u32 to full u64 by combining two randi() calls (Hoshe warning #1) - Persist world_seed to save directory and restore on resume_game() so loaded sessions maintain D-010 deterministic replay (Tyre warning #2) - Constrain EntanglementConfig intrigue range based on flat value so mundane_ratio stays within D-029 spec [45,55]% (both reviewers) - Remove dead VIS_PERIPHERAL constant and _grow_bounds() method - Update test_client_p1 peripheral test for forward-only simplification - Fix misleading exp_fade shader comment (filter_nearest = hard step) Co-Authored-By: Claude Opus 4.6 --- client/scripts/autoloads/fog_state.gd | 30 -------------------- client/scripts/autoloads/session_manager.gd | 31 +++++++++++++++++++-- client/shaders/fog.gdshader | 3 +- client/tests/test_client_p1.gd | 8 ++++-- server/src/content/entanglement.rs | 27 ++++++++---------- 5 files changed, 47 insertions(+), 52 deletions(-) diff --git a/client/scripts/autoloads/fog_state.gd b/client/scripts/autoloads/fog_state.gd index 455621d14..7cef5bbc5 100644 --- a/client/scripts/autoloads/fog_state.gd +++ b/client/scripts/autoloads/fog_state.gd @@ -8,7 +8,6 @@ extends Node # Used by fog shader to distinguish visual treatment per tile. # Test assertions reference these: assert_that(byte).is_equal(FogState.VIS_FORWARD) const VIS_HIDDEN: int = 0 # Not in LOS — fully fogged -const VIS_PERIPHERAL: int = 180 # In LOS, peripheral sector — light fog dimming const VIS_FORWARD: int = 255 # In LOS, forward sector — clear vision const EXP_UNEXPLORED: int = 0 # Never seen — total darkness @@ -139,8 +138,6 @@ func update_from_state() -> void: func _grow_bounds_from_positions(positions: Dictionary) -> Rect2i: ## Compute bounds from visible_positions (Dictionary[Vector2i, bool]). - ## Equivalent to _grow_bounds() but reads from the always-populated - ## positions dict instead of the legacy visible_tiles Array. var min_x := 999999 var min_y := 999999 var max_x := -999999 @@ -158,30 +155,3 @@ func _grow_bounds_from_positions(positions: Dictionary) -> Rect2i: return map_bounds.merge(tile_bounds) -func _grow_bounds(tiles: Array) -> Rect2i: - ## Compute bounds that include all currently visible tiles, merged with - ## existing bounds so the texture only grows — never shrinks. Explored - ## tiles always stay within bounds and render as deep fog, not black. - var min_x := 999999 - var min_y := 999999 - var max_x := -999999 - var max_y := -999999 - for tile in tiles: - if not tile is Dictionary or not tile.has("x") or not tile.has("y"): - continue - min_x = mini(min_x, int(tile.x)) - min_y = mini(min_y, int(tile.y)) - max_x = maxi(max_x, int(tile.x)) - max_y = maxi(max_y, int(tile.y)) - # Guard: all tiles invalid (no x/y) — sentinels would produce negative Rect2i - if min_x > max_x: - return map_bounds - # Margin for fog gradient bleed at edges. - # The Gaussian blur in fog.gdshader samples at 2-texel intervals across a 7x7 kernel, - # reaching ±6 tiles from the fragment position. Margin must be >= 8 to avoid - # clamping artifacts at texture boundaries. - var tile_bounds := Rect2i(min_x - 8, min_y - 8, max_x - min_x + 17, max_y - min_y + 17) - # Merge with existing bounds — grow only - if map_bounds.size.x <= 1 and map_bounds.size.y <= 1: - return tile_bounds - return map_bounds.merge(tile_bounds) diff --git a/client/scripts/autoloads/session_manager.gd b/client/scripts/autoloads/session_manager.gd index 128efc48a..cba63a5d9 100644 --- a/client/scripts/autoloads/session_manager.gd +++ b/client/scripts/autoloads/session_manager.gd @@ -33,16 +33,23 @@ func new_game() -> String: GameState.current_game_id = game_id # #175: Generate world_seed for deterministic simulation (D-010, D-029). - # Uses randi() (u32) for a seed that maps cleanly to Rust u64 via MessagePack. - # 4 billion seeds is sufficient entropy for EntanglementConfig variation (D-029). - GameState.world_seed = rng.randi() + # Combines two randi() calls (u32 each) into full u64 entropy range. + # Without this, upper 32 bits are always zero — halving the seed space. + GameState.world_seed = (rng.randi() << 32) | rng.randi() + + # Persist world_seed to save directory so resume_game() can restore it. + # Without this, loaded sessions would send seed=0, breaking D-010 determinism. + _write_seed_file(save_path, GameState.world_seed) return game_id ## Resume an existing game session by setting the active game-id. +## Restores world_seed from the save directory for D-010 deterministic replay. func resume_game(game_id: String) -> void: GameState.current_game_id = game_id + var save_path := SAVES_DIR + game_id + "/" + GameState.world_seed = _read_seed_file(save_path) ## List all game directories under user://saves/ sorted by last-modified (most recent first). @@ -117,6 +124,24 @@ func _cleanup_quit_dialog() -> void: _quit_dialog = null +## Write world_seed to a file in the save directory for session persistence. +func _write_seed_file(save_path: String, seed: int) -> void: + var file := FileAccess.open(save_path + "world_seed", FileAccess.WRITE) + if file == null: + push_error("SessionManager: failed to write seed file: %s" % error_string(FileAccess.get_open_error())) + return + file.store_64(seed) + + +## Read world_seed from save directory. Returns 0 if file missing (legacy saves). +func _read_seed_file(save_path: String) -> int: + var file := FileAccess.open(save_path + "world_seed", FileAccess.READ) + if file == null: + push_warning("SessionManager: no seed file in %s — using seed=0 (legacy save)" % save_path) + return 0 + return file.get_64() + + func _find_newest_save(dir_path: String) -> String: var dir := DirAccess.open(dir_path) if dir == null: diff --git a/client/shaders/fog.gdshader b/client/shaders/fog.gdshader index 16c89b18d..06fb0b78d 100644 --- a/client/shaders/fog.gdshader +++ b/client/shaders/fog.gdshader @@ -90,8 +90,9 @@ void fragment() { float alpha = mix(fog_alpha, 0.0, clarity); vec3 color = mix(FOG_OVERLAY_COLOR, vec3(0.0), clarity); - // Soft transition at the explored/unexplored tile boundary. + // Hard step at the explored/unexplored tile boundary. // exploration_tex uses filter_nearest: explored is exactly 0.0, ~0.5, or 1.0. + // smoothstep maps these to 0.0 or 1.0 — no sub-tile blending. float exp_fade = smoothstep(0.0, 0.2, explored); alpha = mix(1.0, alpha, exp_fade); color = mix(UNEXPLORED_COLOR, color, exp_fade); diff --git a/client/tests/test_client_p1.gd b/client/tests/test_client_p1.gd index 90ce32365..eed13bc82 100644 --- a/client/tests/test_client_p1.gd +++ b/client/tests/test_client_p1.gd @@ -55,7 +55,9 @@ func test_fog_visibility_forward_tile() -> void: func test_fog_visibility_peripheral_tile() -> void: - # P1 #4: Peripheral-sector tile writes VIS_PERIPHERAL (180) to _vis_bytes. + # P1 #4: Server simplified to forward-only (Sprint 22, #569). All visible + # tiles are now written as VIS_FORWARD regardless of visibility_sectors value. + # Peripheral sector is no longer a distinct visual state. var fog = _get_fog_state() if fog == null: return @@ -65,8 +67,8 @@ func test_fog_visibility_peripheral_tile() -> void: GameState.visibility_sectors = {Vector2i(10, 8): "Peripheral"} fog.update_from_state() assert_that(fog._vis_bytes[8 * 64 + 10]).override_failure_message( - "Peripheral tile at (10,8) should be VIS_PERIPHERAL=%d" % FogState.VIS_PERIPHERAL - ).is_equal(FogState.VIS_PERIPHERAL) + "All visible tiles write VIS_FORWARD after forward-only simplification" + ).is_equal(FogState.VIS_FORWARD) GameState.visible_positions.clear() GameState.visibility_sectors.clear() diff --git a/server/src/content/entanglement.rs b/server/src/content/entanglement.rs index 0de1b9a29..c6b45243a 100644 --- a/server/src/content/entanglement.rs +++ b/server/src/content/entanglement.rs @@ -44,9 +44,13 @@ impl EntanglementConfig { pub fn from_rng(rng: &mut SimRng) -> Self { // Sample flat_ratio ∈ [25, 35] — step of 1% let flat: u8 = rng.rng.random_range(25u8..=35u8); - // Sample intrigue_ratio ∈ [15, 25] — step of 1% - let intrigue: u8 = rng.rng.random_range(15u8..=25u8); - // Mundane fills the remainder (ensures sum = 100) + // Constrain intrigue range so mundane = 100 - flat - intrigue stays in [45, 55]. + // mundane ≥ 45 → intrigue ≤ 55 - flat; mundane ≤ 55 → intrigue ≥ 45 - flat. + // Intersect with D-029 base range [15, 25]. + let intrigue_min: u8 = (45u8.saturating_sub(flat)).max(15); + let intrigue_max: u8 = (55u8.saturating_sub(flat)).min(25); + let intrigue: u8 = rng.rng.random_range(intrigue_min..=intrigue_max); + // Mundane fills the remainder (ensures sum = 100, stays in [45, 55]) let mundane: u8 = 100 - flat - intrigue; Self { flat_ratio: flat, @@ -178,24 +182,17 @@ mod tests { #[test] fn mundane_ratio_within_bounds() { // D-029: mundane ∈ [45, 55]% - // Derivation: flat ∈ [25,35], intrigue ∈ [15,25], mundane = 100 - flat - intrigue - // worst case: flat=35, intrigue=25 → mundane=40 (below 45!) - // CAVEAT: this reveals a potential spec inconsistency — if flat and intrigue - // are sampled independently, mundane can fall outside [45,55]. - // Resolution options: (a) constrain sampling so mundane stays in range, - // (b) accept mundane range as derived. This test documents the actual range. - // TODO: coordinate with Tyre on intended sampling strategy. + // Achieved by constraining intrigue range based on flat value so that + // mundane = 100 - flat - intrigue always stays within spec bounds. for seed in 0u64..200 { let c = EntanglementConfig::from_seed(seed); assert!( c.is_valid(), - "Seed {seed}: ratios must sum to 100 regardless of mundane derivation" + "Seed {seed}: ratios must sum to 100" ); - // Derived mundane range: 100 - 35 - 25 = 40 minimum, 100 - 25 - 15 = 60 maximum - // Note: if spec requires strict [45,55], the sampling ranges must be tighter. assert!( - c.mundane_ratio >= 40 && c.mundane_ratio <= 60, - "Seed {seed}: mundane_ratio {} out of derived [40, 60] range", + c.mundane_ratio >= 45 && c.mundane_ratio <= 55, + "Seed {seed}: mundane_ratio {} out of D-029 [45, 55] bounds", c.mundane_ratio ); }