From 15c8a447134c78d80b8c3f64f20ae0cf266d5b6c Mon Sep 17 00:00:00 2001 From: Vedant Madane <6527493+VedantMadane@users.noreply.github.com> Date: Tue, 25 Aug 2026 03:04:08 +0530 Subject: [PATCH 1/2] fix: R_FindPlane aborts on visplane pool exhaustion - R_FindPlane calls i_error! on visplane pool exhaustion instead of returning null Fixes #117 Signed-off-by: Vedant Madane <6527493+VedantMadane@users.noreply.github.com> --- room/src/doom/r_plane.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/room/src/doom/r_plane.rs b/room/src/doom/r_plane.rs index 5c0c5bd6..da182512 100644 --- a/room/src/doom/r_plane.rs +++ b/room/src/doom/r_plane.rs @@ -23,6 +23,7 @@ use std::ffi::{c_int, c_short, c_uchar}; use std::ptr; +use crate::i_error; use super::m_fixed::{fixed_t, FixedDiv, FixedMul}; use super::r_sky; use super::tables; @@ -41,7 +42,7 @@ const SCREENHEIGHT: usize = crate::doom::i_video::SCREENHEIGHT as usize; /// Maximum number of simultaneous visplanes per frame. /// /// Doom aborts with `I_Error` when this limit is exceeded. The Rust port -/// currently returns a null pointer instead (see [`R_FindPlane`]). +/// aborts via [`i_error!`] when exceeded (see [`R_FindPlane`]). const MAXVISPLANES: usize = 128; /// Maximum number of `c_short` slots in the [`openings`] array. @@ -547,7 +548,7 @@ pub extern "C" fn R_ClearPlanes() { /// and light level 0. /// /// Returns a null pointer if the visplane pool (128 entries) is exhausted -/// (the C source would call `I_Error` instead). +/// (matches the C source `I_Error` path). /// /// Exported as `#[no_mangle]` for C callers. /// @@ -556,9 +557,6 @@ pub extern "C" fn R_ClearPlanes() { /// Reads and writes the `static mut` globals [`visplanes`], [`lastvisplane`], /// and [`r_sky::skyflatnum`]. The returned pointer is valid for the lifetime /// of the current frame (until the next [`R_ClearPlanes`] call). -// FIXME: C aborts with I_Error on MAXVISPLANES overflow; Rust returns null. -// Callers in r_segs dereference the result unconditionally, which will -// cause undefined behavior if the limit is hit. #[no_mangle] pub extern "C" fn R_FindPlane( height: fixed_t, @@ -591,8 +589,9 @@ pub extern "C" fn R_FindPlane( / std::mem::size_of::() >= MAXVISPLANES { - // Would call I_Error — for now just return null - return ptr::null_mut(); + // Match C doomgeneric: abort rather than hand callers a null + // visplane pointer (they dereference unconditionally). + i_error!("R_FindPlane: no more visplanes"); } let new_vp = lastvisplane; From 6ea8c189d9e96321b32ca4d745facc7e61cea375 Mon Sep 17 00:00:00 2001 From: Vedant Madane <6527493+VedantMadane@users.noreply.github.com> Date: Tue, 1 Sep 2026 12:27:33 +0530 Subject: [PATCH 2/2] docs: fix R_FindPlane exhaustion doc to match i_error! abort --- room/src/doom/r_plane.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/room/src/doom/r_plane.rs b/room/src/doom/r_plane.rs index da182512..4c16e12d 100644 --- a/room/src/doom/r_plane.rs +++ b/room/src/doom/r_plane.rs @@ -547,7 +547,7 @@ pub extern "C" fn R_ClearPlanes() { /// All sky flats (`picnum == skyflatnum`) share a single visplane at height 0 /// and light level 0. /// -/// Returns a null pointer if the visplane pool (128 entries) is exhausted +/// Aborts via [`crate::i_error!`] if the visplane pool (128 entries) is exhausted /// (matches the C source `I_Error` path). /// /// Exported as `#[no_mangle]` for C callers.