Stage 4: upgrades, and a quartermaster to spend them at

Every level banks one choice. Choices queue, and are spent at an NPC in the
hub: walk to it, press E, take one of three weighted options. Seven upgrades,
all data — split shot, glass cannon, spread, sniper, doubleshot, poison,
eraser — and SimWorld gained no per-upgrade branch to run any of them.

The four ambiguities in the brief were settled with the user first, since
each changes what gets written:

  damage      base x (1 + sum additive) x product multiplicative. The flat
              +5% every upgrade carries, spread's -10%, doubleshot's -50%
              and glass cannon's +100% pool; sniper multiplies on top, so
              two snipers is 4x and not +200%.
  glass       half the LEVELLED maximum, multiplying if taken twice, so the
              price does not fade to a rounding error by level 15.
  poison      independent stacks, not a refresh.
  split       +/-45 degrees from the original heading.

Independent poison stacks sound expensive and are not: every dose lasts the
same number of ticks, so doses expire in the order they were added, the
pending expiries are a plain FIFO, and PoisonTrack only ever looks at its
front. O(1) per actor per tick however many are live.

Stats are derived from the upgrade list and never stored, the way level is
derived from experience -- a saved stat cannot disagree with the upgrades
that produced it. Upgrade riders (split charges, poison, erase chance) travel
on the bullet instead, because a shot in flight has to keep what it was fired
with rather than gaining Poison because the shooter just took it.

Two invariants this collided with, both now pinned:

  - bullet speed gained a ceiling. Wall collision samples once per tick, so
    anything over a tile per tick tunnels; two snipers asked for 2480 u/s
    against a 1920 threshold, and a tunnelling bullet looks like a bullet.
  - BULLET_INTEREST_RADIUS rose to 2900, because an upgraded player shot is
    now the longest-travelling bullet in the game. test_interest measured
    the worst case from static content, which upgrades quietly invalidated.

Choosing is intent checked three ways: a choice must be owed, the index must
name one of the three options the SERVER put on the table, and the player
must be standing at the NPC. The offer is rolled once and persisted, so
closing the screen is not a reroll and neither is a crash.

tools/diag_upgrades.tscn covers level -> banked choice -> refused in a
dungeon and refused across the room -> taken at the NPC -> new stats ->
on disk. Bots never walk to the quartermaster, so the smoke test cannot.

Known gap recorded in the roadmap: at PLAYER_BULLET_DAMAGE = 6, the +5% the
first upgrade carries rounds back to 6 and visibly does nothing. It comes out
right in aggregate, but the fix is a balance edit across content.gd and so is
the user's call.

check.sh clean, 357 tests, SMOKE PASS (18 assertions), all four diagnostics
green.

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

tools/diag_upgrades.gd

