Skip to content

Commit

Permalink
Debug version of TERRAIN_REGION() that checks for invalid values
Browse files Browse the repository at this point in the history
and improved comments in Play2dSound and ComputePlayInfo()
  • Loading branch information
DanielGibson committed Apr 24, 2024
1 parent 41c4578 commit 923bf4b
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 6 deletions.
11 changes: 11 additions & 0 deletions Descent3/terrain.h
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,18 @@ extern terrain_tex_segment Terrain_tex_seg[TERRAIN_TEX_WIDTH * TERRAIN_TEX_DEPTH
// first object to render after cell has been rendered (only used for SW renderer)
extern short Terrain_seg_render_objs[];

#ifdef RELEASE
#define TERRAIN_REGION(x) ((Terrain_seg[0x7FFFFFFF & x].flags & TFM_REGION_MASK) >> 5)
#else // debug(-ish) builds - check if x is valid
inline int TERRAIN_REGION(int x) {
ASSERT(x != -1 && "invalid/unset room number (-1)!");
// Note: due to the 0x7FFFFFFF mask, terrSegIdx will be >= 0
int terrSegIdx = 0x7FFFFFFF & x;
// catch other invalid cell/segment numbers than -1 as well
ASSERT((terrSegIdx < TERRAIN_WIDTH * TERRAIN_DEPTH) && "invalid cellnum!");
return (Terrain_seg[terrSegIdx].flags & TFM_REGION_MASK) >> 5;
}
#endif

extern terrain_sky Terrain_sky;

Expand Down
15 changes: 9 additions & 6 deletions sndlib/hlsoundlib.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -922,13 +922,14 @@ bool hlsSystem::ComputePlayInfo(int sound_obj_index, vector *virtual_pos, vector
sound_seg = m_sound_objects[sound_obj_index].m_link_info.pos_info.segnum;
}

// sound_seg == -1 causes crashes when BOA_INDEX() calls TERRAIN_REGION()
// with that value (though by pure luck on 32bit platforms the overflow
// and truncation will likely use an address that doesn't crash,
// but is still invalid). At least one case that could cause this was fixed,
// sound_seg == -1 (which just means that the roomnum/segnum hasn't been
// initialized to a proper value yet )causes crashes when BOA_INDEX()
// calls TERRAIN_REGION() with that value. (By pure luck on 32bit platforms
// the overflow and truncation will likely use an address that doesn't crash,
// but it's still invalid). At least one case that could cause this was fixed,
// if there are others, the ASSERT should tell us about it
// (and if assertions are disabled, return false to handle this gracefully)
ASSERT(sound_seg != -1);
ASSERT(sound_seg != -1 && "invalid (unset) roomnum/segnum!");
if (sound_seg == -1)
return false;

Expand Down Expand Up @@ -1091,7 +1092,9 @@ int hlsSystem::Play3dSound(int sound_index, pos_state *cur_pos, object *cur_obj,
return -1;
// if the position doesn't belong to any valid room or cell,
// all this would fail anyway (in Emulate3dSound() -> ComputePlayInfo()),
// so might as well give up now
// so might as well give up now; furthermore, this prevents m_sound_objects[i]
// from remaining in an half-initialized state below (esp. for looping sounds
// where StopSound() wouldn't be called after Emulate3dSound() returns false)
if (cur_pos->roomnum == -1)
return -1;
// initialize sound.
Expand Down

0 comments on commit 923bf4b

Please sign in to comment.