Fix: every zone change was handing the player a blank character

SimWorld knows nothing about characters, so Instance.add_peer builds a fresh
SimPlayer -- level 1, base stats, empty inventory. Something has to give that
player back its character, and only character SELECT ever did. Every portal
into a dungeon and every escape back to the hub therefore reset the player's
level, experience, upgrades and bag. The record on disk stayed correct
throughout, which is what made it read as a display glitch: the level shown was
1 because the level being played really was 1, and the first kill's experience
grant partially repaired it, so the numbers appeared to come and go.

_place now adopts, through a single _adopt_character that every transfer runs.
That let _enter_world_as drop its adopt/reset/adopt dance -- three lines that
existed only because reset_for_instance clobbered the health adopt had just
computed -- and let the level-up path derive maximum health through
recompute_max_hp instead of keeping a second copy of the formula.

diag_upgrades now walks hub -> dungeon -> hub after taking an upgrade and
asserts level, experience, upgrades, damage, maximum health and inventory all
survive each leg. With the fix reverted it reports exactly what was described:
level 1, no upgrades, base damage, empty bag. One of the new checks compared
health against a formula fed the player's own level, which agrees with itself
even when the level is wrong; it compares against the character record instead.

Also drops "(every upgrade)" from the choice cards. That a flat bonus rides
along with all of them is a design principle, not something a player needs
told -- the number is enough.

check.sh clean, 409 tests, SMOKE PASS, all four diagnostics green.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

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

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

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,

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])

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)

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():