refactor(client): make stationary_ticks and zone_id server-authoritative (#557)
apply_snapshot() now reads stationary_ticks and zone_id directly from the server snapshot when present (D-020 compliance). Client-side accumulation and tile lookup retained as deprecated fallbacks until the server populates these fields. Protocol.gd extended with decode paths and TODO markers for the server team. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -109,14 +109,17 @@ var medium_sound_events: Array = []
|
||||
var close_sound_events: Array = []
|
||||
|
||||
# D-071 (#530): Consecutive ticks without player position change.
|
||||
# Incremented per snapshot in apply_snapshot(). Reset to 0 on movement.
|
||||
# D-020: Server-authoritative — read from snapshot "stationary_ticks" field.
|
||||
# Fallback: client-side accumulation (deprecated, remove when server populates field).
|
||||
# ListeningFocus boost activates at 30+ ticks (main.gd manages the dip).
|
||||
var stationary_ticks: int = 0
|
||||
# DEPRECATED: Only used by client-side accumulation fallback. Remove with fallback.
|
||||
var _prev_player_position: Vector2 = Vector2(-1e9, -1e9) # sentinel: no previous position
|
||||
|
||||
# D-073 (#529): Server-authoritative zone_id from the player's current tile.
|
||||
# Extracted in apply_snapshot() — avoids O(N) tile scan in main.gd per Tyre review.
|
||||
# Empty string when zone_id field absent (server hasn't shipped OQ-09 yet).
|
||||
# D-020: Read directly from snapshot "zone_id" field.
|
||||
# Fallback: client-side tile lookup (deprecated, remove when server populates field).
|
||||
# Empty string when zone_id field absent.
|
||||
var current_zone_id: String = ""
|
||||
|
||||
func apply_snapshot(snapshot: Dictionary) -> void:
|
||||
@@ -140,12 +143,19 @@ func apply_snapshot(snapshot: Dictionary) -> void:
|
||||
push_warning("GameState: no Player entity found in %d entities" % [
|
||||
visible_entities.size()])
|
||||
|
||||
# D-071 (#530): Track consecutive stationary ticks for ListeningFocus boost.
|
||||
# Compares current player_position against previous snapshot's position.
|
||||
if player_position == _prev_player_position:
|
||||
stationary_ticks += 1
|
||||
# D-020/D-071 (#530): Server-authoritative stationary_ticks for ListeningFocus boost.
|
||||
# Prefer server-sent value; fall back to client-side accumulation until server populates.
|
||||
if snapshot.has("stationary_ticks") and snapshot.stationary_ticks is int:
|
||||
# D-020: direct field assignment from server-authoritative snapshot.
|
||||
stationary_ticks = snapshot.stationary_ticks
|
||||
else:
|
||||
stationary_ticks = 0
|
||||
# DEPRECATED fallback — client-side accumulation. Remove when server sends
|
||||
# "stationary_ticks" in ObserverSnapshot (D-020 violation: derives behavior-
|
||||
# driving state on the client). Server tracks this in ListeningFocus component.
|
||||
if player_position == _prev_player_position:
|
||||
stationary_ticks += 1
|
||||
else:
|
||||
stationary_ticks = 0
|
||||
_prev_player_position = player_position
|
||||
|
||||
# Tiles for rendering: test mode sends "tiles", live server sends tile data in "visible_tiles"
|
||||
@@ -295,16 +305,24 @@ func apply_snapshot(snapshot: Dictionary) -> void:
|
||||
if snapshot.has("player_knowledge") and snapshot.player_knowledge is Dictionary:
|
||||
player_knowledge = snapshot.player_knowledge
|
||||
|
||||
# D-073 (#529): O(1) zone_id lookup. Build coord→tile dict from member visible_tiles
|
||||
# (populated above from either "tiles" test-mode key or "visible_tiles" live key).
|
||||
# Must use the member var, not snapshot.visible_tiles, so test mode is covered.
|
||||
var _tile_by_coord: Dictionary = {}
|
||||
for vtile in visible_tiles:
|
||||
if vtile is Dictionary and vtile.has("x") and vtile.has("y"):
|
||||
_tile_by_coord[Vector2i(vtile.x, vtile.y)] = vtile
|
||||
var player_pos_key := Vector2i(int(player_position.x), int(player_position.y))
|
||||
var player_tile = _tile_by_coord.get(player_pos_key, null)
|
||||
current_zone_id = player_tile.get("zone_id", "") if player_tile else ""
|
||||
# D-020/D-073 (#529): Server-authoritative zone_id for zone ambient crossfade.
|
||||
# Prefer server-sent top-level value; fall back to client-side tile lookup until
|
||||
# server populates top-level "zone_id" in ObserverSnapshot.
|
||||
if snapshot.has("zone_id") and snapshot.zone_id is String:
|
||||
# D-020: direct field assignment from server-authoritative snapshot.
|
||||
current_zone_id = snapshot.zone_id
|
||||
else:
|
||||
# DEPRECATED fallback — client-side tile lookup. Remove when server sends
|
||||
# top-level "zone_id" in ObserverSnapshot (D-020 violation: derives zone
|
||||
# identity on the client via tile iteration). Server sends zone_id per
|
||||
# VisibleTile but not as a top-level snapshot field.
|
||||
var _tile_by_coord: Dictionary = {}
|
||||
for vtile in visible_tiles:
|
||||
if vtile is Dictionary and vtile.has("x") and vtile.has("y"):
|
||||
_tile_by_coord[Vector2i(vtile.x, vtile.y)] = vtile
|
||||
var player_pos_key := Vector2i(int(player_position.x), int(player_position.y))
|
||||
var player_tile = _tile_by_coord.get(player_pos_key, null)
|
||||
current_zone_id = player_tile.get("zone_id", "") if player_tile else ""
|
||||
|
||||
# v2: visible_tiles with visibility sectors
|
||||
# Derives visible_positions when not explicitly provided (real server mode)
|
||||
|
||||
@@ -244,6 +244,24 @@ static func decode_snapshot(bytes: PackedByteArray) -> Variant:
|
||||
"error": raw_save.get("error"),
|
||||
}
|
||||
|
||||
# TODO(server): Send stationary_ticks in ObserverSnapshot (D-071, D-020).
|
||||
# Server already tracks this in ListeningFocus component (server/src/simulation/listening.rs).
|
||||
# When server populates this field, client-side accumulation fallback in game_state.gd
|
||||
# can be removed — apply_snapshot() should contain only direct field assignments.
|
||||
var stationary_ticks: Variant = null
|
||||
var raw_st: Variant = raw.get("stationary_ticks")
|
||||
if raw_st != null:
|
||||
stationary_ticks = int(raw_st)
|
||||
|
||||
# TODO(server): Send top-level zone_id string in ObserverSnapshot (D-073, D-020).
|
||||
# Server sends zone_id per VisibleTile but not as a top-level snapshot field.
|
||||
# When server populates this, client-side tile iteration fallback in game_state.gd
|
||||
# can be removed — apply_snapshot() should contain only direct field assignments.
|
||||
var zone_id: Variant = null
|
||||
var raw_zid: Variant = raw.get("zone_id")
|
||||
if raw_zid is String:
|
||||
zone_id = raw_zid
|
||||
|
||||
# v14: player_knowledge (#264, D-041) — partial KG dump for journal panel.
|
||||
# {entities: [{entity_id, name, confidence, source, state, relationship, last_observed_tick}],
|
||||
# facts: [{fact_id, confidence, source, state, acquired_tick}]}
|
||||
@@ -302,6 +320,8 @@ static func decode_snapshot(bytes: PackedByteArray) -> Variant:
|
||||
"examine_result": examine_result,
|
||||
"player_knowledge": player_knowledge,
|
||||
"save_result": save_result,
|
||||
"stationary_ticks": stationary_ticks,
|
||||
"zone_id": zone_id,
|
||||
}
|
||||
|
||||
|
||||
|
||||
@@ -19,6 +19,8 @@ func before_each() -> void:
|
||||
GameState.player_stance = "Walk"
|
||||
GameState.player_inventory = []
|
||||
GameState.stationary_ticks = 0
|
||||
GameState._prev_player_position = Vector2(-1e9, -1e9)
|
||||
GameState.current_zone_id = ""
|
||||
GameState.insert_active = true
|
||||
|
||||
|
||||
@@ -115,19 +117,49 @@ func test_apply_snapshot_inventory_absent_clears_list() -> void:
|
||||
assert_that(GameState.player_inventory.size()).is_equal(0)
|
||||
|
||||
|
||||
# -- Stationary tick counter (D-071) -------------------------------------
|
||||
# -- Stationary ticks: server-authoritative (D-020/D-071) -----------------
|
||||
|
||||
func test_stationary_ticks_increments_when_player_position_unchanged() -> void:
|
||||
func test_stationary_ticks_from_server_snapshot() -> void:
|
||||
## D-020: When server sends stationary_ticks, client reads it directly.
|
||||
GameState.apply_snapshot({"tick": 1, "entities": [], "stationary_ticks": 42})
|
||||
assert_int(GameState.stationary_ticks).is_equal(42)
|
||||
|
||||
|
||||
func test_stationary_ticks_server_value_overrides_client_accumulation() -> void:
|
||||
## D-020: Server value takes priority — client must not accumulate on top of it.
|
||||
var snapshot := {
|
||||
"tick": 1,
|
||||
"entities": [{"entity_id": 1, "x": 10.0, "y": 10.0, "z": 0, "kind": {"variant": "Player", "data": null}}],
|
||||
"stationary_ticks": 5,
|
||||
}
|
||||
GameState.apply_snapshot(snapshot)
|
||||
GameState.apply_snapshot(snapshot) # same position, but server sends 5 again
|
||||
assert_int(GameState.stationary_ticks).is_equal(5) # server value, not 6
|
||||
|
||||
|
||||
func test_stationary_ticks_missing_field_degrades_gracefully() -> void:
|
||||
## D-020 fallback: when server omits stationary_ticks, no crash, defaults to 0.
|
||||
GameState.stationary_ticks = 0
|
||||
GameState.apply_snapshot({"tick": 1, "entities": []})
|
||||
# No crash; field retains a valid integer value.
|
||||
assert_int(GameState.stationary_ticks).is_greater_equal(0)
|
||||
|
||||
|
||||
# -- Stationary ticks: deprecated client-side fallback (D-071) -----------
|
||||
|
||||
func test_stationary_ticks_fallback_increments_when_position_unchanged() -> void:
|
||||
## Deprecated fallback: client accumulates when server omits the field.
|
||||
var snapshot := {
|
||||
"tick": 1,
|
||||
"entities": [{"entity_id": 1, "x": 10.0, "y": 10.0, "z": 0, "kind": {"variant": "Player", "data": null}}],
|
||||
}
|
||||
GameState.apply_snapshot(snapshot) # first call: position changes from ZERO
|
||||
GameState.apply_snapshot(snapshot) # first call: position changes from sentinel
|
||||
GameState.apply_snapshot(snapshot) # second call: position unchanged → +1
|
||||
assert_int(GameState.stationary_ticks).is_greater(0)
|
||||
|
||||
|
||||
func test_stationary_ticks_resets_on_player_movement() -> void:
|
||||
func test_stationary_ticks_fallback_resets_on_movement() -> void:
|
||||
## Deprecated fallback: client resets on movement when server omits the field.
|
||||
var s1 := {
|
||||
"tick": 1,
|
||||
"entities": [{"entity_id": 1, "x": 10.0, "y": 10.0, "z": 0, "kind": {"variant": "Player", "data": null}}],
|
||||
@@ -143,6 +175,33 @@ func test_stationary_ticks_resets_on_player_movement() -> void:
|
||||
assert_int(GameState.stationary_ticks).is_equal(0)
|
||||
|
||||
|
||||
# -- Zone ID: server-authoritative (D-020/D-073) -------------------------
|
||||
|
||||
func test_zone_id_from_server_snapshot() -> void:
|
||||
## D-020: When server sends top-level zone_id, client reads it directly.
|
||||
GameState.apply_snapshot({"tick": 1, "entities": [], "zone_id": "zone_alpha"})
|
||||
assert_that(GameState.current_zone_id).is_equal("zone_alpha")
|
||||
|
||||
|
||||
func test_zone_id_server_value_overrides_tile_derivation() -> void:
|
||||
## D-020: Server top-level zone_id takes priority over tile-derived zone_id.
|
||||
var snapshot := {
|
||||
"tick": 1,
|
||||
"entities": [{"entity_id": 1, "x": 5.0, "y": 5.0, "z": 0, "kind": {"variant": "Player", "data": null}}],
|
||||
"tiles": [{"x": 5, "y": 5, "z": 0, "type": "floor", "zone_id": "zone_from_tile"}],
|
||||
"zone_id": "zone_from_server",
|
||||
}
|
||||
GameState.apply_snapshot(snapshot)
|
||||
assert_that(GameState.current_zone_id).is_equal("zone_from_server")
|
||||
|
||||
|
||||
func test_zone_id_missing_field_degrades_gracefully() -> void:
|
||||
## D-020 fallback: when server omits zone_id and no tiles match, defaults to "".
|
||||
GameState.current_zone_id = ""
|
||||
GameState.apply_snapshot({"tick": 1, "entities": []})
|
||||
assert_that(GameState.current_zone_id).is_equal("")
|
||||
|
||||
|
||||
# -- insert_active (OQ-07, #522) -----------------------------------------
|
||||
|
||||
func test_apply_snapshot_insert_active_false() -> void:
|
||||
|
||||
@@ -0,0 +1,164 @@
|
||||
## Sprint 20 #557: GameState.apply_snapshot() refactor tests.
|
||||
##
|
||||
## Verifies D-020 compliance: apply_snapshot() reads server-authoritative values
|
||||
## for stationary_ticks and zone_id directly from the snapshot when present,
|
||||
## and degrades gracefully when the server has not yet added these fields.
|
||||
##
|
||||
## Does NOT replace test_game_state.gd or test_snapshot_zone_id.gd — those cover
|
||||
## existing behaviour. This file covers the new code paths added in Sprint 20.
|
||||
##
|
||||
## D-030: fixture-based, server-free, no subprocess required.
|
||||
class_name TestGameStateSprint20
|
||||
extends GdUnitTestSuite
|
||||
|
||||
|
||||
func before_each() -> void:
|
||||
GameState.stationary_ticks = 0
|
||||
GameState._prev_player_position = Vector2(-1e9, -1e9)
|
||||
GameState.current_zone_id = ""
|
||||
GameState.player_position = Vector2.ZERO
|
||||
GameState.visible_tiles = []
|
||||
GameState.visible_positions = {}
|
||||
GameState.visibility_sectors = {}
|
||||
|
||||
|
||||
# -- stationary_ticks: server-authoritative path (D-020) ----------------------
|
||||
|
||||
func test_stationary_ticks_reads_server_value_when_present() -> void:
|
||||
# When snapshot includes stationary_ticks, apply_snapshot() must use the
|
||||
# server value directly without client-side accumulation (D-020).
|
||||
var snapshot := {
|
||||
"tick": 5,
|
||||
"entities": [
|
||||
{"entity_id": 1, "x": 10.0, "y": 10.0, "z": 0, "kind": {"variant": "Player", "data": null}},
|
||||
],
|
||||
"stationary_ticks": 42,
|
||||
}
|
||||
GameState.apply_snapshot(snapshot)
|
||||
assert_int(GameState.stationary_ticks).override_failure_message(
|
||||
"apply_snapshot() must read stationary_ticks=42 from snapshot (D-020)"
|
||||
).is_equal(42)
|
||||
|
||||
|
||||
func test_stationary_ticks_server_value_does_not_accumulate() -> void:
|
||||
# Server-sent value must be assigned directly — NOT added to existing value.
|
||||
# Two calls with stationary_ticks=10 must yield 10, not 20.
|
||||
var snapshot := {
|
||||
"tick": 1,
|
||||
"entities": [
|
||||
{"entity_id": 1, "x": 5.0, "y": 5.0, "z": 0, "kind": {"variant": "Player", "data": null}},
|
||||
],
|
||||
"stationary_ticks": 10,
|
||||
}
|
||||
GameState.apply_snapshot(snapshot)
|
||||
GameState.apply_snapshot(snapshot)
|
||||
assert_int(GameState.stationary_ticks).override_failure_message(
|
||||
"Server value must be assigned directly, not accumulated (D-020)"
|
||||
).is_equal(10)
|
||||
|
||||
|
||||
func test_stationary_ticks_server_can_reset_to_zero() -> void:
|
||||
# Server sends 0 when player moves — client must accept this reset.
|
||||
GameState.stationary_ticks = 50
|
||||
var snapshot := {
|
||||
"tick": 2,
|
||||
"entities": [
|
||||
{"entity_id": 1, "x": 5.0, "y": 5.0, "z": 0, "kind": {"variant": "Player", "data": null}},
|
||||
],
|
||||
"stationary_ticks": 0,
|
||||
}
|
||||
GameState.apply_snapshot(snapshot)
|
||||
assert_int(GameState.stationary_ticks).override_failure_message(
|
||||
"Server reset to 0 must override client-held value"
|
||||
).is_equal(0)
|
||||
|
||||
|
||||
# -- stationary_ticks: DEPRECATED fallback (graceful degradation) --------------
|
||||
|
||||
func test_stationary_ticks_fallback_when_field_absent() -> void:
|
||||
# When snapshot has no stationary_ticks, client-side accumulation must still
|
||||
# run (backward compat until server ships field). No crash.
|
||||
var snapshot := {
|
||||
"tick": 1,
|
||||
"entities": [
|
||||
{"entity_id": 1, "x": 10.0, "y": 10.0, "z": 0, "kind": {"variant": "Player", "data": null}},
|
||||
],
|
||||
}
|
||||
GameState.apply_snapshot(snapshot) # position changes from ZERO → resets to 0
|
||||
GameState.apply_snapshot(snapshot) # position unchanged → increments
|
||||
assert_int(GameState.stationary_ticks).override_failure_message(
|
||||
"Client-side fallback must increment stationary_ticks when field absent"
|
||||
).is_greater(0)
|
||||
|
||||
|
||||
func test_apply_snapshot_missing_stationary_ticks_no_crash() -> void:
|
||||
# Snapshots lacking stationary_ticks must not crash apply_snapshot().
|
||||
var snapshot := {"tick": 1, "entities": []}
|
||||
# No assertion needed beyond confirming no exception is raised.
|
||||
GameState.apply_snapshot(snapshot)
|
||||
assert_bool(true).is_true()
|
||||
|
||||
|
||||
# -- zone_id: server-authoritative path (D-020/D-073) -------------------------
|
||||
|
||||
func test_current_zone_id_reads_server_value_when_present() -> void:
|
||||
# When snapshot includes top-level zone_id, apply_snapshot() must use it
|
||||
# directly without tile lookup (D-020).
|
||||
var snapshot := {
|
||||
"tick": 1,
|
||||
"entities": [
|
||||
{"entity_id": 1, "x": 5.0, "y": 5.0, "z": 0, "kind": {"variant": "Player", "data": null}},
|
||||
],
|
||||
"zone_id": "zone_server_direct",
|
||||
}
|
||||
GameState.apply_snapshot(snapshot)
|
||||
assert_str(GameState.current_zone_id).override_failure_message(
|
||||
"apply_snapshot() must read zone_id='zone_server_direct' from snapshot (D-020)"
|
||||
).is_equal("zone_server_direct")
|
||||
|
||||
|
||||
func test_current_zone_id_server_value_overrides_tile_data() -> void:
|
||||
# When snapshot has both zone_id and visible_tiles with a different zone,
|
||||
# the top-level zone_id field takes priority.
|
||||
var snapshot := {
|
||||
"tick": 1,
|
||||
"entities": [
|
||||
{"entity_id": 1, "x": 5.0, "y": 5.0, "z": 0, "kind": {"variant": "Player", "data": null}},
|
||||
],
|
||||
"zone_id": "zone_from_server",
|
||||
"visible_tiles": [
|
||||
{"x": 5, "y": 5, "z": 0, "type": "floor", "zone_id": "zone_from_tile"},
|
||||
],
|
||||
}
|
||||
GameState.apply_snapshot(snapshot)
|
||||
assert_str(GameState.current_zone_id).override_failure_message(
|
||||
"Top-level zone_id must override tile-derived zone when both present"
|
||||
).is_equal("zone_from_server")
|
||||
|
||||
|
||||
# -- zone_id: DEPRECATED fallback (graceful degradation) ----------------------
|
||||
|
||||
func test_current_zone_id_fallback_to_tile_lookup_when_absent() -> void:
|
||||
# When snapshot lacks top-level zone_id, tile lookup fallback must run.
|
||||
# Existing test_snapshot_zone_id.gd covers detailed scenarios; this is a
|
||||
# smoke test confirming the fallback path still works after Sprint 20 refactor.
|
||||
var snapshot := {
|
||||
"tick": 1,
|
||||
"entities": [
|
||||
{"entity_id": 1, "x": 3.0, "y": 3.0, "z": 0, "kind": {"variant": "Player", "data": null}},
|
||||
],
|
||||
"visible_tiles": [
|
||||
{"x": 3, "y": 3, "z": 0, "type": "floor", "zone_id": "zone_tile_fallback"},
|
||||
],
|
||||
}
|
||||
GameState.apply_snapshot(snapshot)
|
||||
assert_str(GameState.current_zone_id).override_failure_message(
|
||||
"Tile lookup fallback must populate zone_id when top-level field absent"
|
||||
).is_equal("zone_tile_fallback")
|
||||
|
||||
|
||||
func test_apply_snapshot_missing_zone_id_no_crash() -> void:
|
||||
# Snapshots lacking both zone_id and visible_tiles must not crash.
|
||||
var snapshot := {"tick": 1, "entities": []}
|
||||
GameState.apply_snapshot(snapshot)
|
||||
assert_str(GameState.current_zone_id).is_equal("")
|
||||
Reference in New Issue
Block a user