diff --git a/src/autoload/net.gd b/src/autoload/net.gd index dd6b583..8023b7d 100644 --- a/src/autoload/net.gd +++ b/src/autoload/net.gd @@ -13,6 +13,11 @@ signal state_changed(new_state: State) enum State { OFFLINE, CONNECTING, ONLINE, FAILED } +## How long a refused peer is left connected so its rejection can actually go +## out. Long enough to survive a frame's flush and the round trip, short enough +## that nobody notices; see [method reject_and_kick]. +const REJECT_LINGER := 0.5 + ## Peer id the engine gives the host. On a listen server this is also a real ## player, which is why every send goes through the helpers below. const LOCAL_PEER := 1 @@ -97,9 +102,32 @@ func shutdown() -> void: _set_state(State.OFFLINE) +## Tell a peer why it is not welcome, then close the socket. +## +## The kick is DEFERRED, which is the whole point of this existing. An RPC is +## queued and only leaves on the frame's flush, so disconnecting in the same +## call throws the reason away with the packet -- a client turned away for a +## protocol mismatch was told only that the server closed the connection. The +## peer can do nothing during the wait: it has no account bound, and every +## message that needs one already refuses it. +func reject_and_kick(peer_id: int, reason: String) -> void: + send_reject(peer_id, reason) + if _is_local(peer_id): + return + get_tree().create_timer(REJECT_LINGER).timeout.connect( + func() -> void: kick(peer_id)) + + func kick(peer_id: int) -> void: - if multiplayer.multiplayer_peer is ENetMultiplayerPeer: - (multiplayer.multiplayer_peer as ENetMultiplayerPeer).disconnect_peer(peer_id) + if multiplayer.multiplayer_peer is not ENetMultiplayerPeer: + return + # The peer may have hung up first. A rejected client closes its own socket + # as soon as it reads the reason, and reject_and_kick's timer lands after + # that -- asking ENet to disconnect someone already gone is an engine error, + # not a quiet no-op. + if not multiplayer.get_peers().has(peer_id): + return + (multiplayer.multiplayer_peer as ENetMultiplayerPeer).disconnect_peer(peer_id) func is_server() -> bool: @@ -130,8 +158,14 @@ func _on_connect_failed() -> void: func _on_server_disconnected() -> void: - last_error = "server closed the connection" - GameLog.warn("net", last_error) + # A rejection lands immediately before the socket closes, and it is the + # useful half of the pair: "protocol mismatch: server 9, client 8" is + # actionable and "server closed the connection" is not. Overwriting it here + # made every reject reason in the game unreachable -- the reason was sent, + # decoded, stored, and then clobbered a frame later by this line. + if state != State.FAILED: + last_error = "server closed the connection" + GameLog.warn("net", last_error) shutdown() diff --git a/src/core/movement.gd b/src/core/movement.gd index 8ef544f..160d89f 100644 --- a/src/core/movement.gd +++ b/src/core/movement.gd @@ -37,6 +37,18 @@ static func outside_map(p: Vector2, map: MapGrid) -> bool: return not map.world_rect().grow(SimConfig.BULLET_CULL_MARGIN).has_point(p) +## The furthest a bullet with these parameters could travel before it expires. +## +## Used to decide whether a spawn is worth telling a client about. Deliberately +## an OVER-estimate: acceleration is integrated at face value while a negative +## one is ignored, and a turning bullet curves, so the straight-line answer is +## always at least the real path. Over-estimating costs a packet; under- +## estimating is a bullet that arrives invisible. +static func bullet_reach(speed: float, accel: float, life_ticks: int) -> float: + var t := float(maxi(life_ticks, 0)) * SimConfig.TICK_DELTA + return maxf(speed, 0.0) * t + 0.5 * maxf(accel, 0.0) * t * t + + ## Outside the map, or stopped by geometry. static func bullet_stopped(p: Vector2, map: MapGrid) -> bool: if map == null: diff --git a/src/net/server_runtime.gd b/src/net/server_runtime.gd index 47ceb1b..aad129e 100644 --- a/src/net/server_runtime.gd +++ b/src/net/server_runtime.gd @@ -206,14 +206,29 @@ func on_hello(peer_id: int, version: int, ticket: PackedByteArray) -> void: return # a second hello from the same peer is either a bug or an attack if version != Protocol.VERSION: GameLog.warn("server", "peer %d protocol %d != %d, rejecting" % [peer_id, version, Protocol.VERSION]) - Net.send_reject(peer_id, "protocol mismatch: server %d, client %d" % [Protocol.VERSION, version]) - Net.kick(peer_id) + Net.reject_and_kick(peer_id, + "protocol mismatch: server %d, client %d" % [Protocol.VERSION, version]) return var account := Net.auth.validate(ticket) if account == AuthProvider.NO_ACCOUNT: GameLog.warn("server", "peer %d failed authentication" % peer_id) - Net.send_reject(peer_id, "authentication failed") - Net.kick(peer_id) + Net.reject_and_kick(peer_id, "authentication failed") + return + # One account, one session. Two peers on one account each get their own + # SimPlayer built from the SAME character record, so both hold a copy of the + # same bag and whichever writes last wins -- drop the one potion from both + # copies and two potions hit the floor. Refusing the second connection is the + # only fix that closes it; reconciling two live mirrors of one record after + # the fact cannot be made to work. + # + # The NEW peer is refused rather than the old one kicked, deliberately: with + # an auth provider anyone can spoof, "kick whoever holds this account" would + # be a way to disconnect any player on the server at will. + var held_by := peer_for_account(account) + if held_by != 0: + GameLog.warn("server", "peer %d claimed account %d, already held by peer %d" + % [peer_id, account, held_by]) + Net.reject_and_kick(peer_id, "that account is already playing") return peer_accounts[peer_id] = account Net.send_welcome(peer_id) @@ -374,6 +389,22 @@ func instance_of(peer_id: int) -> Instance: return instances.get(id) +## The peer currently signed in as [param account], or 0 for none. +## +## A peer that dropped inside a dungeon still counts while its linkdead body is +## channelling out, which is the point: reconnecting during that second would +## put the same character in the world twice -- once as the body still being +## shot at, once fresh in the hub -- and a death on the abandoned body would +## then retire the character being played. +func peer_for_account(account: int) -> int: + if account == AuthProvider.NO_ACCOUNT: + return 0 + for peer in peer_accounts: + if peer_accounts[peer] == account: + return peer + return 0 + + # --- Transfers -------------------------------------------------------------- func _place(peer_id: int, inst: Instance) -> void: @@ -655,30 +686,58 @@ func _broadcast_roster() -> void: Net.send_roster(peer, payload) -## Bullet spawns within this peer's interest radius. See -## SimConfig.BULLET_INTEREST_RADIUS for why this radius is much wider than the -## one used for actors. +## Bullet spawns this peer could actually be reached by. +## +## Tested per bullet rather than against a flat radius, and that distinction is +## the whole point. A spawn event carries the firing actor's exact position, so +## a radius wide enough for the fastest shot in the game hands a modified client +## the location of every enemy inside it -- 2900 units, against an 800-unit +## actor cull and a 900-unit map stream. At the depth the game actually runs at +## the map's own diagonal is about 2521, so the flat radius culled NOTHING: an +## enemy firing anywhere on the floor was pinpointed for everyone, through the +## fog, on terrain the client had deliberately not been streamed. +## +## Asking instead whether THIS bullet can reach ground this player can see keeps +## the guarantee the wide radius existed for -- nothing ever arrives invisible -- +## while a slow, short-lived shot across the map stops being a free radar ping. func _spawns_near(inst: Instance, peer_id: int, spawns: Array[Dictionary]) -> Array[Dictionary]: var p: SimPlayer = inst.world.players.get(peer_id) if p == null: return spawns - var cull_sq := SimConfig.BULLET_INTEREST_RADIUS * SimConfig.BULLET_INTEREST_RADIUS var out: Array[Dictionary] = [] for ev in spawns: - if p.pos.distance_squared_to(ev["pos"]) <= cull_sq: + if _bullet_concerns(p.pos, ev["pos"], ev["vel"], ev["accel"], ev["life"]): out.append(ev) return out +## Whether a bullet born at [param at] could reach ground visible from +## [param eye] before it expires. BULLET_INTEREST_RADIUS stays on as a hard +## ceiling: test_interest.gd proves no bullet in the game needs more than it, so +## it can only ever agree with the per-bullet answer -- but if some future +## content outruns the computation, the failure is a culled bullet rather than +## an unbounded broadcast. +func _bullet_concerns(eye: Vector2, at: Vector2, vel: Vector2, accel: float, + life: int) -> bool: + var horizon := minf( + Movement.bullet_reach(vel.length(), accel, life) + SimConfig.FOG_VIEW_RADIUS, + SimConfig.BULLET_INTEREST_RADIUS) + return eye.distance_squared_to(at) <= horizon * horizon + + func _live_bullet_events(world: SimWorld, for_peer: int = 0) -> Array[Dictionary]: var out: Array[Dictionary] = [] var observer: SimPlayer = world.players.get(for_peer) if for_peer != 0 else null - var cull_sq := SimConfig.BULLET_INTEREST_RADIUS * SimConfig.BULLET_INTEREST_RADIUS for i in world.pool.high_water: if world.pool.alive[i] == 0: continue - if observer != null \ - and observer.pos.distance_squared_to(world.pool.pos[i]) > cull_sq: + # Same per-bullet question as _spawns_near. The backlog a joiner is sent + # is a snapshot of every live bullet, so a flat radius here leaked the + # position of every firing enemy in the instance the moment you walked + # in -- the one moment you have seen none of the map yet. + if observer != null and not _bullet_concerns(observer.pos, + world.pool.pos[i], world.pool.vel[i], world.pool.accel[i], + world.pool.life[i]): continue out.append({ "t": SimEvent.Type.BULLET_SPAWN, diff --git a/src/sim/map_grid.gd b/src/sim/map_grid.gd index 319c605..bd4ebe9 100644 --- a/src/sim/map_grid.gd +++ b/src/sim/map_grid.gd @@ -177,7 +177,31 @@ func slide_circle(pos: Vector2, delta: Vector2, radius: float) -> Vector2: ## Bresenham-style sight test between two world points. Used for fog on the ## client and for aggro on the server, so it has to agree on both. +## +## [param include_end] is false here: the endpoints themselves never block, so +## standing in a doorway, or shooting at something embedded in a wall, still +## resolves. func has_line_of_sight(from: Vector2, to: Vector2) -> bool: + return _walk_clear(from, to, BLOCKS_SIGHT, false) + + +## Whether a bullet could travel from [param from] to [param to] without +## crossing geometry. +## +## Needed because a muzzle offset is a teleport: [BulletPool] samples position +## once per tick and never sees the jump from a player's centre out to where the +## shot is actually born, so anything that places a bullet away from its shooter +## has to ask this first. Unlike sight, the DESTINATION counts -- a bullet born +## inside a wall is exactly the case being ruled out. +func bullet_path_clear(from: Vector2, to: Vector2) -> bool: + return _walk_clear(from, to, BLOCKS_BULLET, true) + + +## Shared Bresenham walk behind both tests above, so the two can never disagree +## about which tiles a straight line crosses. [param flags] is one of the +## BLOCKS_* tables; [param include_end] decides whether the destination tile is +## allowed to stop the walk. +func _walk_clear(from: Vector2, to: Vector2, flags: Array, include_end: bool) -> bool: var a := to_tile(from) var b := to_tile(to) var dx := absi(b.x - a.x) @@ -192,12 +216,13 @@ func has_line_of_sight(from: Vector2, to: Vector2) -> bool: var limit := width + height + 4 while steps < limit: steps += 1 - if x == b.x and y == b.y: + var at_end := x == b.x and y == b.y + if at_end and not include_end: return true - # The endpoints themselves never block: standing in a doorway, or - # shooting at something embedded in a wall, must still resolve. - if not (x == a.x and y == a.y) and blocks_sight(x, y): + if not (x == a.x and y == a.y) and bool(flags[at(x, y)]): return false + if at_end: + return true var e2 := 2 * err if e2 >= dy: err += dy diff --git a/src/sim/sim_world.gd b/src/sim/sim_world.gd index 58f6c5b..0334005 100644 --- a/src/sim/sim_world.gd +++ b/src/sim/sim_world.gd @@ -317,9 +317,21 @@ func _fire_player_shot(p: SimPlayer) -> void: forward) +## Place one shot. [param at] is where the muzzle wants it; if geometry stands +## between the shooter and that point the bullet is born on the shooter instead. +## +## A muzzle offset is a teleport, and [BulletPool] never sees it: it samples +## geometry once per tick, so a bullet that starts on the far side of a wall +## simply flies away down the other side. Forward that is only 19px, but +## Doubleshot adds up to 45px SIDEWAYS -- more than a 32px tile is thick, plus +## the 6px hitbox that keeps you off the wall. Six Doubleshots and standing +## flush against a wall was a wallhack reachable with a stock client. Falling +## back to p.pos rather than dropping the shot keeps the shot count honest: +## you fired what the upgrade says you fired, it just left from your centre. func _spawn_player_bullet(p: SimPlayer, at: Vector2, dir: Vector2) -> void: + var from := at if map.bullet_path_clear(p.pos, at) else p.pos var slot := pool.spawn( - at, + from, dir * p.stats.bullet_speed, SimConfig.PLAYER_BULLET_RADIUS, SimConfig.PLAYER_BULLET_LIFETIME, diff --git a/tests/unit/test_interest.gd b/tests/unit/test_interest.gd index a2787d2..7cd562f 100644 --- a/tests/unit/test_interest.gd +++ b/tests/unit/test_interest.gd @@ -112,3 +112,75 @@ func test_no_stack_of_upgrades_can_outrun_the_bullet_radius() -> void: var travel := PlayerStats.build(ids).bullet_speed \ * float(SimConfig.PLAYER_BULLET_LIFETIME) * SimConfig.TICK_DELTA assert_gte(SimConfig.BULLET_INTEREST_RADIUS, travel + SimConfig.FOG_VIEW_RADIUS) + + +# --- Per-bullet interest ---------------------------------------------------- +# The radius tests above pin the CEILING: no bullet in the game may need more +# than BULLET_INTEREST_RADIUS. These pin the rule that actually runs, which asks +# whether a particular bullet can reach ground this player can see. The two are +# different questions, and only the second one closes the leak: a spawn event +# carries the firing actor's exact position, so anything sent on the strength of +# a flat radius is an enemy located for a modified client. + +func _server() -> ServerRuntime: + # Not added to the tree on purpose: _ready() would build a lobby and load a + # character store, and none of that is involved in an interest decision. + return autofree(ServerRuntime.new()) as ServerRuntime + + +func test_a_slow_bullet_fired_across_the_map_is_not_announced() -> void: + var srv := _server() + var slow := Vector2(120.0, 0.0) # 120 u/s for 60 ticks reaches 120 units + assert_false(srv._bullet_concerns(Vector2.ZERO, Vector2(2000.0, 0.0), + slow, 0.0, 60), + "it expires 1400 units short of anything this player can see") + + +func test_a_bullet_that_can_reach_visible_ground_is_announced() -> void: + var srv := _server() + var fast := Vector2(900.0, 0.0) # 900 u/s for 90 ticks reaches 1350 + assert_true(srv._bullet_concerns(Vector2.ZERO, Vector2(1500.0, 0.0), + fast, 0.0, 90), + "1350 of travel plus the fog radius covers 1500, so it must be sent") + + +func test_the_horizon_is_travel_plus_what_the_player_can_see() -> void: + var srv := _server() + # A bullet whose entire travel lands exactly on the edge of sight. + var speed := 600.0 + var life := 60 + var reach := Movement.bullet_reach(speed, 0.0, life) + var vel := Vector2(speed, 0.0) + assert_true(srv._bullet_concerns(Vector2.ZERO, + Vector2(reach + SimConfig.FOG_VIEW_RADIUS - 1.0, 0.0), vel, 0.0, life)) + assert_false(srv._bullet_concerns(Vector2.ZERO, + Vector2(reach + SimConfig.FOG_VIEW_RADIUS + 1.0, 0.0), vel, 0.0, life)) + + +func test_acceleration_counts_toward_the_reach() -> void: + var srv := _server() + var at := Vector2(1200.0, 0.0) + var vel := Vector2(200.0, 0.0) + assert_false(srv._bullet_concerns(Vector2.ZERO, at, vel, 0.0, 90), + "200 u/s for 1.5s is 300 units; nowhere near") + assert_true(srv._bullet_concerns(Vector2.ZERO, at, vel, 900.0, 90), + "the same shot accelerating gets there, so it has to be announced") + + +func test_no_enemy_in_the_content_leaks_its_position_across_a_dungeon() -> void: + # The regression this all exists for. A depth-1 map is 2112x1376 units, so + # its diagonal is about 2521 -- less than BULLET_INTEREST_RADIUS. Under the + # flat radius every firing enemy anywhere on the floor was announced to + # everyone, whatever the fog or the actor cull said. + var srv := _server() + var diagonal := Vector2(66.0, 43.0).length() * MapGrid.TILE + assert_lt(diagonal, SimConfig.BULLET_INTEREST_RADIUS, + "if this stops being true the flat radius was not the bug described here") + var leaked: Array[String] = [] + for id in Content.ALL_ENEMIES: + for e in Content.enemy(id).emitters: + if srv._bullet_concerns(Vector2.ZERO, Vector2(diagonal, 0.0), + Vector2(e.speed, 0.0), 0.0, e.lifetime): + leaked.append("%s (%.0f u/s x %d ticks)" % [id, e.speed, e.lifetime]) + assert_eq(leaked, [] as Array[String], + "an enemy firing at the far corner of the map must not be announced") diff --git a/tests/unit/test_map_grid.gd b/tests/unit/test_map_grid.gd index 33ce2eb..f7fbedd 100644 --- a/tests/unit/test_map_grid.gd +++ b/tests/unit/test_map_grid.gd @@ -102,3 +102,33 @@ func test_standing_inside_a_wall_can_still_see_out() -> void: var inside := grid.tile_centre(0, 0) var outside := grid.tile_centre(1, 1) assert_true(grid.has_line_of_sight(inside, outside)) + + +## bullet_path_clear differs from has_line_of_sight in two ways that matter, and +## both are easy to get wrong: it consults BLOCKS_BULLET (so a barricade stops +## it and a pit does not), and the destination tile counts. +func test_a_bullet_path_is_blocked_by_what_stops_bullets_not_by_what_stops_sight() -> void: + var a := grid.tile_centre(1, 4) + var b := grid.tile_centre(8, 4) + assert_true(grid.bullet_path_clear(a, b), "clear floor between them") + + grid.set_tile(4, 4, MapGrid.Kind.PIT) + assert_true(grid.bullet_path_clear(a, b), "bullets fly over a pit") + + grid.set_tile(4, 4, MapGrid.Kind.BARRICADE) + assert_false(grid.bullet_path_clear(a, b), + "you can SEE over a barricade, which is exactly why sight is the wrong test") + assert_true(grid.has_line_of_sight(a, b), "and sight still passes it") + + grid.set_tile(4, 4, MapGrid.Kind.WALL) + assert_false(grid.bullet_path_clear(a, b), "a wall stops both") + + +func test_a_bullet_path_ending_inside_a_wall_is_refused() -> void: + var from := grid.tile_centre(1, 4) + grid.set_tile(2, 4, MapGrid.Kind.WALL) + assert_false(grid.bullet_path_clear(from, grid.tile_centre(2, 4)), + "the destination counts: a muzzle inside a wall is the case this rules out") + # Sight deliberately does the opposite -- shooting at something embedded in + # a wall must still resolve -- so the two must not be collapsed into one. + assert_true(grid.has_line_of_sight(from, grid.tile_centre(2, 4))) diff --git a/tests/unit/test_server_authority.gd b/tests/unit/test_server_authority.gd index 5a0ef54..92d4fa5 100644 --- a/tests/unit/test_server_authority.gd +++ b/tests/unit/test_server_authority.gd @@ -251,3 +251,58 @@ func test_spawn_grace_expires() -> void: assert_false(p.invulnerable()) _drive(1, Vector2.ZERO, InputFrame.BTN_FIRE) assert_eq(world.pool.live_count, 1, "the gun comes back once grace ends") + + +## A muzzle offset moves a bullet's birthplace away from its shooter, and the +## bullet pool samples geometry once per tick so it never sees that jump. With +## six Doubleshots the sideways offset reaches 45px -- wider than a 32px wall +## plus the 6px hitbox that holds you off it -- so standing flush against a wall +## and firing along it put live shots down the far side. No modified client +## needed, which is what made it worth a test rather than a comment. +func _wall_at_column(column: int) -> SimWorld: + var g := MapGrid.new(10, 6, MapGrid.Kind.FLOOR) + for ty in 6: + g.set_tile(column, ty, MapGrid.Kind.WALL) + var w := SimWorld.new(1) + w.set_map(g) + return w + + +func _fire_flush_against_the_wall(doubleshots: int) -> Dictionary: + var w := _wall_at_column(4) + var p := w.add_player(PEER, "tester") + var face: float = w.map.origin.x + 4.0 * MapGrid.TILE + p.pos = Vector2(face - SimConfig.PLAYER_RADIUS, w.map.tile_centre(3, 3).y) + p.aim = -PI / 2.0 # along the wall, not into it + p.spawn_grace = 0 + p.fire_cooldown = 0 + var ids: Array[StringName] = [] + for _i in doubleshots: + ids.append(Upgrades.DOUBLESHOT) + p.stats = PlayerStats.build(ids) + + w._fire_player_shot(p) + var fired := 0 + var past := 0 + for i in w.pool.high_water: + if w.pool.alive[i] == 0: + continue + fired += 1 + if w.pool.pos[i].x > face: + past += 1 + return {"fired": fired, "past_the_face": past} + + +func test_no_stack_of_upgrades_can_put_a_shot_through_a_wall() -> void: + for doubleshots in [0, 5, 6, 10]: + var r := _fire_flush_against_the_wall(doubleshots) + assert_eq(int(r["past_the_face"]), 0, + "%d Doubleshot(s) put %d shot(s) on the far side of the wall" + % [doubleshots, r["past_the_face"]]) + + +func test_clamping_the_muzzle_does_not_swallow_the_shot() -> void: + # The fix must not silently reduce your firepower for standing near cover: + # a blocked muzzle falls back to the shooter's centre, it does not cancel. + assert_eq(int(_fire_flush_against_the_wall(6)["fired"]), 7, + "six Doubleshots is seven projectiles, wall or no wall") diff --git a/tools/smoke.sh b/tools/smoke.sh index 98c7662..0bfad0a 100755 --- a/tools/smoke.sh +++ b/tools/smoke.sh @@ -55,6 +55,22 @@ for n in 1 2; do sleep 0.4 done +# A bot that claims an account another bot is already playing. One account is +# one session: two peers on one account each get their own player built from the +# SAME character record, so both hold a copy of one bag and whichever writes last +# wins -- drop the single potion from both copies and two potions hit the floor. +# The handshake has to refuse it, and it has to refuse the NEWCOMER, or claiming +# someone's account becomes a way to kick them off the server. +for _ in $(seq 1 60); do + grep -q "authenticated as account 7001" "$OUT/server.log" 2>/dev/null && break + sleep 0.25 +done +"$GODOT" --headless --path . -- --bot --host 127.0.0.1 --port "$PORT" \ + --name "twinbot" --account 7001 --autoquit 300 \ + > "$OUT/twinbot.log" 2>&1 & +PIDS+=($!) +sleep 0.4 + # A fourth bot that leaves *politely* -- the same Net.shutdown() the in-game # menu's "Disconnect" button calls. The escape channel is keyed on the socket # closing, not on how it closed, so a clean exit must be caught exactly like the @@ -118,6 +134,17 @@ no_engine_errors() { # no_engine_errors