fix(client): T-1146 review round — gate the T-1088 seams, kill the settle fallback tautology, neutralize AutoPause leak
PR #190 review (Hoshe + Tyre), all five findings addressed: - Tyre: queue_move_step/queue_stance_toggle (the T-1088 click-to-move seams) now share the full suppression predicate — all three input_queue producers in the module are gated, making the header's 'every action this file can produce' claim true (previously the seams relied on the sandbox caller's own freeze). +3 regression tests. - Hoshe: the live suite's settle=baseline timeout fallback compared baseline to itself, vacuously passing the no-movement claim — a timeout now fails loudly (both tests). - Hoshe: the unit suite's real open_app/close_app transitions fire the global auto-pause handler; with ambient SimBridge.state left CONNECTED by earlier suites (test_hub_teleport has no after_test) each transition silently appended AutoPause/AutoResume to the shared _outbound_buffer. before_test now forces DISCONNECTED (restored in after_test) and clears the buffer. - Hoshe: documented why the live suite's reset_test_state() call is inert but still correct. Full suite: 3374/3374.
This commit is contained in:
@@ -290,7 +290,12 @@ func flush_queue() -> Array[Dictionary]:
|
||||
# false when throttled or input-suppressed. Ordinary Move* actions only: zero
|
||||
# protocol change, no client prediction, the server validates every step (D-010).
|
||||
func queue_move_step(tile_delta: Vector2i) -> bool:
|
||||
if GameState.dialogue_active or GameState.free_camera_mode:
|
||||
# T-1146: same suppression predicate as the _process poll — all three
|
||||
# input_queue producers in this module share it, so the occlusion gate is
|
||||
# self-contained here rather than delegated to seam callers (today the
|
||||
# sandbox freezes the path follower itself, but Phase 5/T-962 lifts
|
||||
# click-to-move into the shared session driver).
|
||||
if GameState.dialogue_active or GameState.free_camera_mode or _gameplay_occluded:
|
||||
return false
|
||||
if tile_delta == Vector2i.ZERO:
|
||||
return false
|
||||
@@ -312,9 +317,9 @@ func queue_move_step(tile_delta: Vector2i) -> bool:
|
||||
# Sprint, down = toward Crouch) through the same input_queue movement rides. Discrete,
|
||||
# so no throttle — the server's handle_toggle_stance has no cooldown, so the sandbox
|
||||
# follower can burst the exact ladder distance and trust it (server validates). No-op
|
||||
# while input is suppressed (dialogue / free camera), matching movement.
|
||||
# while input is suppressed (dialogue / free camera / occlusion), matching movement.
|
||||
func queue_stance_toggle(up: bool) -> void:
|
||||
if GameState.dialogue_active or GameState.free_camera_mode:
|
||||
if GameState.dialogue_active or GameState.free_camera_mode or _gameplay_occluded:
|
||||
return
|
||||
input_queue.append(
|
||||
{
|
||||
|
||||
@@ -132,6 +132,9 @@ func after_test() -> void:
|
||||
SimBridge._bridge = _saved_bridge
|
||||
SimBridge.server_path = _saved_server_path
|
||||
SimBridge.server_port = _saved_server_port
|
||||
# reset_test_state() touches only mock-side state (harness.reset() when a
|
||||
# harness exists, star-map flag, SystemIndex cache) — inert for this live
|
||||
# suite, but it restores whatever ambient mock the run left for later suites.
|
||||
SimBridge.reset_test_state()
|
||||
# Don't let MY servers' snapshots/inputs poison later suites either.
|
||||
SimBridge._last_snapshot = null
|
||||
@@ -283,10 +286,15 @@ func test_wasd_held_during_implant_occlusion_does_not_move_player(
|
||||
Input.action_release("move_north")
|
||||
|
||||
# Drain whatever the server actually streamed during the hold and confirm
|
||||
# the player never left the starting tile.
|
||||
# the player never left the starting tile. A timeout here is a FAILURE, not
|
||||
# a fallback case: substituting baseline would compare baseline to itself
|
||||
# and vacuously pass the very claim under test (Hoshe, PR #190 review).
|
||||
var settle: Dictionary = await _await_next_snapshot(2.0)
|
||||
assert_bool(not settle.is_empty()).override_failure_message(
|
||||
"no snapshot arrived within 2s after the hold — cannot verify the no-movement claim"
|
||||
).is_true()
|
||||
if settle.is_empty():
|
||||
settle = baseline
|
||||
return
|
||||
var after_hold := _find_player(settle)
|
||||
assert_that(after_hold.size()).is_greater(0)
|
||||
if after_hold.is_empty():
|
||||
@@ -348,9 +356,13 @@ func test_close_while_held_requires_repress_before_movement_resumes_live(
|
||||
for _i in range(10):
|
||||
await _tick(0.05)
|
||||
|
||||
# Timeout is a failure, not a fallback — see the sibling test's comment.
|
||||
var settle_still_held: Dictionary = await _await_next_snapshot(2.0)
|
||||
assert_bool(not settle_still_held.is_empty()).override_failure_message(
|
||||
"no snapshot arrived within 2s after the held-through-close phase — cannot verify the no-resume claim"
|
||||
).is_true()
|
||||
if settle_still_held.is_empty():
|
||||
settle_still_held = baseline
|
||||
return
|
||||
var still_held_player := _find_player(settle_still_held)
|
||||
assert_that(still_held_player.size()).is_greater(0)
|
||||
if still_held_player.is_empty():
|
||||
|
||||
@@ -15,12 +15,26 @@ extends GdUnitTestSuite
|
||||
|
||||
const TEST_APP := "implant/t1146_occlusion_test"
|
||||
|
||||
# Ambient SimBridge state to restore in after_test — see the neutralization
|
||||
# comment in before_test.
|
||||
var _saved_sim_state: int = 0
|
||||
|
||||
|
||||
func before_test() -> void:
|
||||
GameState.player_stance = "Walk"
|
||||
GameState.dialogue_active = false
|
||||
GameState.free_camera_mode = false
|
||||
GameState.gauntlet_mode = false
|
||||
# This suite never touches SimBridge, but every real open_app()/close_app()
|
||||
# transition below fires HudGroups' global auto-pause handler, which calls
|
||||
# SimBridge.send_named_action("AutoPause"/"AutoResume") — a silent append to
|
||||
# the shared _outbound_buffer whenever ambient state is CONNECTED (which
|
||||
# other suites leave behind, e.g. test_hub_teleport.gd sets CONNECTED with
|
||||
# no after_test). Force DISCONNECTED for the duration so those sends no-op,
|
||||
# and clear any entries already leaked into the buffer (Hoshe, PR #190).
|
||||
_saved_sim_state = SimBridge.state
|
||||
SimBridge.state = SimBridge.ConnectionState.DISCONNECTED
|
||||
SimBridge._outbound_buffer.clear()
|
||||
InputMapper.input_queue.clear()
|
||||
InputMapper._gameplay_occluded = false
|
||||
InputMapper._suppress_move_until_release = false
|
||||
@@ -40,6 +54,7 @@ func before_test() -> void:
|
||||
|
||||
|
||||
func after_test() -> void:
|
||||
SimBridge.state = _saved_sim_state
|
||||
InputMapper.input_queue.clear()
|
||||
InputMapper._gameplay_occluded = false
|
||||
InputMapper._suppress_move_until_release = false
|
||||
@@ -209,6 +224,42 @@ func test_unoccluded_interact_press_queues_normally() -> void:
|
||||
assert_int(InputMapper.input_queue[0]["action"]).is_equal(InputMapper.Action.INTERACT)
|
||||
|
||||
|
||||
# =============================================================================
|
||||
# T-1088 click-to-move seams share the same suppression predicate — all three
|
||||
# input_queue producers in the module are gated (Tyre, PR #190 review)
|
||||
# =============================================================================
|
||||
|
||||
|
||||
func test_occluded_queue_move_step_queues_nothing() -> void:
|
||||
InputMapper._gameplay_occluded = true
|
||||
_prime_throttle_ready()
|
||||
var queued := InputMapper.queue_move_step(Vector2i(0, -1))
|
||||
assert_bool(queued).is_false()
|
||||
assert_int(InputMapper.input_queue.size()).override_failure_message(
|
||||
"queue_move_step must not enqueue while gameplay is occluded"
|
||||
).is_equal(0)
|
||||
|
||||
|
||||
func test_occluded_queue_stance_toggle_queues_nothing() -> void:
|
||||
InputMapper._gameplay_occluded = true
|
||||
InputMapper.queue_stance_toggle(true)
|
||||
assert_int(InputMapper.input_queue.size()).override_failure_message(
|
||||
"queue_stance_toggle must not enqueue while gameplay is occluded"
|
||||
).is_equal(0)
|
||||
|
||||
|
||||
func test_unoccluded_seams_queue_normally() -> void:
|
||||
_prime_throttle_ready()
|
||||
var queued := InputMapper.queue_move_step(Vector2i(0, -1))
|
||||
assert_bool(queued).is_true()
|
||||
InputMapper.queue_stance_toggle(true)
|
||||
assert_int(InputMapper.input_queue.size()).override_failure_message(
|
||||
"both seams must keep queueing normally while gameplay is not occluded"
|
||||
).is_equal(2)
|
||||
assert_int(InputMapper.input_queue[0]["action"]).is_equal(InputMapper.Action.MOVE_NORTH)
|
||||
assert_int(InputMapper.input_queue[1]["action"]).is_equal(InputMapper.Action.TOGGLE_STANCE_UP)
|
||||
|
||||
|
||||
# =============================================================================
|
||||
# INSERT mode must NOT occlude/gate — e.g. main.gd's "implant/economics" panel
|
||||
# (D-181/#824) renders alongside still-playable gameplay.
|
||||
|
||||
Reference in New Issue
Block a user