Gameplay variable audit: the tables were right, nothing was reading them

CHANGELOG.md

@@ -32,6 +32,11 @@ change; see [`docs/RELEASING.md`](docs/RELEASING.md).
 - _Nothing yet._
 
 ### Fixes
+- Armour did nothing but the weakest thing it could. The projection that hands the player to the damage function wrote a literal 0 into the armour class, and 0 is jacket — so combat and body armour both absorbed 0.30 of an ordinary hit instead of 0.60 and 0.80, and neither absorbed anything at all from an energy weapon, where jacket's column is genuinely zero. Because that projection is rebuilt on *every* damage attempt there was no window in which the field could hold anything else, and no save or pickup could get a real class past it. The power shield was worse off still: the two bits the damage path tests live in the inventory's flag word, which the projection never copied, so `q2_combat_power_armour_absorb` returned at its first guard, spent no cells and saved nothing. A player wearing body armour now takes 19 of a 100-point hit rather than 69.
+- Quad damage multiplied nothing. Every fire function on the disc picks one of two immediates by comparing the level clock against the player's own expiry word, and the sim called them with that comparison hardcoded to false. The powerup was picked up, drawn on the HUD, counted down and played its firing sound for thirty seconds while the shotgun kept doing 6 a pellet.
+- The difficulty never reached the damage function. `skill` sat at the default 1 for the whole run because only the clock was refreshed in the combat rules, so easy was exactly as dangerous as medium: the rule that halves what a monster does to you at skill 0 could not fire. It now comes from the same global the creature AI already reads, so the two cannot disagree.
+- Nothing ever told the sim it was in a deathmatch. `q2_sim.multiplayer` had one writer — the save loader — and a match started from the menu left it false, so every rule that hangs off it ran in its single-player form inside a deathmatch: the railgun did 100 instead of 150, armour used the rounding bias that favours the wearer, items gave single-player amounts and did not respawn, and the rocket-jump ceiling was applied where the console does not apply it.
+- The player carried two different gib thresholds. The damage path used -100 and the death chain used -40, which is the one the disc writes at `0x800397FC`; there is now one copy. The corpse health floor was likewise spelled out twice under two names, so the two clamps could drift apart while both looked cited.
 - Warnings-as-errors had never passed on any compiler, so CI had been red since 20 August and nothing it said could be trusted. Every one is fixed rather than switched off: 33 on GCC and Clang, and another 20 behind them on MSVC, which stops at the first and so had never reported the rest. Six were real — two undefined behaviour (`gte_sxy` read through a `psx_xy` in the glint and bolt draws), a computation left dead by an earlier fix, an always-true bound on a `u8`, a POSIX function reached through a platform `#ifdef` that bought nothing, and a nested struct zeroed with too few braces. Thirteen were formats that really could truncate a path or a menu label and now say what they cut to. Two MSVC warnings are turned off, both with a reason: C4100 is the unreferenced parameter this project already ignores on GCC and Clang, and C4127 fires on every `CHECK(SOME_TRANSCRIBED_CONSTANT == 36, ...)` in the test suites, which is what those suites are for.
 - A solid collision node did not hold nothing. `q2_coll_point_in_node` carried its own copy of the solid-bit mask instead of asking `q2_collision_node_is_solid`, and on one MSVC build the copy did not fire while the accessor did — so a node marked impassable let a point sit inside it. There is now one place that decides what solid means. Found only because MSVC had never been able to build the project in CI, so its tests had never run there.
 

src/build/weapontables.c