@@ -0,0 +1,209 @@
+extends Node
+## End-to-end check of the upgrade loop: level -> banked choice -> the three
+## on the table -> spent at the NPC -> new stats -> persisted.
+##
+##   godot --headless --path . res://tools/diag_upgrades.tscn
+##
+## Runs as a scene because ServerRuntime needs the Net autoload. Exits non-zero
+## on any failure, so it gates like a test.
+##
+## The unit tests cover the formula and the store in isolation. What only shows
+## up here is the wiring between them: that a level earned in a dungeon reaches
+## the hub as a choice, that the server refuses one made from the wrong place,
+## and that taking it changes the numbers the player is actually shooting with.
+
+const STORE_PATH := "user://diag_upgrades.json"
+
+var _fails: Array[String] = []
+var _step: int = 0
+var _srv: ServerRuntime
+var _account: int = 616161
+var _character: Character
+var _offer_before: Array[StringName] = []
+var _damage_before: int = 0
+
+
+func _ready() -> void:
+	DirAccess.remove_absolute(ProjectSettings.globalize_path(STORE_PATH))
+	GameOpts.bot_client = true
+	GameOpts.account_override = _account
+	if Net.host(27403) != OK:
+		push_error("could not host")
+		get_tree().quit(1)
+		return
+	_srv = Net.server
+	_srv.store = CharacterStore.new(STORE_PATH)
+	_srv.store.load_from_disk()
+	Net.start_local_client()
+
+
+func _check(ok: bool, what: String) -> void:
+	if ok:
+		print("  ok   %s" % what)
+	else:
+		print("  FAIL %s" % what)
+		_fails.append(what)
+
+
+func _me() -> SimPlayer:
+	var inst := _srv.instance_of(Net.LOCAL_PEER)
+	return inst.world.players.get(Net.LOCAL_PEER) if inst != null else null
+
+
+func _stored() -> Character:
+	return _srv.store.get_character(_account, _character.id)
+
+
+func _physics_process(_delta: float) -> void:
+	_step += 1
+	match _step:
+		20: _login()
+		30: _earn_a_level()
+		40: _refused_in_a_dungeon()
+		50: _back_to_the_hub()
+		60: _refused_away_from_the_npc()
+		70: _take_it_at_the_npc()
+		80: _it_changed_things()
+		90: _refused_with_nothing_pending()
+		100: _finish()
+
+
+func _login() -> void:
+	_character = _srv.store.last_played(_account)
+	_check(_character != null, "a character exists after login")
+	if _character == null:
+		_finish()
+		return
+	_check(_character.upgrades.is_empty(), "and starts with no upgrades")
+	_check(_character.pending_choices == 0, "and nothing to spend")
+	_check(Net.client.upgrades_pending == 0, "the client agrees")
+	_damage_before = _me().stats.damage
+	_check(_damage_before == SimConfig.PLAYER_BULLET_DAMAGE,
+		"shooting for the base damage")
+
+
+## Levelling happens in a dungeon; the NPC is in the hub. The choice has to
+## survive the journey or it would be lost the moment it was earned.
+func _earn_a_level() -> void:
+	_srv._send_to_dungeon(Net.LOCAL_PEER)
+	_srv._grant_xp(Net.LOCAL_PEER, Progression.total_xp_for_level(3))
+	var c := _stored()
+	_check(c.level >= 2, "experience produced a level (now %d)" % c.level)
+	_check(c.pending_choices == c.level - 1,
+		"one choice per level gained (%d)" % c.pending_choices)
+	_check(c.offer.size() == SimConfig.UPGRADE_CHOICES,
+		"and %d options on the table" % SimConfig.UPGRADE_CHOICES)
+	_check(Net.client.upgrades_pending == c.pending_choices,
+		"the client was told without asking")
+	_check(Net.client.upgrade_offer.size() == SimConfig.UPGRADE_CHOICES,
+		"and was given the same table")
+	_offer_before = c.offer.duplicate()
+
+
+func _refused_in_a_dungeon() -> void:
+	Net.choose_upgrade(0)
+	_check(_stored().upgrades.is_empty(),
+		"an upgrade cannot be taken from inside a dungeon")
+	_check(_stored().offer == _offer_before,
+		"and the refusal does not reroll the table")
+
+
+func _back_to_the_hub() -> void:
+	_srv._send_to_lobby(Net.LOCAL_PEER)
+	# Stop the bot steering itself around the hub -- see _stand_at_the_npc().
+	Net.client.set_physics_process(false)
+
+
+func _refused_away_from_the_npc() -> void:
+	var inst := _srv.instance_of(Net.LOCAL_PEER)
+	_check(inst != null and inst.kind == Protocol.InstanceKind.LOBBY,
+		"back in the hub")
+	var p := _me()
+	# Far from the quartermaster. Where a player is standing is the one thing a
+	# modified client cannot fake, which is why the check lives here and not in
+	# the UI that greys out the button.
+	p.pos = inst.world.upgrade_npc + Vector2(SimConfig.UPGRADE_NPC_RADIUS * 4.0, 0.0)
+	Net.choose_upgrade(0)
+	_check(_stored().upgrades.is_empty(),
+		"standing across the room is not standing at the NPC")
+	_check(_stored().pending_choices > 0, "and the choice is still owed")
+
+
+## Standing at the NPC is set deliberately from here on. Whether the player
+## happens to be near it is exactly what is under test, and a bot steering
+## itself around the hub would decide that at random.
+func _stand_at_the_npc() -> void:
+	var inst := _srv.instance_of(Net.LOCAL_PEER)
+	if inst != null and _me() != null:
+		_me().pos = inst.world.upgrade_npc
+
+
+func _take_it_at_the_npc() -> void:
+	var inst := _srv.instance_of(Net.LOCAL_PEER)
+	var owed_before := _stored().pending_choices
+	_stand_at_the_npc()
+	_check(inst.world.at_upgrade_npc(_me().pos), "setup: standing at the NPC")
+	Net.choose_upgrade(1)
+	var c := _stored()
+	_check(c.upgrades.size() == 1, "the upgrade is taken")
+	_check(c.upgrades[0] == _offer_before[1],
+		"and it is the one the index named, from the server's own table")
+	_check(c.pending_choices == owed_before - 1,
+		"exactly one choice was spent (%d -> %d)" % [owed_before, c.pending_choices])
+	_check(not c.offer.is_empty() or c.pending_choices == 0,
+		"a table is on offer exactly while something is pending")
+	_check(c.offer != _offer_before, "and the next table is a fresh one")
+
+
+func _it_changed_things() -> void:
+	var p := _me()
+	var c := _stored()
+	_check(p.stats.upgrade_count == c.upgrades.size(),
+		"the player in the world is shooting with the new list")
+	var expected := PlayerStats.build(c.upgrades)
+	_check(p.stats.damage == expected.damage,
+		"damage matches the formula (%d -> %d)" % [_damage_before, p.stats.damage])
+	_check(p.max_hp == maxi(1, roundi(float(Progression.max_hp_for_level(p.level))
+		* expected.max_hp_mult)), "and maximum health follows the multiplier")
+	_check(Net.client.upgrades_taken.size() == c.upgrades.size(),
+		"the client's copy was refreshed")
+
+	# Persisted, not merely held in memory: the character store is what makes an
+	# upgrade survive a restart, and it is written the moment one is taken.
+	var reloaded := CharacterStore.new(STORE_PATH)
+	reloaded.load_from_disk()
+	var saved := reloaded.get_character(_account, _character.id)
+	_check(saved != null and saved.upgrades == c.upgrades,
+		"the upgrade is on disk already")
+	_check(saved != null and saved.offer == c.offer,
+		"and so is the table, so a crash is not a reroll")
+
+
+func _refused_with_nothing_pending() -> void:
+	var c := _stored()
+	# Bounded rather than a plain "while owed": a choice that were ever refused
+	# here would otherwise spin forever instead of reporting a failure, which is
+	# precisely what it did the first time this was written.
+	var guard := Progression.MAX_LEVEL + 2
+	while c.pending_choices > 0 and guard > 0:
+		guard -= 1
+		_stand_at_the_npc()
+		Net.choose_upgrade(0)
+		c = _stored()
+	_check(c.pending_choices == 0, "every owed choice could actually be spent")
+	var held := c.upgrades.size()
+	_stand_at_the_npc()
+	Net.choose_upgrade(0)
+	_check(_stored().upgrades.size() == held,
+		"with nothing owed, asking again gets nothing")
+	_check(_stored().offer.is_empty(), "and no table is left on the counter")
+
+
+func _finish() -> void:
+	print("---")
+	if _fails.is_empty():
+		print("UPGRADES_OK")
+	else:
+		print("UPGRADES_FAIL (%d)" % _fails.size())
+	Net.shutdown()
+	get_tree().quit(0 if _fails.is_empty() else 1)

