Use DEH-backed green armor class in `P_TouchSpecialThing` by Copilot · Pull Request #139 · sunsided/room

Copilot · 2026-05-21T21:51:39Z

P_TouchSpecialThing handled SPR_ARM1 differently from the C implementation by hardcoding armor class 1. That bypassed the runtime deh_green_armor_class value and ignored Dehacked overrides for green armor pickups.

Behavior fix
- Switch the SPR_ARM1 pickup path in room/src/doom/p_inter.rs from a hardcoded armor class to deh_green_armor_class.
- Align the Rust port with the original C behavior for green armor handling.
DEH-backed state
- Introduce a mutable deh_green_armor_class global initialized from DEH_DEFAULT_GREEN_ARMOR_CLASS.
- Preserve current default behavior while allowing runtime overrides to flow through the pickup path.
Code cleanup
- Remove the stale FIXME documenting the hardcoded value once the port mismatch is resolved.
Regression coverage
- Add a focused unit test asserting that the green armor runtime value defaults to the expected DEH constant.

SPR_ARM1 => {
    if P_GiveArmor(player, deh_green_armor_class) == 0 {
        return;
    }
    (*player).message = DEH_String(GOTARMOR);
}

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

dl.google.com
- Triggering command: /usr/lib/apt/methods/https /usr/lib/apt/methods/https /home/REDACTED/work/room/room/target/debug/deps/bytemuck_derive-16f098d6c9015d52.bembed-bitcode=no /home/REDACTED/work/room/room/target/debug/deps/bytemuck_derive-16f098d6c9015d52.b-C /home/REDACTED/work/room/room/target/debug/deps/bytemuck_derive-16f098d6c9015d52.bdebuginfo=2 ist /home/REDACTED/work/room/room/target/debug/deps/bytemuck_derive-16f098d6c9015d52.bcfg(docsrs,test) /home/REDACTED/work/room/room/target/debug/deps/bytemuck_derive-16f098d6c9015d52.b--check-cfg /home/REDACTED/work/room/room/target/debug/deps/rustcpubmlD/rmeta.o iserror_impl.43a1e06bdc12391-cgu.00.rcgu.o iserror_impl.43a1e06bdc12391-cgu.01.rcgu.o iserror_impl.43a1e06bdc12391-cgu.02.rcgu.o iserror_impl.43a1e06bdc12391-cgu.03.rcgu.o iserror_impl.43a1e06bdc12391-cgu.04.rcgu.o iserror_impl.43a1e06bdc12391-cgu.05.rcgu.o iserror_impl.43a1e06bdc12391-cgu.06.rcgu.o iserror_impl.43a1e06bdc12391-cgu.07.rcgu.o iserror_impl.43a1e06bdc12391-cgu.08.rcgu.o iserror_impl.43a1e06bdc12391-cgu.09.rcgu.o iserror_impl.43a1e06bdc12391-cgu.10.rcgu.o iserror_impl.43a1e06bdc12391-cgu.11.rcgu.o (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Configure Actions setup steps to set up my environment, which run before the firewall is enabled
Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

Agent-Logs-Url: https://github.com/sunsided/room/sessions/459b1093-feb2-4ba6-8124-91f7ab52ba3c Co-authored-by: sunsided <495335+sunsided@users.noreply.github.com>

Copilot

Pull request overview

This PR fixes a behavioral mismatch in the Doom gameplay port by ensuring the green armor pickup (SPR_ARM1) uses a DEH-backed armor class value instead of a hardcoded 1, matching the upstream C logic and enabling runtime overrides to flow through the pickup path.

Changes:

Add a #[no_mangle] pub static mut deh_green_armor_class global initialized from DEH_DEFAULT_GREEN_ARMOR_CLASS.
Update P_TouchSpecialThing so the green armor pickup calls P_GiveArmor(player, deh_green_armor_class).
Add a unit test asserting the global defaults to the DEH constant and remove the stale FIXME about the previous hardcoding.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Auto-generated GitNexus summary refreshed after re-analyze.

…in pickup paths Address PR #139 review feedback and reduce raw-pointer use in the pickup helpers. What: - Add DEH-tunable runtime globals matching upstream Chocolate Doom `p_inter.c`: `deh_max_health`, `deh_max_armor`, `deh_blue_armor_class`, `deh_max_soulsphere`, `deh_soulsphere_health`, `deh_megasphere_health`. Pickup paths (SPR_ARM2 / BON1 / BON2 / SOUL / MEGA) now read the runtime globals instead of `DEH_DEFAULT_*` constants, and the megasphere armor call drops the hardcoded `2`. - Drop the stale FIXME doc block on `P_TouchSpecialThing`: the `toucher->health <= 0` guard is already in the function body. - Expand the global-defaults unit test to cover every new `deh_*` static. - Convert pickup-path raw pointers to references inside `P_TouchSpecialThing`, `P_GiveAmmo`, `P_GiveWeapon`, `P_GiveBody`, `P_GiveArmor`, `P_GiveCard`, and `P_GivePower`. The `extern "C"` signatures still take `*mut PlayerT` / `*mut mobj_t`; the bodies bind `&mut` once after entry and use field access via `.`. Raw pointers are still passed to FFI helper calls. Why: - Match upstream behavior so DEHacked overrides flow through every pickup, not only green armor. - Improve doc accuracy after the dead-toucher guard was added in #128. - Reduce raw-pointer dereferences in the hot pickup paths so the borrow checker covers the common case while keeping the C ABI intact.

…or-class-hardcode # Conflicts: # room/src/doom/p_inter.rs

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

…ve aliasing UB Address PR #139 review feedback flagging undefined behavior: holding `&mut PlayerT` derived from a raw pointer while calling an `extern "C"` helper that re-derives `&mut PlayerT` from the same raw pointer creates overlapping mutable references and violates Rust's aliasing rules. What: - Add Rust-only inner functions taking `&mut PlayerT`: `p_give_ammo`, `p_give_weapon`, `p_give_body`, `p_give_armor`, `p_give_card`, `p_give_power`. - Reduce each `extern "C" fn P_Give*` to a thin shim that does the single `&mut *player` deref and delegates. - `p_give_weapon` and `p_give_power` now call the inner `p_give_*` helpers directly, passing the existing `&mut PlayerT` instead of reborrowing through the raw pointer. - `P_TouchSpecialThing` holds `&mut PlayerT` for the long-lived binding and uses the inner helpers, so no second `&mut PlayerT` is ever derived. `special` and `toucher` go back to raw `*mut` dereferences because `P_RemoveMobj` (an `extern "C"` helper that reborrows) is called against `special` in the epilogue. Why: - The previous "one top-level binding" pattern compiled but was UB under Stacked / Tree Borrows: every nested FFI call reborrowed the same `*mut PlayerT`, producing two live `&mut PlayerT` to the same memory. - Funneling the single `&mut PlayerT` through Rust-side helpers keeps the borrow checker covering the hot pickup paths while leaving the C ABI surface unchanged.

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

…ts to locals Address two PR #139 review comments. What: - Revert the megasphere (`SPR_MEGA`) armor grant to a hardcoded `2`. Upstream `p_inter.c` documents this explicitly: "We always give armor type 2 for the megasphere; dehacked only affects the MegaArmor". Routing it through `deh_blue_armor_class` would have diverged from vendor behavior. - Mirror the vendor comment about `deh_green_armor_class` not applying to the armor helmet (`SPR_BON2`). - In `P_TouchSpecialThing`, snapshot every `deh_*` `static mut` into a local at the top of the function instead of reading the global inside each match arm. Each unsafe static-mut read now happens once, in one place, instead of being scattered through the dispatch. Why: - Match upstream gameplay exactly; only the standalone MegaArmor pickup should react to DEHacked armor-class overrides. - Make the unsafety of static-mut reads visible at a single location and avoid hidden `unsafe_op_in_unsafe_fn`-style noise in the dispatch arms.

Initial plan

b5d5b2f

Copilot AI assigned Copilot and sunsided May 21, 2026

Copilot started work on behalf of sunsided May 21, 2026 21:51 View session

Copilot AI linked an issue May 21, 2026 that may be closed by this pull request

p_inter: P_TouchSpecialThing hardcodes green armor class instead of using deh_green_armor_class #126

Closed

Use DEH green armor class in p_inter

4e713f9

Agent-Logs-Url: https://github.com/sunsided/room/sessions/459b1093-feb2-4ba6-8124-91f7ab52ba3c Co-authored-by: sunsided <495335+sunsided@users.noreply.github.com>

Copilot AI changed the title ~~[WIP] Fix hardcoded green armor class in P_TouchSpecialThing~~ Use DEH-backed green armor class in P_TouchSpecialThing May 21, 2026

Copilot finished work on behalf of sunsided May 21, 2026 22:01

Copilot AI requested a review from sunsided May 21, 2026 22:01

sunsided marked this pull request as ready for review May 21, 2026 22:05

Copilot AI review requested due to automatic review settings May 21, 2026 22:05

Copilot started reviewing on behalf of sunsided May 21, 2026 22:05 View session

github-actions Bot added the area-p_inter Area: p_inter label May 21, 2026

Copilot AI reviewed May 21, 2026

View reviewed changes

Comment thread room/src/doom/p_inter.rs Outdated

Comment thread room/src/doom/p_inter.rs Outdated

sunsided added 2 commits May 22, 2026 18:28

chore(docs): refresh GitNexus index counts

0a0cbbd

Auto-generated GitNexus summary refreshed after re-analyze.

sunsided requested a review from Copilot May 22, 2026 16:33

Copilot started reviewing on behalf of sunsided May 22, 2026 16:33 View session

Merge remote-tracking branch 'origin/main' into copilot/fix-green-arm…

51148c4

…or-class-hardcode # Conflicts: # room/src/doom/p_inter.rs

Copilot AI reviewed May 22, 2026

View reviewed changes

Comment thread room/src/doom/p_inter.rs Outdated

Comment thread room/src/doom/p_inter.rs Outdated

Comment thread room/src/doom/p_inter.rs

sunsided requested a review from Copilot May 22, 2026 16:52

Copilot started reviewing on behalf of sunsided May 22, 2026 16:52 View session

Copilot AI reviewed May 22, 2026

View reviewed changes

Comment thread room/src/doom/p_inter.rs Outdated

Comment thread room/src/doom/p_inter.rs

sunsided merged commit a5740c7 into main May 22, 2026
3 checks passed

sunsided deleted the copilot/fix-green-armor-class-hardcode branch May 22, 2026 17:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use DEH-backed green armor class in `P_TouchSpecialThing`#139

Use DEH-backed green armor class in `P_TouchSpecialThing`#139
sunsided merged 7 commits into
mainfrom
copilot/fix-green-armor-class-hardcode

Copilot AI commented May 21, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Copilot AI commented May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

I tried to connect to the following addresses, but was blocked by firewall rules:

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Copilot AI commented May 21, 2026 •

edited

Loading