Quake 2 follow-up for #586-#589

[corepunch]

Follow-up to the four PRs landed today (#589, #588, #587, #586), applying Quake 2 review findings in one commit.

Net (#589): fix the stale loopback disconnect at the session boundary

• Root cause: a dead server never reads its queue. main.c only calls SV_Frame while SV_IsActive(), the same as Q2's if (!svs.initialized) return. The client's farewell disconnect therefore survives SV_Shutdown. Q2 is protected because the next client's getchallenge/connect handshake rides the same FIFO, so the stale packet is consumed while every slot is cs_free. Our SV_ClientConnect admits the local client straight into slot 0, so the stale packet hit a live slot.
• Fix: SV_InitGame discards the dead session's client→server loopback datagrams through the new NET_ClearLoopPackets.
• Revert of Prevent mission exit from disconnecting the next local session #589's approach: the client goes back to Q2 CL_Disconnect semantics and always tells its server. This removes the send_disconnect flag and the NA_LOOPBACK special case at the mission-exit call site, so every shutdown path is covered, not just one.
• Cleanup: removed a duplicated ss_dead branch in SV_ReadPackets.

Movement (#588)

• Unreachable-goal ownership is decided once. Point orders own a private waypoint, like Q2's movetarget; entity goals are never moved, as in M_MoveToGoal. Behaviour is equivalent, with one edict scan instead of up to three.
• The gate test now starts the attacker off the gate's row, so it proves the attacker actually steers to the approach point.

Powerups (#587)

• One table-driven pickup decision: a new AB_POWERUP ability flag, like Q2's gitem_t.pickup, replaces two duplicated if-chains.
• Bug: a powerup that affected nobody (for example, a speed rune picked up by a mechanical unit) went into an inventory slot. It is now always consumed.
• Bug: the speed cap follows the ability procedure rather than three rawcodes, so custom runes based on AIsp are capped.
• The speed macro is replaced by S_SpellApplyTimedTargetStatus.

Ancients and bots (#586)

• Bug: uprooted Ancients retaliated with the rooted weapon mask. An air attacker started a melee counterattack that cancelled on the next think.
• One S_AncientIsMorphing helper and one mask-taking attack predicate are now shared by orders and retaliation (cf. Q2 M_CheckAttack). Retaliation now also respects G_BuildingIsUnsummoning.
• Bots iterate shop stock directly instead of re-parsing a static buffer that G_IsItemShop rewrites mid-parse. Behaviour change: bots can buy items added with AddItemToStock, as players can.

Protocol / save impact

• Network wire format: unchanged.
• Save format: unchanged. No edict_t fields or callbacks added.

Tests

Each regression test was confirmed to fail before its fix:

• server_net.stale_loopback_disconnect_does_not_drop_next_local_session: exercises the real SV_Map → stale disconnect → SV_Shutdown → SV_Map → SV_Frame path.
• net.loopback_clear_discards_only_the_named_receiver_queue
• client_session.disconnect_always_tells_the_local_server_it_left
• In t_items: speed rune with no recipient is consumed; custom speed rawcode is capped; full inventory accepts only powerups.
• wc3_ancient_root.uprooted_protector_does_not_counterattack_with_rooted_weapon
• wc3_bot.hero_item_policy_buys_trigger_added_stock
• Strengthened attacking_unreachable_gate_keeps_gate_at_authored_position.

make test: 46,819/46,819 assertions in 2,345 tests.

Possible split, in dependency order, if you'd rather review separately: (1) net, (2) movement, (3) powerups, (4) Ancients/bots. The four parts are independent.

🤖 Generated with Claude Code