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 <noreply@anthropic.com>
This commit is contained in:
@@ -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)
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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()
|
||||
|
||||
|
||||
@@ -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
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user