From 39d857c4d1d2e7f4d11aa2b9064da9ea60404f73 Mon Sep 17 00:00:00 2001 From: claude Date: Mon, 7 Sep 2026 17:09:43 +0200 Subject: [PATCH] Bound what one packet can cost, and stop the hub eating its own loot Selecting a character rewrote the entire store to disk, re-placed the player, re-streamed its map chunks and broadcast a roster to everyone -- off one small reliable packet, at whatever rate a client cared to send. Repeating a selection you already hold now answers yes and stops, and select/create are paced per peer. Item moves and experience grants, the two paths a client can trigger every tick, mark the store dirty and the server flushes once per tick instead of once per event. That flush also fixes experience quietly not being saved: grant_xp only wrote when a level was crossed, and level is DERIVED from lifetime xp, so the number nobody was keeping was the only number there is. A restart rolled a character back to its last level-up. The emergency escape ran in the hub, where completing it means transferring into the instance you are already in: teleported to spawn, healed to full, instanced loot dropped, every map chunk re-streamed, once a second for as long as the key is held. The hub now opts out. Ground loot evicts the oldest item when the floor fills. That is right for a dungeon, which closes and takes its litter with it, and wrong for the hub, which never closes -- a player dropping into a full floor destroyed whatever someone else had left there longest, and drop is edge-triggered per tick. A player's drop is refused instead; the world's own drops still make room. Also: a linkdead body is not in inst.peers, so closing a CLEARED dungeon walked past it and left that peer's account and name in the dictionaries forever -- with the new one-account guard, enough to lock someone out of their own session. And several encoders wrote mini(size, 255) as a count and then looped the whole array, which puts records after the count on the wire; MAX_CLIENTS makes it unreachable, which is why it would have survived to the day it was not. Co-Authored-By: Claude Opus 5 --- src/core/sim_config.gd | 15 ++++++-- src/instances/instance.gd | 3 ++ src/meta/character_store.gd | 42 ++++++++++++++++++-- src/net/net_codec.gd | 17 ++++++-- src/net/server_runtime.gd | 60 ++++++++++++++++++++++++++++- src/sim/sim_world.gd | 17 ++++++++ tests/unit/test_character_store.gd | 50 ++++++++++++++++++++++++ tests/unit/test_emergency_escape.gd | 30 +++++++++++++++ tests/unit/test_loot.gd | 32 +++++++++++++++ 9 files changed, 253 insertions(+), 13 deletions(-) diff --git a/src/core/sim_config.gd b/src/core/sim_config.gd index ea14382..4e1f15d 100644 --- a/src/core/sim_config.gd +++ b/src/core/sim_config.gd @@ -65,10 +65,17 @@ const SPAWN_GRACE_TICKS := 120 # 2 seconds ## it cannot outpace anything that is actually shooting at you. const HP_REGEN_PERCENT_PER_SEC := 0.5 -## A downed player cannot leave for the hub until this has elapsed. Enforced -## here rather than only by grey-ing out the button, because a disabled button -## is a suggestion -- the server is the only thing a modified client cannot -## argue with. +## A downed player cannot leave for the hub until this has elapsed. Enforced in +## the simulation rather than only by grey-ing out the button, because a +## disabled button is a suggestion. +## +## Worth knowing what actually reaches it: a player whose peer is still +## connected never waits, because ServerRuntime unbinds it on the same tick it +## dies and leaves it at the roster screen -- that is a settled decision, not an +## oversight (docs/DECISIONS.md, "Death unbinds the character entirely"). What +## the lockout governs is the LINKDEAD body, which has nobody to show a roster +## to and asks for itself; the countdown is what stops it vanishing out of a +## fight the instant it is killed. const RESPAWN_LOCKOUT_TICKS := 180 # 3 seconds # --- Input timing ----------------------------------------------------------- diff --git a/src/instances/instance.gd b/src/instances/instance.gd index 93559ff..3da08eb 100644 --- a/src/instances/instance.gd +++ b/src/instances/instance.gd @@ -47,6 +47,9 @@ static func make_lobby(instance_id: int) -> Instance: inst.world.upgrade_npc = built["npc"] inst.world.has_upgrade_npc = true inst.world.spawn_point = built["spawn"] + # Nothing here to escape from, and completing the channel in the hub means + # transferring to the hub -- see SimWorld.escape_enabled. + inst.world.escape_enabled = false inst.state = State.ACTIVE # A single inert practice target so players can feel out the gun before # committing to a run. diff --git a/src/meta/character_store.gd b/src/meta/character_store.gd index cb56164..e8857de 100644 --- a/src/meta/character_store.gd +++ b/src/meta/character_store.gd @@ -20,6 +20,9 @@ const MAX_ACTIVE := 5 var _accounts: Dictionary[int, Dictionary] = {} var _path: String = SAVE_PATH var _rng := RandomNumberGenerator.new() +## Set by [method save_deferred]; cleared by the next write. See that method for +## why one path defers and every other one does not. +var _dirty: bool = false func _init(path: String = SAVE_PATH) -> void: @@ -108,12 +111,25 @@ func retire_character(account_id: int, character_id: String) -> void: ## Write a character's carried items back to disk. Separate from grant_xp ## rather than folded into a general "save this character", so the one caller ## reads as what it is. -func set_inventory(account_id: int, character_id: String, items: Array[StringName]) -> void: +## +## [param defer] coalesces the write to the end of the server tick. Item events +## are the one persistence path a client can trigger at the tick rate -- the +## slot byte counts toward the edge, so alternating it with drop held produces +## an action every tick -- and each one otherwise re-serialised every account on +## the server to disk. Deferring bounds that at one write per tick however many +## players are doing it. The risk the immediate write guards against is a crash +## between moving an item and recording it; a tick is 16ms of that, and the +## direction of the loss is an item that stays in your bag, never a duplicate. +func set_inventory(account_id: int, character_id: String, items: Array[StringName], + defer: bool = false) -> void: var c := get_character(account_id, character_id) if c == null: return c.set_inventory(items) - save() + if defer: + save_deferred() + else: + save() ## Award level-up choices and, if nothing is on the table yet, roll one. @@ -162,14 +178,32 @@ func grant_xp(account_id: int, character_id: String, amount: int) -> int: if c == null: return 0 var gained := c.grant_xp(amount) - if gained > 0: - save() + # Every grant is written, not only the ones that cross a level boundary. + # Saving on gained > 0 alone meant a character could kill its way to one + # point short of a level and lose all of it to a restart, because the level + # is DERIVED from lifetime experience -- the number that was not being kept + # is the only number there is. Deferred so a busy fight still costs one + # write per tick rather than one per kill. + save_deferred() return gained # --- Persistence ------------------------------------------------------------ +## Mark the store as needing a write, without doing it now. The owner calls +## [method flush] once per tick; see [method set_inventory]. +func save_deferred() -> void: + _dirty = true + + +## Write only if something asked for one since the last write. +func flush() -> void: + if _dirty: + save() + + func save() -> void: + _dirty = false var accounts := {} for account_id in _accounts: var entry: Dictionary = _accounts[account_id] diff --git a/src/net/net_codec.gd b/src/net/net_codec.gd index bfb028b..a8fe524 100644 --- a/src/net/net_codec.gd +++ b/src/net/net_codec.gd @@ -38,7 +38,13 @@ static func encode_snapshot(world: SimWorld, or eye.distance_squared_to(p.pos) <= cull_sq: visible_players.append(p) - b.put_u8(mini(visible_players.size(), 255)) + # Sliced, not just clamped. Writing mini(size, 255) as the header and then + # looping the full array puts records after the count on the wire, and the + # decoder reads the enemy count out of the middle of a player. MAX_CLIENTS + # keeps it unreachable today, which is exactly why it would survive to the + # day it is not. + visible_players = visible_players.slice(0, 255) + b.put_u8(visible_players.size()) for p in visible_players: b.put_u32(p.peer_id) b.put_float(p.pos.x) @@ -81,7 +87,8 @@ static func encode_snapshot(world: SimWorld, if observer != null and eye.distance_squared_to(e.pos) > cull_sq: continue live_enemies.append(e) - b.put_u16(mini(live_enemies.size(), 65535)) + live_enemies = live_enemies.slice(0, 65535) + b.put_u16(live_enemies.size()) for e in live_enemies: b.put_u32(e.id) b.put_float(e.pos.x) @@ -123,7 +130,8 @@ static func encode_snapshot(world: SimWorld, if eye.distance_squared_to(l.pos) > cull_sq: continue visible_loot.append(l) - b.put_u16(mini(visible_loot.size(), 65535)) + visible_loot = visible_loot.slice(0, 65535) + b.put_u16(visible_loot.size()) for l in visible_loot: b.put_u32(l.id) b.put_float(l.pos.x) @@ -361,7 +369,8 @@ static func decode_inputs(data: PackedByteArray) -> Array[InputFrame]: static func encode_roster(entries: Array[Dictionary]) -> PackedByteArray: var b := StreamPeerBuffer.new() b.big_endian = false - b.put_u8(mini(entries.size(), 255)) + entries = entries.slice(0, 255) + b.put_u8(entries.size()) for e in entries: b.put_u32(int(e["peer"])) b.put_utf8_string(String(e["name"])) diff --git a/src/net/server_runtime.gd b/src/net/server_runtime.gd index aad129e..e0786be 100644 --- a/src/net/server_runtime.gd +++ b/src/net/server_runtime.gd @@ -24,8 +24,20 @@ var store: CharacterStore = null var peer_chunks: Dictionary[int, Dictionary] = {} var lobby: Instance +## Ticks a peer must wait between control messages. Selecting or creating a +## character rewrites the whole store to disk, re-places the player (which wipes +## its map-chunk record and re-streams the neighbourhood), and broadcasts a +## roster to every peer on the server -- all off one small reliable packet. +## Unrated, one client could hold the server in a synchronous JSON write. A +## quarter of a second is far under anything a person clicks deliberately. +const CONTROL_COOLDOWN_TICKS := 15 + var _next_instance_id: int = SimConfig.LOBBY_INSTANCE_ID var _snapshot_phase: int = 0 +## Server clock, in physics ticks. Only used to pace the control messages above. +var _tick: int = 0 +## Tick each peer was last allowed a control message on. +var _peer_control_tick: Dictionary[int, int] = {} func _ready() -> void: @@ -49,6 +61,7 @@ func _take_instance_id() -> int: func _physics_process(_delta: float) -> void: + _tick += 1 _snapshot_phase += 1 var send_snapshot := _snapshot_phase % SimConfig.SNAPSHOT_INTERVAL == 0 var closing: Array[int] = [] @@ -79,6 +92,12 @@ func _physics_process(_delta: float) -> void: for id in closing: _close_dungeon(id) + # One write per tick at most, however many item pickups and experience + # grants the tick produced. Everything that defers is bounded here; the + # paths that matter more (a character created, retired, or levelled) still + # write the moment they happen. + store.flush() + ## Split the tick's events into the ones clients need and the ones only the ## server acts on (escape completion, portal use, respawn requests). @@ -185,6 +204,7 @@ func _forget_peer(peer_id: int) -> void: peer_chunks.erase(peer_id) peer_accounts.erase(peer_id) peer_characters.erase(peer_id) + _peer_control_tick.erase(peer_id) _broadcast_roster() @@ -270,6 +290,16 @@ func on_select_character(peer_id: int, character_id: String) -> void: Net.send_select_result(peer_id, Protocol.SelectResult.NOT_IN_HUB, "you can only change character in the hub") return + # Already playing this one, and already where a swap would put you. Answering + # yes and stopping is not just an optimisation: the full path rewrites the + # store, re-places the player and re-streams its map, so a client that simply + # repeats its own selection was free work for the asking. + if peer_characters.get(peer_id, "") == character_id and here != null: + Net.send_select_result(peer_id, Protocol.SelectResult.OK, "") + _send_characters(peer_id) + return + if not _accept_control(peer_id): + return var c := store.get_character(account, character_id) if c == null: Net.send_select_result(peer_id, Protocol.SelectResult.NO_SUCH_CHARACTER, "no such character") @@ -293,6 +323,8 @@ func on_create_character(peer_id: int, character_name: String) -> void: Net.send_select_result(peer_id, Protocol.SelectResult.NOT_IN_HUB, "you can only change character in the hub") return + if not _accept_control(peer_id): + return var c := store.create_character(account, character_name) if c == null: Net.send_select_result(peer_id, Protocol.SelectResult.LIMIT_REACHED, @@ -389,6 +421,18 @@ func instance_of(peer_id: int) -> Instance: return instances.get(id) +## Whether [param peer_id] may spend a control message this tick, recording it +## if so. Silent when it refuses: there is no SelectResult for "too fast", and a +## dropped message at this rate can only come from a client that is not a menu. +func _accept_control(peer_id: int) -> bool: + var last: int = _peer_control_tick.get(peer_id, -CONTROL_COOLDOWN_TICKS) + if _tick - last < CONTROL_COOLDOWN_TICKS: + GameLog.warn("server", "peer %d control message dropped, too fast" % peer_id) + return false + _peer_control_tick[peer_id] = _tick + return true + + ## 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 @@ -521,6 +565,17 @@ func _close_dungeon(id: int) -> void: return for peer in inst.peers.duplicate(): _transfer(peer, lobby) + # A linkdead body is not in inst.peers, so the loop above walks straight past + # it. The empty-instance path checks has_linkdead() and waits; the CLEARED + # path does not, so a party that killed the boss with one member disconnected + # left that peer's account, name and instance id in the dictionaries with no + # instance to resolve -- invisible, permanent, and enough to keep the account + # locked out of its own session by the one-account guard. + for p in inst.world.players.values(): + if p.linkdead: + GameLog.info("server", "peer %d released with instance %d as it closed" + % [p.peer_id, id]) + _forget_peer(p.peer_id) instances.erase(id) GameLog.info("server", "closed dungeon instance %d" % id) @@ -579,7 +634,10 @@ func _persist_inventory(inst: Instance, peer_id: int) -> void: var p: SimPlayer = inst.world.players.get(peer_id) if p == null: return - store.set_inventory(account, character_id, p.inventory) + # Deferred: a client can produce an item action every tick, and each one used + # to re-serialise every account on the server. ServerRuntime flushes once at + # the end of the tick. + store.set_inventory(account, character_id, p.inventory, true) func _grant_xp(peer_id: int, amount: int) -> void: diff --git a/src/sim/sim_world.gd b/src/sim/sim_world.gd index 0334005..3007177 100644 --- a/src/sim/sim_world.gd +++ b/src/sim/sim_world.gd @@ -46,6 +46,14 @@ var spawn_point := Vector2(0.0, 240.0) ## Arrival protection granted to players entering this world. 0 in the hub, ## SimConfig.SPAWN_GRACE_TICKS in a dungeon. var spawn_grace_ticks: int = 0 +## Whether the emergency escape does anything here. False only in the hub: the +## escape's completion is routed to "send this peer to the lobby", and in the +## lobby that is a transfer to the instance you are already in -- which re-places +## the player, teleporting it to the spawn point, healing it to full, dropping +## its instanced loot and re-streaming every map chunk around it, once a second +## for as long as the key is held. A world you can leave is the default; the hub +## is the one that opts out. +var escape_enabled: bool = true var _next_actor_id: int = 1 var _ctx := EmitContext.new() @@ -471,6 +479,13 @@ func _drop_slot(p: SimPlayer, slot_index: int) -> void: return if p.inventory[slot_index] == Items.NONE: return + # Refused rather than made room for. spawn_loot evicts the oldest item when + # the floor is full, which is right for a dungeon's own drops and wrong for a + # player's: the hub never closes, so a player dropping into a full floor was + # destroying whatever someone else had left there longest -- and the drop key + # can be pressed every tick. Your item stays in your bag instead. + if loot.size() >= SimConfig.MAX_LOOT_PER_INSTANCE: + return var item := p.take_slot(slot_index) # Anything dropped becomes world-shared, even if it arrived as an instanced # drop. That is what makes dropping worth having: an item you do not want @@ -480,6 +495,8 @@ func _drop_slot(p: SimPlayer, slot_index: int) -> void: func _step_escape(p: SimPlayer, frame: InputFrame) -> void: + if not escape_enabled: + return # A dropped connection is treated as holding the button down. Pulling the # plug then costs exactly what pressing escape costs -- one second of # standing there, still killable -- instead of an instant, safe exit. diff --git a/tests/unit/test_character_store.gd b/tests/unit/test_character_store.gd index 7fa8477..8f91278 100644 --- a/tests/unit/test_character_store.gd +++ b/tests/unit/test_character_store.gd @@ -312,3 +312,53 @@ func test_an_unknown_upgrade_in_a_save_is_dropped() -> void: assert_eq(restored.upgrades, [Upgrades.SNIPER] as Array[StringName]) assert_eq(restored.offer, [Upgrades.POISON] as Array[StringName]) assert_eq(restored.pending_choices, 2) + + +# --- Deferred writes -------------------------------------------------------- +# Two paths a client can trigger at the tick rate -- item moves and experience +# grants -- mark the store dirty instead of writing, and the server flushes once +# per tick. Everything else still writes the moment it happens. + +func test_experience_below_a_level_boundary_is_still_written() -> void: + var c := store.create_character(ACC, "Grinder") + assert_eq(store.grant_xp(ACC, c.id, 5), 0, "nowhere near a level") + store.flush() + + var reloaded := CharacterStore.new(store._path) + assert_true(reloaded.load_from_disk()) + assert_eq(reloaded.get_character(ACC, c.id).total_xp, 5, + "experience that has not crossed a level is the only record of it: " + + "level is derived from lifetime xp, so losing the xp loses the level") + + +func test_a_deferred_inventory_write_lands_on_the_flush() -> void: + var c := store.create_character(ACC, "Carrier") # writes immediately + var bag: Array[StringName] = [Items.HEALTH_POTION, Items.NONE, Items.NONE, Items.NONE] + store.set_inventory(ACC, c.id, bag, true) + + var before := CharacterStore.new(store._path) + before.load_from_disk() + assert_eq(before.get_character(ACC, c.id).inventory[0], Items.NONE, + "deferred means deferred; the file still holds the older bag") + + store.flush() + var after := CharacterStore.new(store._path) + after.load_from_disk() + assert_eq(after.get_character(ACC, c.id).inventory[0], Items.HEALTH_POTION) + + +func test_flushing_a_clean_store_writes_nothing() -> void: + store.create_character(ACC, "Idle") + var stamp := FileAccess.get_modified_time(store._path) + store.flush() + assert_eq(FileAccess.get_modified_time(store._path), stamp, + "a tick in which nothing happened must not cost a disk write") + + +func test_an_immediate_write_clears_the_pending_one() -> void: + var c := store.create_character(ACC, "Mixed") + store.grant_xp(ACC, c.id, 7) # dirty, not written + store.set_last_played(ACC, c.id) # writes now, and takes the xp with it + var reloaded := CharacterStore.new(store._path) + reloaded.load_from_disk() + assert_eq(reloaded.get_character(ACC, c.id).total_xp, 7) diff --git a/tests/unit/test_emergency_escape.gd b/tests/unit/test_emergency_escape.gd index 535852e..8f921e6 100644 --- a/tests/unit/test_emergency_escape.gd +++ b/tests/unit/test_emergency_escape.gd @@ -112,3 +112,33 @@ func test_escape_completion_never_reaches_the_client_directly() -> void: for ev: Dictionary in packet["events"]: assert_ne(int(ev["t"]), SimEvent.Type.ESCAPE_COMPLETED, "the transfer is the server's decision; clients only see the outcome") + + +## The hub has nothing to escape from, and completing the channel there routes +## to "send this peer to the lobby" -- a transfer into the instance you are +## already in, which re-places the player: teleported to spawn, healed to full, +## instanced loot dropped, every map chunk re-streamed. Once a second, for as +## long as the key is held. +func test_the_hub_has_no_escape_to_channel() -> void: + var hub := Instance.make_lobby(1) + assert_false(hub.world.escape_enabled, + "the lobby is the one world that opts out") + hub.add_peer(99, "tester") + for i in SimConfig.ESCAPE_CHANNEL_TICKS * 2: + var frames: Array[InputFrame] = [InputFrame.make(hub.world.tick + 1, + Vector2.ZERO, 0.0, InputFrame.BTN_ESCAPE)] + hub.world.queue_input(99, frames) + hub.step() + var p: SimPlayer = hub.world.players[99] + assert_eq(p.escape_ticks, 0, "nothing should have been channelling") + for ev in hub.world.events: + assert_ne(int(ev["t"]), SimEvent.Type.ESCAPE_STARTED, + "holding escape in the hub must not start a channel") + assert_ne(int(ev["t"]), SimEvent.Type.ESCAPE_COMPLETED, + "and must certainly never complete one") + + +func test_a_dungeon_still_lets_you_out() -> void: + # The guard above must not be a way to disable the escape everywhere. + var run := Instance.make_dungeon(2, 1234) + assert_true(run.world.escape_enabled) diff --git a/tests/unit/test_loot.gd b/tests/unit/test_loot.gd index b1e252b..8e17e5e 100644 --- a/tests/unit/test_loot.gd +++ b/tests/unit/test_loot.gd @@ -418,3 +418,35 @@ func test_the_same_tick_tie_break_is_join_order_not_distance() -> void: assert_eq(world.players[ME].inventory[0], Items.HEALTH_POTION, "the player who joined first wins, despite standing further away") assert_eq(them.inventory[0], Items.NONE) + + +## Ground loot evicts the oldest item when the floor fills up, which is the +## right answer for a dungeon's own drops and the wrong one for a player's: the +## hub never closes, so dropping into a full floor destroyed whatever someone +## else had left there longest -- and drop is edge-triggered per tick, so a +## client could clear the hub floor in seconds. +func test_a_player_drop_never_destroys_someone_elses_item() -> void: + var w := SimWorld.new(1) + var mine := w.add_player(1, "me") + mine.pos = Vector2.ZERO + mine.inventory[0] = Items.HEALTH_POTION + + var theirs: Array[int] = [] + for i in SimConfig.MAX_LOOT_PER_INSTANCE: + theirs.append(w.spawn_loot(Items.WARDENS_RATION, Vector2(500.0, float(i))).id) + assert_eq(w.loot.size(), SimConfig.MAX_LOOT_PER_INSTANCE, "the floor is full") + + w._drop_slot(mine, 0) + assert_eq(mine.inventory[0], Items.HEALTH_POTION, + "the drop is refused, so the item stays in the bag") + for id in theirs: + assert_true(w.loot.has(id), "nothing already on the floor may be evicted") + + +func test_the_cap_still_bites_for_drops_the_world_generates_itself() -> void: + # A dungeon's own drops keep the old behaviour: they must never fail to + # appear, and the instance closes and takes the litter with it anyway. + var w := SimWorld.new(1) + for i in SimConfig.MAX_LOOT_PER_INSTANCE + 5: + w.spawn_loot(Items.WARDENS_RATION, Vector2(float(i), 0.0)) + assert_eq(w.loot.size(), SimConfig.MAX_LOOT_PER_INSTANCE)