diff --git a/CLAUDE.md b/CLAUDE.md index 339d59f..f60dc06 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -165,6 +165,10 @@ ticks in milliseconds with no SceneTree. speed, so `SimConfig.MAX_BULLET_SPEED` clamps the result — without it two Snipers put shots through walls. Pinned by `test_bullet_speeds_stay_below_the_tunnelling_threshold`. +- **A character becomes a live player only in `ServerRuntime._adopt_character`, + from `_place`.** `SimWorld` knows nothing about characters and builds a blank + player on every instance change, so any new path that puts someone in a world + must go through `_place` or it hands them a level-1 body with an empty bag. - **A player's combat numbers are derived, never stored.** `PlayerStats.build()` recomputes them from the character's upgrade ids every time, so a saved stat cannot disagree with the upgrades that produced it. Upgrade *riders* (split diff --git a/docs/DECISIONS.md b/docs/DECISIONS.md index 578c7f4..3a6dd23 100644 --- a/docs/DECISIONS.md +++ b/docs/DECISIONS.md @@ -297,6 +297,15 @@ lives in `SimConfig.UPGRADE_DAMAGE_BONUS` and is applied once per upgrade held, rather than being baked into seven definitions where it would be seven places to get wrong. +**A character record becomes a live player in exactly one place.** +`ServerRuntime._adopt_character`, called from `_place`, which every transfer +goes through. `SimWorld` knows nothing about characters and hands back a blank +player on every instance change, so something has to give it back its level, +upgrades and bag — and for a long time only character *select* did. Every +portal and every escape reset the player to level 1 with base stats and an +empty inventory, while the record on disk stayed correct, which made it look +like a display bug. + **Stats are derived from the upgrade list, never stored.** `PlayerStats.build()` recomputes them from the ids the character holds, the same way `Progression` derives level from experience. One source of truth means a saved stat can never diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index 80c69d8..275dc2d 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -25,7 +25,7 @@ What "everything passes" currently means. Numbers move; the shape does not. | `diag_prediction.tscn` | client-prediction gap, with injected clock drift | ~10s | | `diag_progression.tscn` | kill → xp → level → health, death → retire → roster, swap guards | ~10s | | `diag_loot.tscn` | drop → snapshot → pick up → persist → use → drop, and both loot visibilities on the wire | ~10s | -| `diag_upgrades.tscn` | level → banked choice → refused in a dungeon and away from the NPC → taken → new stats → persisted | ~10s | +| `diag_upgrades.tscn` | level → banked choice → refused in a dungeon and away from the NPC → taken → new stats → persisted → **survives every zone change** | ~10s | The four diagnostics exist because the smoke test structurally cannot reach what they cover: bots are poor shots (so they neither level up, produce drops, diff --git a/src/net/server_runtime.gd b/src/net/server_runtime.gd index e0ec1a2..47ceb1b 100644 --- a/src/net/server_runtime.gd +++ b/src/net/server_runtime.gd @@ -299,12 +299,10 @@ func _enter_world_as(peer_id: int, c: Character) -> void: peer_characters[peer_id] = c.id peer_names[peer_id] = c.display_name store.set_last_played(account, c.id) + # _place adopts, so the old three-line adopt/reset/adopt dance here is gone: + # it existed to work around reset_for_instance clobbering the health that + # adopt had just computed, and both now derive it the same way. _place(peer_id, lobby) - var p: SimPlayer = lobby.world.players.get(peer_id) - if p != null: - p.adopt(c) - p.reset_for_instance(lobby.world.spawn_point, 0) - p.adopt(c) GameLog.info("server", "peer %d playing '%s' (level %d)" % [peer_id, c.display_name, c.level]) _send_upgrades(peer_id) @@ -381,6 +379,13 @@ func instance_of(peer_id: int) -> Instance: func _place(peer_id: int, inst: Instance) -> void: inst.add_peer(peer_id, peer_names.get(peer_id, "player")) peer_instance[peer_id] = inst.id + # The world hands back a BLANK player -- level 1, base stats, empty bag -- + # because SimWorld knows nothing about characters. Giving it back its + # character is this layer's job, and for a long time it only happened on + # character select: every portal and every escape quietly reset the player + # to level 1 with no upgrades and nothing carried. The record on disk was + # always correct, which is what made it look like a display bug. + _adopt_character(peer_id, inst) # Size only: the seed stays server-side, or a client could rebuild the map. peer_chunks[peer_id] = {} Net.send_enter_instance(peer_id, inst.id, int(inst.kind), inst.world.tick, @@ -400,6 +405,26 @@ func _place(peer_id: int, inst: Instance) -> void: inst.exit_countdown_seconds(), peer_id)) +## Give the player in [param inst] the character its peer is playing. +## +## The one place a character record becomes a live player, so there is a single +## thing to get right rather than one per transition. +func _adopt_character(peer_id: int, inst: Instance) -> void: + var account: int = peer_accounts.get(peer_id, AuthProvider.NO_ACCOUNT) + var character_id: String = peer_characters.get(peer_id, "") + if account == AuthProvider.NO_ACCOUNT or character_id.is_empty(): + return # authenticated but still at the roster screen + var c := store.get_character(account, character_id) + var p: SimPlayer = inst.world.players.get(peer_id) + if c == null or p == null: + return + p.adopt(c) + # Arriving anywhere is a fresh start. adopt() only clamps health down to the + # new ceiling, which would leave a character whose upgrades RAISED it + # arriving wounded for no reason. + p.hp = p.max_hp + + func _transfer(peer_id: int, to: Instance) -> void: var from := instance_of(peer_id) if from != null: @@ -558,7 +583,9 @@ func _grant_xp(peer_id: int, amount: int) -> void: if p != null: var before := p.max_hp p.level = c.level - p.max_hp = c.max_hp() + # Derived rather than copied from the character, so there is one + # formula for maximum health and not two that can disagree. + p.recompute_max_hp() p.hp = mini(p.hp + (p.max_hp - before), p.max_hp) GameLog.info("server", "peer %d reached level %d (%d choice(s) pending)" % [peer_id, c.level, c.pending_choices]) diff --git a/src/ui/upgrade_screen.gd b/src/ui/upgrade_screen.gd index 64af3d3..a9a5de0 100644 --- a/src/ui/upgrade_screen.gd +++ b/src/ui/upgrade_screen.gd @@ -143,12 +143,11 @@ func _make_card(index: int, id: StringName) -> Control: body.add_theme_font_size_override("font_size", 13) card.add_child(body) - # Shown on every card, because it applies to every card. The brief asks for - # the choice screen to show this buff as well as the upgrade's own effects, - # and a player comparing three cards should not have to remember it. + # Shown on every card because it applies to every card. Deliberately NOT + # labelled "every upgrade": that a flat bonus rides along with all of them + # is a design principle, and the player only needs the number. var bonus := Label.new() - bonus.text = "+%d%% damage (every upgrade)" % roundi( - SimConfig.UPGRADE_DAMAGE_BONUS * 100.0) + bonus.text = "+%d%% damage" % roundi(SimConfig.UPGRADE_DAMAGE_BONUS * 100.0) bonus.add_theme_font_size_override("font_size", 12) bonus.add_theme_color_override("font_color", Color(0.55, 0.8, 0.6)) card.add_child(bonus) diff --git a/tools/diag_upgrades.gd b/tools/diag_upgrades.gd index 2ed5742..cb07971 100644 --- a/tools/diag_upgrades.gd +++ b/tools/diag_upgrades.gd @@ -21,6 +21,12 @@ var _account: int = 616161 var _character: Character var _offer_before: Array[StringName] = [] var _damage_before: int = 0 +## What the character was carrying and shooting with before it changed zones. +var _carried: Array[StringName] = [] +var _upgrades_before: Array[StringName] = [] +var _stats_before: PlayerStats = null +var _level_before: int = 0 +var _xp_before: int = 0 func _ready() -> void: @@ -65,7 +71,11 @@ func _physics_process(_delta: float) -> void: 70: _take_it_at_the_npc() 80: _it_changed_things() 90: _refused_with_nothing_pending() - 100: _finish() + 95: _walk_into_a_dungeon() + 105: _still_mine_in_the_dungeon() + 110: _walk_back_to_the_hub() + 120: _still_mine_in_the_hub() + 130: _finish() func _login() -> void: @@ -199,6 +209,66 @@ func _refused_with_nothing_pending() -> void: _check(_stored().offer.is_empty(), "and no table is left on the counter") +## Everything below is about ONE bug, and it is worth naming: the world hands +## back a blank player on every instance transfer, and for a long time only +## character *select* gave it back its character. Every portal and every escape +## silently reset the player to level 1 with base stats and an empty bag. The +## record on disk stayed correct the whole time, so it read as a display +## glitch, and nothing here or in the test suite looked. +func _walk_into_a_dungeon() -> void: + var inst := _srv.instance_of(Net.LOCAL_PEER) + var p := _me() + # Carry something, the way a player would after a pickup. + p.add_item(Items.HEALTH_POTION) + _srv._persist_inventory(inst, Net.LOCAL_PEER) + _carried = p.inventory.duplicate() + _upgrades_before = _stored().upgrades.duplicate() + _stats_before = p.stats + _level_before = p.level + _xp_before = p.total_xp + _check(_level_before > 1, "setup: the character has levelled") + _check(not _upgrades_before.is_empty(), "setup: and holds an upgrade") + _srv._send_to_dungeon(Net.LOCAL_PEER) + + +func _carried_through(where: String) -> void: + var p := _me() + if p == null: + _check(false, "there is a player in the %s at all" % where) + return + _check(p.level == _level_before, + "%s: level survives the trip (%d)" % [where, p.level]) + _check(p.total_xp == _xp_before, "%s: so does experience" % where) + _check(p.stats.upgrade_count == _upgrades_before.size(), + "%s: and the upgrades (%d)" % [where, p.stats.upgrade_count]) + _check(p.stats.damage == _stats_before.damage, + "%s: so the gun still hits for %d" % [where, p.stats.damage]) + # Compared against the CHARACTER's health, not against a formula fed the + # player's own level -- that agrees with itself even when the level is + # wrong, which is exactly the state this whole section exists to catch. + _check(p.max_hp == _stored().max_hp(), + "%s: and maximum health matches the record (%d)" % [where, p.max_hp]) + _check(p.inventory == _carried, "%s: the bag came too" % where) + + +func _still_mine_in_the_dungeon() -> void: + var inst := _srv.instance_of(Net.LOCAL_PEER) + _check(inst != null and inst.kind == Protocol.InstanceKind.DUNGEON, + "walked into a dungeon") + _carried_through("dungeon") + + +func _walk_back_to_the_hub() -> void: + _srv._send_to_lobby(Net.LOCAL_PEER) + + +func _still_mine_in_the_hub() -> void: + var inst := _srv.instance_of(Net.LOCAL_PEER) + _check(inst != null and inst.kind == Protocol.InstanceKind.LOBBY, + "escaped back to the hub") + _carried_through("hub") + + func _finish() -> void: print("---") if _fails.is_empty():