tools/diag_upgrades.gd.uid

@@ -0,0 +1 @@
+uid://70ijixd36nt5

tools/diag_upgrades.tscn

@@ -0,0 +1,6 @@
+[gd_scene load_steps=2 format=3]
+
+[ext_resource type="Script" path="res://tools/diag_upgrades.gd" id="1"]
+
+[node name="DiagUpgrades" type="Node"]
+script = ExtResource("1")

tools/smoke.sh

@@ -134,6 +134,11 @@ check  "the dropped body is released"  "$OUT/server.log" "released from instance
 check  "a polite disconnect leaves too"  "$OUT/leavebot.log" "BOT_GRACEFUL_LEAVE"
 check  "a polite disconnect is also channelled" \
                                      "$OUT/server.log" "'leavebot' dropped in instance [0-9]+, channelling out"
+# No assertion for upgrades here on purpose. Bots never walk to the
+# quartermaster, so anything written would either duplicate a check above or
+# assert nothing at all. The upgrade-state message DOES cross the wire on every
+# login, so a malformed one shows up in the script-error refutations below --
+# and tools/diag_upgrades.tscn covers the loop itself.
 refute "no server script errors"     "$OUT/server.log" "SCRIPT ERROR|Parse Error|USER ERROR"
 refute "no client script errors"     "$OUT/bot1.log"   "SCRIPT ERROR|Parse Error|USER ERROR"