fix(client): address PR #38 review — 6 warnings + 4 suggestions
Hoshe:
- COLOR_FADE_DURATION 0.7 → 0.5 to match D-033 spec ("0.5s fade")
- Gauntlet guard tests now exercise InputMapper._unhandled_input()
with synthesized InputEventKey instead of asserting a bool
- Buffer clearing tests use SimBridge pipeline instead of manual nulls
- Add mid-transition re-trigger test (rapid relationship changes)
- Add relationship field to test snapshot NPC
Tyre:
- Add _teleport_in_progress flag to defer smoothing re-enable by one
frame after teleport (prevents same-_process() re-enable race)
- Add _test_gauntlet_mode to SimBridge test snapshot
- Extract TELEPORT_DISTANCE_THRESHOLD constant, mirror in tests
- Add comments: flash preemption, modulate/color independence
- Rename "hub teleport" → "Gauntlet dev teleport" in code comments
to clarify this is not production fast-travel
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -20,7 +20,7 @@ enum Action {
|
||||
TOGGLE_STANCE_UP, TOGGLE_STANCE_DOWN,
|
||||
BUG_REPORT, # #495: F12 WRONG button — client-only, not sent to server
|
||||
SET_FACING, # D-054: facing octant update (no movement)
|
||||
TELEPORT_HUB, # #501: Home key — teleport to hub (Gauntlet-only)
|
||||
TELEPORT_HUB, # #501: Home key — Gauntlet dev teleport (not production fast-travel)
|
||||
}
|
||||
|
||||
var input_queue: Array[Dictionary] = []
|
||||
|
||||
@@ -10,6 +10,8 @@ var _test_player_pos: Vector2i = Vector2i(10, 10)
|
||||
var _test_facing: String = "North"
|
||||
var _test_input_queue: Array = [] # Queued actions for test mode
|
||||
var _test_in_dialogue: bool = false # Mock dialogue state (#434)
|
||||
var _test_gauntlet_mode: bool = false # #501: Gauntlet mode for dev teleport guard
|
||||
var _test_npc_relationship: String = "Unknown" # #521: NPC relationship for D-033 color
|
||||
var _last_snapshot: Variant = null # Most recent decoded snapshot (consumed by poll_snapshot)
|
||||
var _outbound_buffer: Array[Dictionary] = [] # Raw inputs awaiting batch encode + transport
|
||||
|
||||
@@ -40,6 +42,8 @@ func reset_test_state() -> void:
|
||||
_test_facing = "North"
|
||||
_test_input_queue.clear()
|
||||
_test_in_dialogue = false
|
||||
_test_gauntlet_mode = false
|
||||
_test_npc_relationship = "Unknown"
|
||||
|
||||
# Change connection state and emit signal
|
||||
func _set_state(new_state: ConnectionState) -> void:
|
||||
@@ -270,7 +274,7 @@ static func _action_enum_to_wire(action: int) -> String:
|
||||
InputMapper.Action.SET_FACING:
|
||||
return "SetFacing" # D-054: facing octant update (no movement)
|
||||
InputMapper.Action.TELEPORT_HUB:
|
||||
return "TeleportToHub" # #501: hub teleport (Gauntlet-only)
|
||||
return "TeleportToHub" # #501: Gauntlet dev teleport (not production fast-travel)
|
||||
_:
|
||||
push_warning("SimBridge: unknown action enum %s" % action)
|
||||
return ""
|
||||
@@ -333,6 +337,7 @@ func _test_snapshot() -> Dictionary:
|
||||
"z": 0,
|
||||
"kind": { "variant": "Npc", "data": null },
|
||||
"visibility": sector,
|
||||
"relationship": _test_npc_relationship,
|
||||
})
|
||||
|
||||
# v4: nearby_interactions when NPC is nearby and visible (#404/#405)
|
||||
@@ -411,6 +416,7 @@ func _test_snapshot() -> Dictionary:
|
||||
"current_monologue": monologue,
|
||||
"current_dialogue": dialogue,
|
||||
"pending_recognitions": pending_recs,
|
||||
"gauntlet_mode": _test_gauntlet_mode,
|
||||
}
|
||||
|
||||
# Generate a small test room: 8x6 room with walls, a door, and floor
|
||||
|
||||
+19
-6
@@ -20,7 +20,8 @@ var _last_dialogue_npc_id: int = -1 # D-064: NPC entity_id for WalkAway input
|
||||
var _camera_anchored: bool = false
|
||||
var _last_monologue_tick: int = -1 # Prevent re-consuming monologue when same tick polled twice
|
||||
var _last_dialogue_tick: int = -1
|
||||
var _flash_rect: ColorRect = null # #502: ephemeral screen flash overlay
|
||||
var _flash_rect: ColorRect = null # #502/#501: ephemeral screen flash overlay (shared: teleport preempts amber)
|
||||
var _teleport_in_progress: bool = false # #501: defer smoothing re-enable by one frame after teleport
|
||||
|
||||
func _ready() -> void:
|
||||
print("The Settled Reach — client initialized")
|
||||
@@ -123,9 +124,15 @@ func _process(_delta: float) -> void:
|
||||
# rendered used smoothing=OFF (correct viewport from frame one). Now we
|
||||
# turn smoothing back on and sync its internal state so subsequent frames
|
||||
# get smooth camera tracking during gameplay.
|
||||
# #501: Skip re-enable during teleport — _teleport_transition() disables
|
||||
# smoothing for a clean camera snap. Defer by one frame to avoid the
|
||||
# re-enable block in the same _process() call undoing the snap.
|
||||
if _camera_anchored and not camera.position_smoothing_enabled:
|
||||
camera.position_smoothing_enabled = true
|
||||
camera.reset_smoothing()
|
||||
if _teleport_in_progress:
|
||||
_teleport_in_progress = false
|
||||
else:
|
||||
camera.position_smoothing_enabled = true
|
||||
camera.reset_smoothing()
|
||||
|
||||
# Send queued input to simulation
|
||||
var inputs = InputMapper.flush_queue()
|
||||
@@ -236,17 +243,23 @@ func _on_connection_state_changed(old_state: SimBridge.ConnectionState, new_stat
|
||||
|
||||
|
||||
# #501: Detect large position jump indicating a teleport (not normal movement).
|
||||
const TELEPORT_DISTANCE_THRESHOLD: float = 5.0
|
||||
|
||||
func _detect_teleport(old_pos: Vector2, new_pos: Vector2) -> bool:
|
||||
return old_pos.distance_to(new_pos) > 5.0
|
||||
return old_pos.distance_to(new_pos) > TELEPORT_DISTANCE_THRESHOLD
|
||||
|
||||
|
||||
# #501: Hub teleport transition — snap camera + 0.3s fade-from-black.
|
||||
# #501: Gauntlet dev teleport transition — snap camera + 0.3s fade-from-black.
|
||||
# Clears dialogue/monologue/interaction state (server clears its side too).
|
||||
# Scoped to Gauntlet testing only — production fast-travel uses diegetic gates.
|
||||
func _teleport_transition() -> void:
|
||||
# Snap camera: disable smoothing, force re-anchor
|
||||
# Snap camera: disable smoothing, force re-anchor.
|
||||
# _teleport_in_progress defers smoothing re-enable by one frame so the
|
||||
# re-enable block at the bottom of _process() doesn't undo the snap.
|
||||
camera.position_smoothing_enabled = false
|
||||
camera.global_position = GameState.player_position * Constants.TILE_SIZE
|
||||
_camera_anchored = true
|
||||
_teleport_in_progress = true
|
||||
|
||||
# Clear client-side buffers
|
||||
GameState.current_monologue = null
|
||||
|
||||
@@ -26,8 +26,8 @@ var _entity_targets: Dictionary = {} # entity_id -> Vector2 (target pixel positi
|
||||
var _entity_relationships: Dictionary = {} # #521: entity_id -> String (last relationship)
|
||||
var _entity_tweens: Dictionary = {} # #521: entity_id -> {target: Color, elapsed: float}
|
||||
|
||||
# #521: Color transition duration in seconds (D-033/D-063: 0.5-1s spec, 0.7s chosen)
|
||||
const COLOR_FADE_DURATION: float = 0.7
|
||||
# #521: Color transition duration in seconds (D-033: "0.5s fade")
|
||||
const COLOR_FADE_DURATION: float = 0.5
|
||||
|
||||
func _ready() -> void:
|
||||
print("EntityRenderer: Initialized")
|
||||
@@ -133,7 +133,7 @@ func _update_entity_node(entity_id: int, entity_data: Dictionary) -> void:
|
||||
floorf(entity_data.y) * TILE_SIZE + ENTITY_OFFSET
|
||||
)
|
||||
|
||||
# #521: Detect relationship change → fade D-033 color (0.7s via _process)
|
||||
# #521: Detect relationship change → fade D-033 color (0.5s via _process)
|
||||
var new_rel: String = entity_data.get("relationship", "Unknown")
|
||||
var old_rel: String = _entity_relationships.get(entity_id, "Unknown")
|
||||
if new_rel != old_rel:
|
||||
@@ -145,6 +145,10 @@ func _update_entity_node(entity_id: int, entity_data: Dictionary) -> void:
|
||||
"elapsed": 0.0,
|
||||
}
|
||||
|
||||
# Note: modulate.a (peripheral dimming below) and color (D-033 tint above)
|
||||
# are compositionally independent — both can change simultaneously without
|
||||
# interference. If alpha tweening is added later, coordinate with color tween.
|
||||
|
||||
# v2: Peripheral vision dimming (D-015)
|
||||
# null visibility (v1 backward compat) defaults to full alpha
|
||||
var visibility: Variant = entity_data.get("visibility")
|
||||
|
||||
@@ -240,6 +240,34 @@ func test_color_shift_reaches_target() -> void:
|
||||
assert_that(node.color.is_equal_approx(Constants.ENTITY_COLOR_HOSTILE)).is_true()
|
||||
renderer.queue_free()
|
||||
|
||||
func test_color_shift_mid_transition_retrigger() -> void:
|
||||
# Rapid relationship changes: Unknown → Friendly → Hostile in quick succession.
|
||||
# The second change should preempt the first tween and converge to Hostile.
|
||||
var renderer: Node2D = _make_entity_renderer()
|
||||
if not _entity_uses_relationship(renderer):
|
||||
renderer.queue_free()
|
||||
return
|
||||
if not _renderer_has_tween_support(renderer):
|
||||
renderer.queue_free()
|
||||
return
|
||||
renderer.update_entities([_make_entity(2, "Npc", "Unknown")])
|
||||
# First change: Unknown → Friendly
|
||||
renderer.update_entities([_make_entity(2, "Npc", "Friendly")])
|
||||
# Advance partway (0.1s of a 0.5s tween)
|
||||
for i in range(6):
|
||||
renderer._process(1.0 / 60.0)
|
||||
# Second change mid-tween: Friendly → Hostile (preempts first)
|
||||
renderer.update_entities([_make_entity(2, "Npc", "Hostile")])
|
||||
# Advance past full duration
|
||||
var elapsed: float = 0.0
|
||||
while elapsed < 1.0:
|
||||
renderer._process(1.0 / 60.0)
|
||||
elapsed += 1.0 / 60.0
|
||||
var node: ColorRect = renderer.entity_nodes[2] as ColorRect
|
||||
assert_that(node.color.is_equal_approx(Constants.ENTITY_COLOR_HOSTILE)).is_true()
|
||||
renderer.queue_free()
|
||||
|
||||
|
||||
func test_color_shift_same_relationship_no_tween() -> void:
|
||||
var renderer: Node2D = _make_entity_renderer()
|
||||
if not _entity_uses_relationship(renderer):
|
||||
|
||||
@@ -73,20 +73,34 @@ func test_teleport_hub_action_exists() -> void:
|
||||
|
||||
# -- InputMapper: Gauntlet mode guard -----------------------------------------
|
||||
|
||||
func test_teleport_hub_gated_by_gauntlet_mode() -> void:
|
||||
# Non-gauntlet: Home key should NOT queue TELEPORT_HUB
|
||||
func test_teleport_hub_blocked_outside_gauntlet() -> void:
|
||||
# Non-gauntlet: Home key input should NOT queue TELEPORT_HUB
|
||||
GameState.gauntlet_mode = false
|
||||
# Simulate what _unhandled_input does: check gauntlet_mode before queuing
|
||||
# (We test the guard logic, not the full input event pipeline)
|
||||
var should_queue := GameState.gauntlet_mode
|
||||
assert_that(should_queue).is_false()
|
||||
InputMapper.input_queue.clear()
|
||||
var event := InputEventKey.new()
|
||||
event.physical_keycode = KEY_HOME
|
||||
event.pressed = true
|
||||
InputMapper._unhandled_input(event)
|
||||
var has_teleport := false
|
||||
for entry in InputMapper.input_queue:
|
||||
if entry.action == InputMapper.Action.TELEPORT_HUB:
|
||||
has_teleport = true
|
||||
assert_that(has_teleport).is_false()
|
||||
|
||||
|
||||
func test_teleport_hub_allowed_in_gauntlet_mode() -> void:
|
||||
# Gauntlet mode: Home key SHOULD queue TELEPORT_HUB
|
||||
GameState.gauntlet_mode = true
|
||||
var should_queue := GameState.gauntlet_mode
|
||||
assert_that(should_queue).is_true()
|
||||
InputMapper.input_queue.clear()
|
||||
var event := InputEventKey.new()
|
||||
event.physical_keycode = KEY_HOME
|
||||
event.pressed = true
|
||||
InputMapper._unhandled_input(event)
|
||||
var has_teleport := false
|
||||
for entry in InputMapper.input_queue:
|
||||
if entry.action == InputMapper.Action.TELEPORT_HUB:
|
||||
has_teleport = true
|
||||
assert_that(has_teleport).is_true()
|
||||
|
||||
|
||||
# -- GameState: gauntlet_mode from snapshot ------------------------------------
|
||||
@@ -161,67 +175,70 @@ func test_test_mode_teleport_clears_dialogue() -> void:
|
||||
|
||||
|
||||
# -- Teleport detection -------------------------------------------------------
|
||||
# Threshold constant lives on the main scene node (TELEPORT_DISTANCE_THRESHOLD = 5.0).
|
||||
# These tests verify the distance math against that threshold.
|
||||
|
||||
const _THRESHOLD: float = 5.0 # Mirror of main.gd TELEPORT_DISTANCE_THRESHOLD
|
||||
|
||||
func test_detect_teleport_large_jump() -> void:
|
||||
# Position jump > 5 tiles should be detected as teleport
|
||||
# _detect_teleport is a method on the main scene — test the math directly
|
||||
# Position jump > threshold should be detected as teleport
|
||||
var old_pos := Vector2(10.0, 10.0)
|
||||
var new_pos := Vector2(50.0, 50.0)
|
||||
var distance := old_pos.distance_to(new_pos)
|
||||
assert_that(distance > 5.0).is_true()
|
||||
assert_that(old_pos.distance_to(new_pos) > _THRESHOLD).is_true()
|
||||
|
||||
|
||||
func test_detect_teleport_normal_movement() -> void:
|
||||
# Normal 1-tile movement should NOT be detected as teleport
|
||||
var old_pos := Vector2(10.0, 10.0)
|
||||
var new_pos := Vector2(11.0, 10.0)
|
||||
var distance := old_pos.distance_to(new_pos)
|
||||
assert_that(distance > 5.0).is_false()
|
||||
assert_that(old_pos.distance_to(new_pos) > _THRESHOLD).is_false()
|
||||
|
||||
|
||||
func test_detect_teleport_diagonal_movement() -> void:
|
||||
# Diagonal movement (1,1) — distance ~1.41, not a teleport
|
||||
var old_pos := Vector2(10.0, 10.0)
|
||||
var new_pos := Vector2(11.0, 11.0)
|
||||
var distance := old_pos.distance_to(new_pos)
|
||||
assert_that(distance > 5.0).is_false()
|
||||
assert_that(old_pos.distance_to(new_pos) > _THRESHOLD).is_false()
|
||||
|
||||
|
||||
func test_detect_teleport_boundary_exactly_five() -> void:
|
||||
# Exactly 5.0 tiles — should NOT trigger (threshold is > 5.0, not >=)
|
||||
func test_detect_teleport_boundary_exactly_threshold() -> void:
|
||||
# Exactly threshold — should NOT trigger (> not >=)
|
||||
var old_pos := Vector2(10.0, 10.0)
|
||||
var new_pos := Vector2(15.0, 10.0)
|
||||
var distance := old_pos.distance_to(new_pos)
|
||||
assert_that(distance > 5.0).is_false()
|
||||
assert_that(old_pos.distance_to(new_pos) > _THRESHOLD).is_false()
|
||||
|
||||
|
||||
func test_detect_teleport_boundary_just_over_five() -> void:
|
||||
# 5.1 tiles — should trigger
|
||||
func test_detect_teleport_boundary_just_over() -> void:
|
||||
# Just over threshold — should trigger
|
||||
var old_pos := Vector2(10.0, 10.0)
|
||||
var new_pos := Vector2(15.1, 10.0)
|
||||
var distance := old_pos.distance_to(new_pos)
|
||||
assert_that(distance > 5.0).is_true()
|
||||
assert_that(old_pos.distance_to(new_pos) > _THRESHOLD).is_true()
|
||||
|
||||
|
||||
# -- Buffer clearing on teleport -----------------------------------------------
|
||||
|
||||
func test_teleport_clears_monologue_state() -> void:
|
||||
# Teleport transition must clear current_monologue
|
||||
GameState.current_monologue = {"id": "test_mono", "text": "test", "duration_seconds": 5.0}
|
||||
# Simulate what _teleport_transition does
|
||||
GameState.current_monologue = null
|
||||
assert_that(GameState.current_monologue).is_null()
|
||||
func test_teleport_clears_dialogue_in_test_mode() -> void:
|
||||
# TeleportToHub in test mode should clear dialogue state via the pipeline
|
||||
SimBridge.reset_test_state()
|
||||
SimBridge._test_in_dialogue = true
|
||||
SimBridge._test_input_queue.append("TeleportToHub")
|
||||
var snap: Dictionary = SimBridge._test_snapshot()
|
||||
# Dialogue should be cleared by teleport
|
||||
assert_that(SimBridge._test_in_dialogue).is_false()
|
||||
assert_that(snap.current_dialogue).is_null()
|
||||
|
||||
|
||||
func test_teleport_clears_dialogue_state() -> void:
|
||||
# Teleport transition must clear current_dialogue and dialogue_active
|
||||
GameState.current_dialogue = {"npc_name": "Kael", "speech": "test", "options": []}
|
||||
GameState.dialogue_active = true
|
||||
# Simulate what _teleport_transition does
|
||||
GameState.current_dialogue = null
|
||||
GameState.dialogue_active = false
|
||||
assert_that(GameState.current_dialogue).is_null()
|
||||
assert_that(GameState.dialogue_active).is_false()
|
||||
func test_teleport_resets_position_in_test_mode() -> void:
|
||||
# TeleportToHub must reset to hub spawn and clear dialogue (integration)
|
||||
SimBridge.reset_test_state()
|
||||
SimBridge._test_player_pos = Vector2i(50, 50)
|
||||
SimBridge._test_in_dialogue = true
|
||||
SimBridge._test_input_queue.append("TeleportToHub")
|
||||
var snap: Dictionary = SimBridge._test_snapshot()
|
||||
var player: Dictionary = snap.entities[0]
|
||||
assert_that(player.x).is_equal(10.0)
|
||||
assert_that(player.y).is_equal(10.0)
|
||||
assert_that(SimBridge._test_in_dialogue).is_false()
|
||||
|
||||
|
||||
# -- Send input integration (test mode) ----------------------------------------
|
||||
@@ -248,6 +265,17 @@ func test_send_teleport_hub_queues_wire_action() -> void:
|
||||
assert_that(SimBridge._test_input_queue.has("TeleportToHub")).is_true()
|
||||
|
||||
|
||||
func test_gauntlet_mode_from_test_snapshot() -> void:
|
||||
# Verify test snapshot includes gauntlet_mode field
|
||||
SimBridge.reset_test_state()
|
||||
SimBridge._test_gauntlet_mode = true
|
||||
var snap: Dictionary = SimBridge._test_snapshot()
|
||||
assert_that(snap.gauntlet_mode).is_true()
|
||||
SimBridge._test_gauntlet_mode = false
|
||||
snap = SimBridge._test_snapshot()
|
||||
assert_that(snap.gauntlet_mode).is_false()
|
||||
|
||||
|
||||
# -- Live mode outbound encoding -----------------------------------------------
|
||||
|
||||
func test_teleport_hub_outbound_entry() -> void:
|
||||
|
||||
Reference in New Issue
Block a user