User TRUEpiiiicness reports that valptridx<player>::check_index_range
traps when "The Apocalyptic Factor"[1] level 14 boss fires its Omega-based
weapon. Code inspection shows that this is expected, since the original
designers assumed OMEGA_ID would only ever be used by players, and coded
various shortcuts accordingly. No one told the level author this. The
boss is an interesting concept and not difficult to support, so adjust
the code to handle this situation correctly:
- Check that the shooter is a player before checking its player ID.
Without this, a robot that happened to have the same ID as the player
would interpret robot data as player data, likely causing corruption
when it tried to update the omega cannon charge. Even if the robot ID
does not match, this step causes a diagnostic[2] reporting that a
robot ID is being misused as a player ID.
- Remove the shortcut that assumes the shooter is a player. Store the
shooter's actual type.
- Remove the unnecessary and counterproductive path that:
1. Uses the object pointer to get the player's ID
2. Uses that player ID to get a player pointer
3. Uses that player pointer to get an object number
4. Uses that object number to get an object pointer
When invariants are maintained, the pointer derived in step 4 is equal
to the pointer used at the start of step 1. Use that pointer directly
instead of rederiving it. The reported exception was due to step 2,
which requires that the player ID is in range. When the shooter is a
player, this is true. When the shooter is not a player, it may not
be true.
[1] http://www.enspiar.com/dmdb/viewMission.php?id=418
```
sha1sum af.hog af.mn2
133c52fb4b4e5fd40bf7b2321789841b727d1d0b af.hog
0bb5f0dd1803b0fc1aac7c2023eacc97e9ab872b af.mn2
stat -c '%s %Y %n' af.hog af.mn2
4024838 1094445016 af.hog
566 1013524628 af.mn2
```
[2] `similar/main/laser.cpp:560: BUG: object 0x555556078958 has type 2, expected 4`
Reported-by: TRUEpiiiicness <https://forum.dxx-rebirth.com/showthread.php?tid=1038>
AddressSanitizer in gcc-7 inhibits deletion of certain otherwise
unnecessary nullptr checks, causing otherwise absent references to
class null_pointer_exception. As a convenience to AddressSanitizer
users, instantiate all valptridx templates when AddressSanitizer is
enabled. This is slightly excessive, but users building for
sanitization are not aiming for a minimal size build anyway.
AlexanderBorisov reported[1] a broken demo that ultimately traced to
incorrect reuse of stale data. The campaign[2] includes per-level robot
definitions for level 3. Descent incorrectly accessed Robot_info before
reloading it for the new level, so it used the level 3 data on level 4
robots during a warm start of level 4 for regular play, but used only
the level 4 data for a cold start of level 4 for regular play and for
playback of a demo recorded solely on level 4. The particular
customizations applied on level 3 caused a level 4 demo recorded with
level 3 robot customizations to use different record lengths than a
level 4 demo recorded with level 4 robot customizations. Since demos do
not record their record lengths, mismatched record lengths lead to
effectively corrupt demos. In this case, it manifested as writing 8
anim_angles (per model_num 108, the model incorrectly inherited from
level 3) instead of 1 anim_angles (per model_num 37, used only on cold
starts). The extra anim_angles were then misinterpreted as a run of 42
ND_EVENT_EOF, because the extra angles were unused and the newdemo code
defines the null byte as EOF (separate from receiving an actual EOF
indicator from the file I/O library). That run of spurious EOF caused
the demo playback code to refuse to play past the spurious EOF,
resulting in a seemingly broken demo.
As an unfortunate, but unavoidable, consequence, this change modifies
the common path (warm playthrough of every level in the campaign) to
work like the uncommon path (cold start of each level). Further, the
affected robot triggers a trap in d2x::do_silly_animation[3] when using
the correct model_num, because that polygon model has too few models for
the joints used by the robot. When using the incorrect level 3 data,
the robot animates without trapping.
[1] https://forum.dxx-rebirth.com/showthread.php?tid=1023
[2] http://www.enspiar.com/dmdb/viewMission.php?id=212
```
sha1sum ENTROPY.HOG ENTROPY.MN2
7bc7a12d00a1ddd3ae92ce90eb67d581ecab004a ENTROPY.HOG
f2688a634f22b30a02c43d8fa1a049deb5a03f70 ENTROPY.MN2
stat -c '%s %Y %n' ENTROPY.HOG ENTROPY.MN2
2402638 875757712 ENTROPY.HOG
511 875757164 ENTROPY.MN2
```
[3]
```
799 if (jointnum >= Polygon_models[objp.rtype.pobj_info.model_num].n_models) {
800 Int3(); // Contact Mike: incompatible data, illegal jointnum, problem in pof file?
801 continue;
802 }
```
gcc silently permits this, as does every clang version I tested.
However, in issue #360, thfrwn referenced an e-mail thread that claims
OpenBSD clang warns for this. [1] This capture is not needed, and as
far as I can tell, was not needed even when it was added. It was likely
required in an earlier iteration of the code, but should not have been
pushed. Remove it.
[1] https://marc.info/?l=openbsd-ports&m=151402743101761&w=2
Fixes: d4df692d30 ("Use range_for in draw_all_edges")
Presently, user profiles are not portable between a build with
max_axes_per_joystick=0 and one with max_axes_per_joystick!=0.
Attempting to do so causes some buttons to be assigned to the wrong
action. Since the intended use case for max_axes_per_joystick=0 and
similar is to completely remove joystick support on a system that will
never use a joystick, this is acceptable for now. A future overhaul to
improve storage of user kconfig settings may fix this.
hengyn45 reports that D1 improperly allows cooperative games on anarchy
only maps.
D1 seems not to have implemented this for a very long time, if ever,
although the code existed in commented out form. When the trees were
combined, the commented out form was treated as blank, so the
restriction was placed inside a #if-d2 guard. There is no reason to
make this D2-only. Remove the guard so that D1 enforces anarchy-only.
Reported-by: hengyn45 <https://forum.dxx-rebirth.com/showthread.php?tid=1017>
vclip::frame_time equal to 0 would cause an infinite loop.
vclip::frame_time less than 0 would cause the loop to run until
underflow, which could be very slow if frame_time is near 0.
- Enable display of subsecond (Linux: microseconds; Windows:
milliseconds) precision on gamelog timestamps.
- Add disabled support for YYYY-MM-DD leaders on gamelog timestamps.
Activate it by defining DXX_CONSOLE_TIME_FORMAT_YMD to true.
- Add disabled support for capturing caller __FILE__, __LINE__ in calls
to con_printf, con_puts. Activate it by defining
DXX_CONSOLE_SHOW_FILE_LINE to true. If captured, write those to
gamelog after the timestamp and before the text. This feature (and
only this feature) requires that DXX_HAVE_CXX_BUILTIN_FILE_LINE be
defined, which is conditional on if the compiler has __builtin_FILE()
and __builtin_LINE(). If the compiler lacks this support, attempts to
enable this feature are ignored.
- Switch to using GetLocalTime on Windows.
It generates code different from the current definitions, even when no
logical changes are present. This code has not been used in at least 2
years, since commit bc7c469ab2 ("Use
array<> for more globals") changed the definitions of some variables
that this code regenerates, and this code still generates the old
definition. Future changes will cause further deviation.
Commit 7d36df315e restructured
draw_weapon_vclip, but unintentionally broke the spin of proximity bombs
because `modtime` could exceed `play_time * 2`, causing
`draw_vclip_object` to clamp `bitmapnum` to the last frame of the
animation. Switch from `-=` to `%=` so that modtime is always brought
back in range.
Reported-by: Descender1032 <https://forum.dxx-rebirth.com/showthread.php?tid=1005>
Fixes: 7d36df315e ("Simplify draw_weapon_vclip")
When `apply_force_damage` was called to damage a robot, it tested if the
`ctype.laser_info.parent_signature` of the other object matched
`ConsoleObject->signature` and, if so, awarded points to the console
player. This allowed the player to sometimes get points for ramming a
robot to death, but was buggy. When the player rams a robot, the player
is the other object, so accessing `ctype.laser_info` is wrong.
Historically, the player's signature was zero and player
`ctype.laser_info.parent_signature` (an inactive and therefore incorrect
branch of the union) happened to be zero, so the test succeeded. Commit
44753209d6 changed the layout of
`struct player_info` such that
`ConsoleObject->ctype.laser_info.parent_signature` was not zero, causing
the player not to be his own parent. Prior to this commit, the player
could still wrongly fail the test if the `player_info` member that
aliased `laser_info.parent_signature` was nonzero. In the commit
preceding the obvious break, `player_info.player_flags` aliased
`laser_info.parent_signature`, so a player with certain flags (such as
invulnerability) would wrongly fail the test. The exact manifestation
has changed over time as `struct player_info` gained members.
Restructure the test so that a ramming kill always awards points,
without checking ancestry.
Reported-by: Descender1032 <https://forum.dxx-rebirth.com/showthread.php?tid=1006> (no commit identifiers given)
Fixes: 44753209d6 ("Move homing_object_dist to object.ctype.player_info")
`Descent 2: Counterstrike` level 9 specifies a control center trigger on
a side with no wall. This is not valid. In normal play, wall_toggle
silently ignored this, and newdemo_pop_ctrlcen_triggers had undefined
behavior. Add a warning in wall_toggle to report levels like this. Add
handling in newdemo_pop_ctrlcen_triggers to detect and suppress this
bogus data.
`Descent 2: Counterstrike` level 11 specifies a control center trigger
on a side with no child segment. This is not valid, so
newdemo_pop_ctrlcen_triggers again had undefined behavior. Add handling
to detect and suppress this bogus data.
Reported-by: Descender1032 <https://forum.dxx-rebirth.com/showthread.php?tid=992>
clang rejects 0x80000000 as a non-type template argument of type `int`
due to narrowing. Use `INT32_MIN` instead, which has the same numeric
value, and should be accepted without requiring narrowing.
Reported-by: Havner <https://github.com/dxx-rebirth/dxx-rebirth/issues/353>
Fixes: 88832e3679 ("Use constexpr integral_constant for various magic numbers")
Global variable `grd_curcanv` is set to a variety of canvases, some of
which are local stack variables. Use of global variables in this way is
fragile, but works as long as the global is not used beyond the life of
the backing local.
Unfortunately, some existing uses do access the canvas beyond the
lifetime of the backing local. Playing movies sets the font of the
current canvas. If the current canvas is an expired stack variable,
setting the font overwrites other stack data. This data corruption
causes various symptoms, such as inability to play the escape tunnel
movie.
Prior to 03cca2b3dc, the corruption on
playing the endlevel movie had no user-visible effect. That commit
created a large local variable, which changed stack layout. Starting
with that commit, the corruption causes the movie to play as all black.
Fix this, and protect against some other data corruption possiblities,
by clearing the global when the local goes out of scope.
Reported-by: Havner <https://github.com/dxx-rebirth/dxx-rebirth/issues/345> (only as cutscene failure to play, not as the underlying corruption issue)
Rather than use an inline wrapper and rely on the compiler optimizer to
redirect gr_set_current_canvas(nullptr) to gr_set_default_canvas,
rewrite all relevant calls directly in the source.
git grep -l 'gr_set_current_canvas' | xargs sed -i -e 's:gr_set_current_canvas(\s*NULL\|nullptr\s*);:gr_set_default_canvas();:'
Defining SLEW_ON breaks the build due to use of member variable names
not used since before the D2X import in 2001.
`unifdef -USLEW_ON -m -- similar/main/endlevel.cpp`
This code was part of a feature abandoned before retail. It cannot be
usefully used in campaigns. Remove it to reduce code size and simplify
later changes.