@@ -48,9 +48,12 @@ static const q2_weapon_tables k_builtin = {
     .autoswitch = { 10, 9, 8, 5, 4, 3, 2, 1, 0, 0, 0, 0 },
     .autoswitch_count = 8,   /* excludes the terminating zero */
 
-    /* muzzle — 0x800AE9C4 onward, stored (right, up, forward) with the middle
-     * component already negated as every fire function negates it. Weapons
-     * whose offset lives inside their projectile spawner read zero here. */
+    /* muzzle — 0x800AE9C4 onward, stored (right, DOWN, forward), exactly as
+     * the disc holds them. The fire function negates the middle component into
+     * the rotation (0x8004C01C) and negates the rotated Y coming out
+     * (0x8004C04C), so the two cancel and nothing here pre-negates — see the
+     * header, and the loader below, which say the same. Weapons whose offset
+     * lives inside their projectile spawner read zero here. */
     .muzzle = {
         {   0,   0,   0 },   /* 0  no weapon                                 */
         {  80,  56, 250 },   /* 1  blaster           0x800AE9C4              */

src/client/main.c

@@ -6673,6 +6673,28 @@ static void client_input_simulated(client *c, float dt)
     if (c->mp_enabled)
         client_targets_for(c, 0);
 
+    /*
+     * AND THE SIM IS TOLD IT IS A DEATHMATCH, which nothing ever did.
+     *
+     * `q2_sim.multiplayer` is 0x800AEBCC, and it is read in five places — the
+     * -3072 impulse ceiling, the end-of-frame basis rebuild, and the three that
+     * derive `sim->ent_world.deathmatch` from it, which is in turn what gates
+     * doubled item amounts, weapons-stay and item respawn. `sync_rules` in
+     * simcombat.c now takes the combat half from the same flag, so the
+     * railgun's 150 and armour's 2048 bias hang off it too.
+     *
+     * The only writer it had was the save loader. A match started from the menu
+     * left it false, so every one of those rules ran in its single-player form
+     * inside a deathmatch. Written here, once a frame and before the tick,
+     * because the sims are created and reset by the zone load rather than by
+     * `client_mp_configure` — setting it there alone would not survive.
+     */
+    {
+        int si;
+        for (si = 0; si < Q2_MP_MAX_PLAYERS; si++)
+            c->sim[si].multiplayer = c->mp_enabled;
+    }
+
     q2_combat_scan_who = c->mp_enabled ? 0 : Q2_COMBAT_SCAN_OTHER;
     /*
      * WHETHER THE WORLD ACTUALLY MOVED. `q2_sim_advance` returns 0 when the

src/game/combat.c

@@ -1,5 +1,7 @@
 #include "combat.h"
 
+#include "playerdeath.h"   /* Q2_PDEATH_GIB_HEALTH: one copy of 0x800397FC */
+
 #include <math.h>
 #include <string.h>
 
@@ -227,9 +229,29 @@ void q2_actor_from_player(q2_actor *a, const q2_inventory *inv,
         return;
     a->health       = inv->health;
     a->armour       = inv->armour;
-    a->armour_class = 0;
+    /*
+     * WHICH ARMOUR, not always the weakest. This was a hardcoded 0, and 0 is
+     * jacket — so a player wearing body armour absorbed 0.30 of an ordinary hit
+     * instead of 0.80, and 0.00 of an energy hit instead of 0.60. Because this
+     * runs on EVERY damage attempt the class could never survive one either:
+     * there was no path by which the field could hold anything else.
+     *
+     * `q2_item_armour_amount` (0x8003733C) is what maintains the class, and it
+     * is the same index the three-record table at 0x8009C5EC is read with.
+     */
+    a->armour_class = inv->armour_class;
     a->cells        = inv->ammo[Q2_AMMO_CELLS];
-    a->gib_health   = -100;
+    /*
+     * The power items live in the inventory's flag word, and
+     * Q2_POWERUP_POWER_ARMOUR (0x18000) is exactly Q2_INV_POWER_SHIELD |
+     * Q2_INV_POWER_SCREEN — the pair 0x80057AC4 tests together. Leaving this
+     * zero meant q2_combat_power_armour_absorb returned at its first guard, so
+     * the shield spent no cells and saved nothing.
+     */
+    a->powerups     = inv->flags;
+    /* 0x800397FC: the player's own gib threshold, which playerdeath.h reads
+     * from the same instruction. -100 here was a second, disagreeing copy. */
+    a->gib_health   = Q2_PDEATH_GIB_HEALTH;
     a->invuln_until = inv->invuln_until;
     a->protect_until = inv->enviro_until;
 }
@@ -439,6 +461,35 @@ q2_damage_result q2_combat_damage(q2_actor *attacker, q2_actor *target,
      * 0x800582C8: at skill 0, a monster hitting a player does half. The test is
      * on the ATTACKER having no client block, which is what makes it "a monster
      * hit you" rather than "you were hurt".
+     *
+     * AND IT ROUNDS UP, which is the whole of the arithmetic and was the one
+     * figure in this file carrying no instruction behind it. The prologue fixes
+     * the registers — 0x80057D5C `addu s5, a0` is the attacker, 0x80057D64
+     * `addu s2, a1` the target, 0x80057D6C `addu s1, a2` the damage, and
+     * 0x80057D94 `lw s0, 12(s2)` the target's client block — and the four
+     * guards and the halving read:
+     *
+     *     800582C8  beq   s0, zero, ...      ; the target has a client
+     *     800582D0  lh    v0, 0x800B334A     ; the skill halfword
+     *     800582D8  bne   v0, zero, ...      ; only at skill 0
+     *     800582E0  lw    v0, 748(s5)        ; attacker+0x2EC, its entity
+     *     800582E8  beq   v0, zero, ...
+     *     800582F0  lw    v0, 12(s5)         ; the attacker's client block
+     *     800582F8  bne   v0, zero, ...      ; only when it has none
+     *     800582FC  addiu v0, s1, 1          ; delay slot: damage + 1
+     *     80058300  sra   s1, v0, 1          ; damage = (damage + 1) >> 1
+     *
+     * The +1 is UNCONDITIONAL. A compiler's signed `/2` would have emitted the
+     * sign-bit idiom instead — `srl v0, x, 31; addu; sra` — and there is no
+     * such term here, so this is `(damage + 1) >> 1` in the source and not a
+     * truncating divide. 25 points of monster damage arrive as 13, not 12,
+     * which also gives id's "never rounds to nothing" for free.
+     *
+     * The third guard is the one thing not reproduced: `attacker+0x2EC` is the
+     * actor's entity back-pointer (monster.h), zeroed at 0x8007F0DC when a body
+     * stops being a creature, so the console additionally refuses the halving
+     * for an attacker that has become a corpse. `attacker != NULL` stands in
+     * for it; nothing in this port damages anyone from a detached actor.
      */
     if (rules->skill == 0 && target->has_client &&
         attacker && !attacker->has_client)

src/game/monster.c

@@ -1,5 +1,6 @@
 #include "monster.h"
 
+#include "combat.h"        /* Q2_HEALTH_FLOOR: one copy of 0x800629B4's clamp */
 #include "worldscale.h"
 
 #include <stdlib.h>
@@ -376,8 +377,12 @@ bool q2_monster_corpse_tick(q2_monster *m)
  * A corpse's health floors here rather than in the arithmetic: 0x800629B4
  * clamps to -9999 AFTER the subtraction, so a rocket into a body that is
  * already down cannot drive it arbitrarily negative and out of gib range.
+ *
+ * ONE COPY of that figure. This file used to spell it out a second time under
+ * its own name, so the two clamps in the port — this one and combat.c's — could
+ * drift apart while both looked cited.
  */
-#define Q2_MONSTER_HEALTH_FLOOR (-9999)
+#define Q2_MONSTER_HEALTH_FLOOR Q2_HEALTH_FLOOR
 
 /* Nightmare skill: five seconds on the 10 Hz clock, 0x80062B20. */
 #define Q2_PAIN_DEBOUNCE_SKILL3 Q2_AI_SECONDS(5)

src/game/simcombat.c

@@ -400,6 +400,35 @@ bool q2_sim_autoselect_weapon(q2_sim *sim)
  * origin + dir — so the fraction the hull returns is exactly what the entity
  * pass needs to bound itself with.
  */
+/*
+ * THE COMBAT RULES ARE SESSION STATE, and until now only the clock in them was.
+ *
+ * `q2_combat_rules_default` memsets the struct and writes skill 1, and the two
+ * call sites that touched it afterwards refreshed `level_time` and nothing
+ * else. So the other two fields were frozen at their defaults for the whole run
+ * and every rule that reads them was dead:
+ *
+ *   - `skill` never left 1, so 0x800582C8's "a monster hits you at skill 0 for
+ *     half" never fired, even though the front end already knows the skill and
+ *     hands it to the creature AI through `q2_cre_set_skill`. Easy was as
+ *     dangerous as medium.
+ *   - `deathmatch` never left false, so the railgun's 150 (0x8004D5D8) was
+ *     unreachable, armour always used the 4095 single-player bias instead of
+ *     2048, and the -3072 rocket-jump ceiling was applied inside deathmatch,
+ *     where 0x800580E8 does not apply it.
+ *
+ * Both are read from where the rest of the port already keeps them rather than
+ * from a new copy: the skill from the AI's own global, deathmatch from the
+ * `sim->multiplayer` flag that `sim->ent_world.deathmatch` is already taken
+ * from, so the item half and the combat half of deathmatch cannot disagree.
+ */
+static void sync_rules(q2_sim *sim)
+{
+    sim->combat.rules.level_time = sim->level_time;
+    sim->combat.rules.skill      = (s16)q2_cre_skill();
+    sim->combat.rules.deathmatch = sim->multiplayer;
+}
+
 static s32 world_fraction_for(q2_sim *sim, const s32 origin[3],
                               const s32 dir[3])
 {
@@ -435,7 +464,7 @@ q2_fire_result_v2 q2_sim_fire(q2_sim *sim)
     q2_sim_eye(sim, eye);
     q2_sim_aim(sim, aim);
 
-    sim->combat.rules.level_time = sim->level_time;
+    sync_rules(sim);
     q2_actor_from_player(&sim->combat.self, &sim->combat.inv, sim->player[sim->cur_player].pos);
 
     r = q2_weapon_fire(&sim->combat.inv, &sim->combat.rng, NULL,
@@ -444,7 +473,21 @@ q2_fire_result_v2 q2_sim_fire(q2_sim *sim)
                        sim->player[sim->cur_player].pitch,
                        sim->player[sim->cur_player].roll, aim,
                        sim->level_time, sim->combat.next_fire,
-                       false, sim->combat.rules.deathmatch,
+                       /*
+                        * QUAD, which was a hardcoded `false` — so the powerup
+                        * was picked up, counted down on the HUD and played
+                        * `itm_damage3` on every shot while multiplying nothing.
+                        *
+                        * Every fire function opens by comparing the level clock
+                        * against the player's own expiry word (client+0xAC,
+                        * which is `quad_until`) and picks the second immediate
+                        * on the near side of it — 8 or 32, 6 or 24, 120 or 480.
+                        * That comparison is this argument, and the view model
+                        * already forms exactly it for the sound
+                        * (`q2_vw.quad_active`).
+                        */
+                       sim->level_time < sim->combat.inv.quad_until,
+                       sim->combat.rules.deathmatch,
                        sim->combat.chaingun_bullets);
 
     sim->combat.last_shot = r;
@@ -615,7 +658,7 @@ q2_damage_result q2_sim_hurt_player(q2_sim *sim, q2_actor *attacker,
     /* Health and armour live in the inventory, everything else in the actor, so
      * the two are synchronised around the call rather than duplicated. */
     q2_actor_from_player(&sim->combat.self, &sim->combat.inv, sim->player[sim->cur_player].pos);
-    sim->combat.rules.level_time = sim->level_time;
+    sync_rules(sim);
 
     /*
      * A CONTACT HIT LANDS AT THE ATTACKER, and that is not the caller's choice

src/game/weapon.h

@@ -77,6 +77,15 @@
  * which is all of them except the hyperblaster, whose rate comes from its view
  * model animation loop instead. So the console does not have per-weapon fire
  * rates in the PC sense; it has one gate plus animation length.
+ *
+ * AND NOTHING READS IT BACK, which is why every `refire` in the table below is
+ * zero rather than 30. The write to client+204 is real and FORMATS.md §13.1
+ * records it; no instruction in the weapon path loads that word again. The
+ * console's idle state calls the fire function once per fire pass and then
+ * enters the FIRE clip, so the CLIP is the rate — and a port that also armed a
+ * 30-tick gate here would be adding a second, invented limiter on top of it.
+ * `Q2_WEAPON_DRY_REFIRE` is the one gate this module does arm, and it is the
+ * port's own, for the empty-trigger case the console has no deadline for.
  */
 #ifndef Q2PSX_WEAPON_H
 #define Q2PSX_WEAPON_H

tests/test_combat.c

@@ -19,6 +19,7 @@
 #include "combat.h"
 #include "monster.h"
 #include "multiplayer.h"
+#include "playerdeath.h"
 #include "projectile.h"
 #include "trig.h"
 #include "weapon.h"
@@ -111,6 +112,51 @@ static void test_player_powerup_sync(void)
     hit = q2_combat_damage(NULL, &player, 40, Q2_MOD_LAVA, pos, &rules);
     check(!hit.blocked, "the protection expiry tick is no longer protected");
     check(player.health < 100, "damage resumes on the expiry tick");
+
+    /*
+     * WHICH ARMOUR THE PLAYER IS WEARING, which this projection used to drop:
+     * `armour_class` was written as a literal 0, so every class absorbed at
+     * jacket's 1229/4096 and none of them absorbed energy at all. The
+     * projection runs on every damage attempt, so there was no window in which
+     * the field could hold anything else.
+     */
+    q2_inventory_init(&inv);
+    inv.armour       = 200;
+    inv.armour_class = Q2_ARMOUR_BODY;
+    q2_actor_from_player(&player, &inv, pos);
+    check_eq_i(player.armour_class, Q2_ARMOUR_BODY,
+               "the armour class reaches the damage actor");
+
+    rules.level_time = 0;
+    hit = q2_combat_damage(NULL, &player, 100, Q2_MOD_BULLET, pos, &rules);
+    check_eq_i(hit.absorbed_armour, 81,
+               "body armour saves (4095 + 3277*100) >> 12, not jacket's 31");
+
+    q2_actor_from_player(&player, &inv, pos);
+    hit = q2_combat_damage(NULL, &player, 100, Q2_MOD_ENERGY_BOLT, pos, &rules);
+    check_eq_i(hit.absorbed_armour, 61,
+               "and 2458/4096 against energy, where jacket's column is zero");
+
+    /*
+     * And the two power bits. Q2_POWERUP_POWER_ARMOUR is exactly the pair of
+     * inventory flags 0x80057AC4 tests, so the whole word carries across;
+     * `powerups` used to be left at zero and the shield absorbed nothing.
+     */
+    q2_inventory_init(&inv);
+    inv.flags = Q2_INV_POWER_SHIELD;
+    inv.ammo[Q2_AMMO_CELLS] = 100;
+    q2_actor_from_player(&player, &inv, pos);
+    check_eq_i(player.powerups & Q2_POWERUP_POWER_ARMOUR, Q2_INV_POWER_SHIELD,
+               "the power shield's bit reaches the damage actor");
+
+    hit = q2_combat_damage(NULL, &player, 30, Q2_MOD_BULLET, pos, &rules);
+    check_eq_i(hit.absorbed_power, 20, "and it absorbs two thirds of the hit");
+    check_eq_i(player.cells, 90, "spending one cell per two points");
+
+    /* 0x800397FC's threshold, which playerdeath.h reads from the same
+     * instruction. This projection used to carry a disagreeing -100. */
+    check_eq_i(player.gib_health, Q2_PDEATH_GIB_HEALTH,
+               "the player's gib threshold is the one playerdeath.h names");
 }
 
 /* ------------------------------------------------------------------------- */
@@ -307,6 +353,9 @@ static void test_damage(void)
         target.has_client = true;
         place(&attacker, 0, 0, -1000, 100);   /* no client: a creature */
 
+        /* Rounding UP, and the disc says so: 0x800582FC `addiu v0, s1, 1` into
+         * 0x80058300 `sra s1, v0, 1`, with the +1 unconditional rather than the
+         * sign-bit term a truncating `/2` would carry. 31 halves to 16. */
         q2_combat_damage(&attacker, &target, 31, Q2_MOD_BULLET, NULL, &easy);
         check_eq_i(target.health, 100 - 16, "skill 0 halves, rounding up");