Commit 88b5e616a9 ("Replace calls to
window_set_visible in DoPlayerDead() with stop/start_time()") switched
from hiding the game window to only stopping time, and that only if
hiding the window would have stopped time. In multiplayer, this allows
time to continue running. This introduced a crash if the player dies
during the mine countdown. When the player dies, the game checks if the
reactor has been destroyed. If so, the game immediately advances to the
next level. That advance will try to draw the game window if it is
visible. When the window is drawn, if time is not stopped, then game
logic runs. Some of that logic is not prepared to deal with the
inconsistent state present when a new level is only partially loaded.
That inconsistent state then causes a crash. Call stack:
#11 slew_frame () at similar/main/slew.cpp:152
#12 in d2x::object_move_one () at similar/main/object.cpp:1758
#13 in d2x::object_move_all () at similar/main/object.cpp:1956
#14 in d2x::GameProcessFrame () at similar/main/game.cpp:1848
#15 d2x::game_handler () at similar/main/game.cpp:1615
#16 in dcx::window_send_event () at common/include/window.h:116
#17 dcx::event_process () at common/arch/sdl/event.cpp:176
#18 in newmenu_do2 () at similar/main/newmenu.cpp:498
#19 in newmenu_do2<dcx::unused_newmenu_userdata_t> () at common/main/newmenu.h:184
#20 newmenu_do<dcx::unused_newmenu_userdata_t const> () at common/main/newmenu.h:190
#21 newmenu_do<1ul, dcx::unused_newmenu_userdata_t const> () at common/main/newmenu.h:196
#22 net_udp_wait_for_requests () at similar/main/net_udp.cpp:4563
#23 net_udp_level_sync () at similar/main/net_udp.cpp:4607
#24 in multi_level_sync () at similar/main/multi.cpp:3458
#25 in d2x::StartNewLevelSub () at similar/main/gameseq.cpp:1803
#26 in d2x::StartNewLevel () at similar/main/gameseq.cpp:2018
#27 in d2x::AdvanceLevel () at similar/main/gameseq.cpp:1648
#28 in d2x::DoPlayerDead () at similar/main/gameseq.cpp:1721
The root cause of this is the layering violation:
- Killing the player can have the side effect of advancing the level
- Advancing the level can have the side effect of calling multiplayer code while the level data is in an inconsistent state
- Calling multiplayer code can cause the event system to redraw the game
- Redrawing the game can cause game logic to run
Hack around this by restoring the logic that hides the game window, so
that the window is not redrawn and the game logic is not run. This does
not fix the layering problem, but prevents crashing affected users. To
avoid undoing the feature from the breaking commit, hide the window only
when advancing to a new level, rather than unconditionally. A player
advancing to a new level already lacks the move-at-cold-start capability
even on successfully escaping the mine, so no functionality is lost with
this change. Players who are dead and do not advance to a new level
retain that capability.
Fixes: 88b5e616a9 ("Replace calls to window_set_visible in DoPlayerDead() with stop/start_time()")
Reported-by: Ninjared <https://forum.dxx-rebirth.com/showthread.php?tid=1097>
- Raise the player limit to 8.
- Remove the logic that forces player counts up/down when switching
between cooperative and deathmatch game modes.
- Add heuristics to add start positions for the extra players, since
standard maps will not have the required number of starts.
Add macro cf_assert ("control flow" assert) to hint to gcc that certain
conditions are impossible. Use it to avoid generating range checks for
situations that never happen. If the event did happen, the only
consequence would be truncated UI text, rather than a correctness
problem.
Move it to a structure. Make all the modifiers methods. Change all
callers of those methods to pass the structure. This makes the stuck
object handling free of direct access to global game data.
The player appearance sound was tied to the player appearance vclip,
but this is wrong because the vclip has a shorter lifetime than the
sound. This mistake was previously hidden by the hack that caused
non-permanent sounds to be routed off to a separate queue that lost the
association between object and sound. Commit 4a98e79 eliminated that
hack because it complicated the work of that commit. However, without
the hack, object sound effects cannot outlive their host object.
Fix this by binding the sound to the position of the appearance effect,
not to the appearance object. This works since the appearance effect
cannot move, so there was no value to binding the sound to the object.
This solution could not be used for objects that move and terminate
before their associated sound effect.
Reported-by: Ryusei117 <https://github.com/dxx-rebirth/dxx-rebirth/issues/88#issuecomment-390054173>
Fixes: 4a98e796ab ("Prevent stacking weapon rotation sounds")
digi_play_sample_once is supposed to cancel prior instances of the
sound, but that functionality was lost in
21082c6db2. As a result, the sound can be
stacked up to the limit of the engine. Even when the functionality
existed, its implementation was wrong.
- Change these sounds to be attached to the player generating them.
- Pass the cancellation flag to other players when sending a sound.
- Send the full value of `volume`, rather than truncating it.
- Implement cancellation by killing and restarting the earlier version of a cancelled sound.
- Delete an ugly hack that prevented the patched logic from ever running.
- Fix an ancient quirk in digi_mixer that caused it to report all sounds as not playing, which then caused digi_sync_sounds to instantly cancel the new sound.
Reported-by: Mako88 <https://github.com/dxx-rebirth/dxx-rebirth/issues/88>
Reported-by: ryusei117 <https://github.com/dxx-rebirth/dxx-rebirth/issues/88#issuecomment-269597361>
Fixes: 21082c6db2 ("Added own channel management to SDL_mixer sound interface since the builtin channel management of this lib cannot handle our needs; Little code cleanup")
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 }
```
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();:'
Commit f4b21088a0 ("Track vulcan ammo explicitly") fixed an original
retail bug that prevented the thief from stealing energy weapons,
because the thief could only steal weapons for which the player had ammo
and energy weapons never have ammo. This went unremarked for several
years, until a recent report of the new semantics as a game-breaking
regression because the thief is now "ridiculously potent".
Address this report, as well as an intermittently raised issue from
various users over time, by adding two new knobs to both the single
player "Gameplay" menu and the multiplayer setup screen: "Remove Thief
at level start" and "Prevent Thief Stealing Energy Weapons".
"Remove Thief" deletes the thief object during level load. It has no
impact on save games, and changing it after entering a level has no
effect on any thief already in the level.
"Prevent Thief Stealing" is checked at the moment of theft and, when
enabled, prevents stealing primary weapons other than Vulcan/Gauss.
This can be changed at will in single player and is immediately
effective. In multiplayer, this option can only be changed by the game
host in the pre-game setup.
For both knobs, there is one pair of checkboxes to control this as a
player preference, which applies in single player games. There is a
second pair of checkboxes in the multiplayer setup, which applies only
to multiplayer games. Therefore, in multiplayer, the host chooses thief
settings and all clients use the host's choice. The host may configure
the thief differently in multiplayer from how the host plays in single
player.
For users who wanted to remove the thief, no specific tally has been
kept for who requested it or when. Now that the code is being updated,
this is thrown in as an easy addition.
Reported-by: MegaDescent <http://forum.dxx-rebirth.com/showthread.php?tid=980> (for the thief stealing energy weapons as a game-breaking regression)
`value` is generic and unclear. It is always meant to be used as an
index into the Station array, so rename it `station_idx` to show this.
Define and consistently use `station_none` to represent that no station
is assigned.
All supported compilers have an acceptable <type_traits>. Commit
4cb3d46148 ("Move <type_traits> test to Cxx11RequiredFeature") made
<type_traits> support mandatory in August and no one has objected.
Remove the indirection and use namespace std directly for type_traits
members.
Previously, valptridx used PREFIX for allow-invalid+mutable, c#PREFIX
for allow-invalid+const, v#PREFIX for require-valid+mutable, vc#PREFIX
for require-valid+const. Convert the types, factories, and all usage
sites to specify a qualifier for all four combinations:
im#PREFIX -> allow-invalid+mutable
ic#PREFIX -> allow-invalid+const
vm#PREFIX -> require-valid+mutable
vc#PREFIX -> require-valid+const
Changes to common/include/valptridx.h and common/include/fwd-valptridx.h
are manual. All other changes are generated by:
git grep -lz -e '\(obj\|seg\|clwall\|wall\|actdoor\|trg\)\(ptridx\|ptr\|idx\)\(_t\)\?\>' | xargs -0 sed -i -e 's/\<\(v\?\)\(\(obj\|seg\|clwall\|wall\|actdoor\|trg\)\(ptridx\|ptr\|idx\)\(_t\)\?\)\>/\1m\2/g'
for the 'm' prefix and:
git grep -lz -e '\(obj\|seg\|clwall\|wall\|actdoor\|trg\)\(ptridx\|ptr\|idx\)\(_t\)\?\>' | xargs -0 sed -i -e 's/\<\([cm]\(obj\|seg\|clwall\|wall\|actdoor\|trg\)\(ptridx\|ptr\|idx\)\(_t\)\?\)\>/i&/g'
for the 'i' prefix.
clang becomes confused trying to determine which vm_distance_squared
constructor to use for a literal input of 0x7fffffffffffffff, even
though the size of the input requires it to be `long` and only one
constructor can take a `long`. Switch from an explicit
0x7fffffffffffffff to the symbolic constant INT64_MAX, which has the
same value, but a platform-appropriate suffix to force the compiler to
pick the right type.
For general clarity, switch some other instances of integer maximum
literals to symbolic constants of the same value.
This commit has no effect on the generated code (except for changes to
line numbers).
Reported-by: kreatordxx <https://github.com/dxx-rebirth/dxx-rebirth/pull/324>
Fixes: 17208cca79 ("Disallow int for vm_distance_squared")
This is so game_flush_inputs() isn't called - part of change allowing player to respawn and begin moving with the same keypress. With this commit, you can use this feature with the 'when dead, respawn by pressing the Fire key' option - i.e. hold down movement key, then press fire key to respawn.
Kreator proposed restoring the Descent 2 cheat that grants homing
capability to all weapons. This commit implements that proposition,
with some changes to the implementation details.
Based-on-patch-by: Chris Taylor <chris@icculus.org>
Requested-by: Chris Taylor <https://github.com/dxx-rebirth/dxx-rebirth/pull/318>
Replace call to window_close(Game_wind) with returning window_event_result::close to game_handler. Applies to when there is a failure in net_udp_level_sync(). Closing a window within its handler is problematic - it can result in an unstable state.
Replace calls to window_close(Game_wind) with returning window_event_result::close to game handler. Applies to when DoEndGame() is called, DoGameOver() is called, aborting in the kmatrix screen (multiplayer game) during AdvanceLevel() and playing one demo frame causes playback to stop in GameProcessFrame(). Closing a window within its handler is problematic - it can result in an unstable state.
