`make_nearby_robot_snipe` computed the `robot_info` of every `object` in the
area before checking whether those `object`s were of type `OBJ_ROBOT`. This
is wrong, but usually harmless since it checked the type before using
the resulting `robot_info`. Starting in commit 9f26e2211, this triggered
a warning whenever `make_nearby_robot_snipe` checked nearby non-robot
`object`s.
Rewrite the tests to check for type `OBJ_ROBOT` before reading the
`object`'s id.
Reported-by: zicodxx <https://github.com/dxx-rebirth/dxx-rebirth/issues/429>
Fixes: 9f26e2211e ("Warn on invalid object ID access")
Commit 9898d13 fixed a crash on long filenames, but also tried to
optimize the Destination Saturn hack. However, this optimization was
implemented incorrectly, and caused the game to read only the
Destination Saturn highest_level entry when playing the First Strike
campaign, rather than using the greater level of Destination Saturn or
First Strike.
Restore the old logic, but restrict it to D1X. D2X never used an empty
string for the First Strike campaign, so this logic could not trigger
there, which is also how the bug was missed in initial testing.
Reported-by: zicodxx <https://github.com/dxx-rebirth/dxx-rebirth/issues/428>
Fixes: 9898d136f1 ("Fix fortification crash loading missions with long filenames")
In multiplayer, the host sends a destroy message for both sides of the
wall. `do_exploding_wall_frame` processes both, but
`num_exploding_walls` is only decreased by 1, causing an assertion
failure at the end. Remove the assertion, since the wall does not
explode instantly on join, despite being marked as WALL_EXPLODING.
PhysFS 3.0 tests whether archive members are directories, but treats a
symbolic link that points to a directory as distinct from the underlying
directory, even when symlink following is enabled. As a result, if
$D2X_REBIRTH_HOME/missions is a symlink to a directory, then it is
ignored and no missions are available. In PhysFS 2.0, the link was
followed and the missions were available.
Add a trailing dot to the path, which already has a trailing slash, to
force PhysFS to consider the underlying directory, not the link.
In one path, load_briefing_screen is called with `br->background_name`
as an input. This caused a call to `snprintf(a, sizeof(a), "%s", a);`,
which is undefined behavior. Switch back to the prior style of
unconditionally declaring a local array, performing filename
manipulation in that array, and copying the array back to
`br->background_name` afterward.
Reported-by: zicodxx <https://github.com/dxx-rebirth/dxx-rebirth/issues/420>
Remove use of object_guidebot_cannot_reach and instead track the
reachability of the object as a separate flag. This allows the game to
remember when an object was found, but unreachable. Previously, it
would store only that some unreachable object was found. Now, it stores
the index of the unreachable object. This extra information is not used
yet.
Reported-by: Dainslaif <https://github.com/dxx-rebirth/dxx-rebirth/issues/421>
Fixes: c3cead4319 ("Move Escort_goal_index to d_unique_buddy_state")
`blast_nearby_glass` calls `check_effect_blowup`, but ignores the result.
If `check_effect_blowup` executes a blow-up effect, and the blown object
is a switch, the return value will instruct the caller to execute the
associated trigger. Since `blast_nearby_glass` ignores the return
value, it never executes a trigger. This was originally masked by a bug
that passed undefined data from `blast_nearby_glass` to
`check_effect_blowup`, which usually confused `check_effect_blowup` into
treating the explosion as not originating from the player. This
confusion then forced an early return when a switch was hit, effectively
accidentally making switches invulnerable to `blast_nearby_glass`.
Commit f6352e7 fixed the data confusion, allowing `check_effect_blowup`
to recognize that the explosion came from a player's weapon. However,
it did not add handling for the trigger, so now `blast_nearby_glass` can
destroy the switch without activating the trigger. This is at the least
annoying, and in some levels may make progress impossible. There are
two ways to resolve this:
- Fix the logic to always execute the switch when the switch is
destroyed, whether by direct hit or by `blast_nearby_glass`.
- Restore the quirk that switches are invulnerable to
`blast_nearby_glass`.
Conveniently, the `force_blowup_flag` is set by `blast_nearby_glass` and
clear for all other callers, so it can be used to recognize that the
caller will not handle the switch. Use this to implement choice 2.
Even with this change, a direct hit by a missile can still destroy a
switch and activate its trigger, since that is handled through
`collide_weapon_and_wall`. Thus, players can still use the technique of
using a guided missile to activate an otherwise impossible switch.
Fixes: f6352e7957 ("Pass correct object to check_effect_blowup")
Reported-by: wm4 <https://github.com/dxx-rebirth/dxx-rebirth/issues/419>
gcc-9 warns on taking the address of an unaligned member in a packed
structure. This structure does not need to be packed. Remove
__attribute__((packed)) and fix the code to implement I/O correctly
without packing.
Utility xrange, inspired by the Python2 feature of the same name,
provides an object that returns successive values from [start, end). It
is useful when the end index is known in advance, and is particularly
helpful when that index is expensive to recompute.
Descent 1 mangles colors during `g3_init_polygon_model`, so this must
not be called on polygons not designed for mangling. Rearrange the
logic to allow Descent 1 to verify that polygon models are well-formed
without using the functions that mangle the colors.
Fixes: 42a2e3ab0b ("Avoid crash loading polymodels with invalid subcalls")
Reported-by: derhass <https://github.com/dxx-rebirth/dxx-rebirth/issues/416>
Windows std::ptrdiff_t is `int` instead of `long` as it should be.
Expand the values out to `long` (which is the same size as `int` on
Win32!) before printing them. This fixes format string warnings.
Reported-by: Ninjared <https://forum.dxx-rebirth.com/showthread.php?tid=857&pid=12555#pid12555>
Fixes: 42a2e3ab0b ("Avoid crash loading polymodels with invalid subcalls")
The highest-level tracking code assumed filenames would always fit in a
char[9]. This was true on DOS, but has not been true in Rebirth for
many years. Builds without fortification caused silent memory
corruption in this case.
Refuse to create highest-level entries if they would cause corruption.
Log a diagnostic telling the user that this happened.
Fix original bug that buddy was limited to 5 hints per program run, not
5 per boss as it probably should have been.
Due to savegame format limitations, this still is not right. Reloading
the game should restore Buddy_gave_hint_count to its value at save time,
but will not.
Bitmaps based on grs_main_bitmap own their data. Bitmaps based on
grs_bitmap do not. Adjust prototypes to prevent initializing a
grs_main_bitmap with data it will not own.
Release builds should never have an object in CT_SLEW state. If they
do, they abort the program. This is an extreme reaction. Change the
logic to report the problem, then coerce the type to CT_NONE and try to
resume operation.
Processing sharepath in SConf is incorrect, because targets can share a
build directory (and therefore an SConf run), but not share a sharepath.
Move sharepath handling out of SConf. Move DXX_USE_SHAREPATH handling
from CGameArg to GameArg, since one game can be built with a sharepath
while the other is built without.
clang warns for using an uninitialized array during the
member-initialization-list, before the union constructor would have done
nothing. clang permits using the still-uninitialized array in the
constructor body, after the union constructor has done nothing.
The same code is generated both before and after this commit, but the
old code produces a warning and the new code is silent.
```
similar/main/physics.cpp:282:5: error: field 'a' is uninitialized when used here [-Werror,-Wuninitialized]
e(a.begin())
```
gcc-4.9 rejects defining a class with a reference member and no
constructor, even though the use of the class uses a
brace-initialization expression to set the reference at construction
time.
Fixes: fa654324ad ("Pass context to wall.cpp:cwframe, cw_removal_predicate")
PHYSFS_init is not guaranteed to succeed. Using PHYSFS functions after
PHYSFS_init fails is likely to fail badly. On Windows, failure may take
the form of a crash in ntdll. Avoid this by exiting gracefully.
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.
`check_effect_blowup` should receive the `laser_info` of the weapon that
caused the blast. Previously, it was given the `laser_info` of the
parent of that weapon. The parent was not of type `OBJ_WEAPON`, so
passing its `laser_info` is meaningless.
Fixes: 9bd1ba7c47
clang warns that this interferes with copy elision. The generated code
is slightly worse after following clang's advice and removing this, but
this is not a hot path, so take the hit to silence clang instead of
complicating the source.
Fixes: d97afc2ad5 ("Retain directory structure in New Game dialog")
gcc-4.9 std::array::size() is `constexpr`, but gcc-4.9 refuses to
initialize a local `constexpr std::size_t` from the result of calling
`size()` on a `std::array`. Later gcc permit this. gcc-8 generates the
same code whether the variable is `constexpr std::size_t` or `const
std::size_t`, and the latter allows gcc-4.9 to build, so remove
`constexpr` and use plain `const`.
Reported-by: joolswills <https://github.com/dxx-rebirth/dxx-rebirth/issues/411>
Fixes: 91d6285751 ("Factor out shortening game/mission names")
gcc-4.9 shipped without support for std::is_trivially_move_assignable.
This is only needed in a sanity check, so preprocess it out when using a
gcc below gcc-5.
Reported-by: joolswills <https://github.com/dxx-rebirth/dxx-rebirth/issues/411>
Fixes: 57334255ac ("Simplify stuck object cleanup")
gcc permits this, but clang complains. Match the format specifier to
the type of the underlying temporary variable.
Fixes: a65068fed1 ("Move OGL RLE bitmap buffer to stack")
Add experimental support for using ADL MIDI instead of SDL for music
playback. Support for ADL MIDI contributed by Github user jpcima. This
feature is minimally supported by the core Rebirth team, but is included
as a courtesy so that users need not patch in support separately.
Suggested-by: jpcima <https://github.com/dxx-rebirth/dxx-rebirth/pull/408>
Add configuration file entries for number of chips, bank index, and
whether to use ADL MIDI. Currently, there is no GUI for this.
Interested users must enable it via direct configuration file editing.
A menu interface will come later.
Shrink it from 1MB to 300KB. Add a diagnostic if the expansion fails
due to insufficient capacity.
300KB is required for the ship cockpit bitmap. Everything else seems to
be smaller.
For each link given as http://, verify that the site is accessible over
https:// and, if so, switch to it. These domains were converted:
* llvm.org
* clang.llvm.org
* en.cppreference.com
* www.dxx-rebirth.com
* www.libsdl.org
* www.scons.org
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.
Commit a833d73d44 added the SDL_mixer backend for Rebirth sound. That
commit set the length of the converted sound equal to the size of the
buffer that SDL requested as a work area. No one ever touched that
logic, until now. In SDL1, that choice worked fine. In SDL2, this
causes garbage to play after the sound, because SDL2 considers the
buffer to be defined only up to the length returned in
`SDL_AudioCVT::cvt_len`. Bytes beyond that length are undefined[1], and
in practice contain garbage. Fix the noise by setting the sound length
equal to the length returned by SDL2, so that the undefined bytes are
not treated as sound. SDL1 also maintains this length value, so no
version-specific logic is required.
[1]: https://wiki.libsdl.org/SDL_ConvertAudio
Reported-by: heftig <https://github.com/dxx-rebirth/dxx-rebirth/issues/396>
This commit enables Rebirth to build with SDL2, but the result is not
perfect.
- SDL2 removed some sticky key support. Rebirth may behave differently
now in this area.
- SDL2 removed some key-repeat related support. Rebirth may behave
differently now in this area.
- SDL2 gained the ability to make a window fullscreen by sizing it to
the desktop instead of by changing the desktop resolution. Rebirth
uses this, and it mostly works.
- Resizing while in the automap does not notify the automap code, so
the view is wrong until the player switches out of automap mode and
back in.
- SDL2 changed how to enumerate available resolutions. Since
fitting the window to the desktop is generally more useful than
fitting the desktop to the window, I chose to drop support for
enumerating resolutions instead of porting to the new API. Users can
now enter an arbitrary window dimension and Rebirth will make an
attempt to use it.
- It might be useful to cap the window dimension at the desktop
dimension, but that is not done yet.
- Entering fullscreen mode through the Controls->Graphics submenu
failed to notify the relevant subsystems, causing the rendered
content not to rescale. For now, compile out the option to toggle
full screen through that menu. Toggling through Alt+Enter works
properly.
Despite these quirks, this is a substantial improvement over the prior
commit, where SDL2 cannot be used at all. The remaining issues can be
resolved in future work.
References: <https://github.com/dxx-rebirth/dxx-rebirth/issues/82>
User Nemesis reported an unspecified crash, but never responded to
inquiries to provide more data. Users Tourmeister and Buff Skeleton
reported a fatal exception thrown while paging in a vclip, with initial
triggering conditions matching the report from Nemesis. Both
Tourmeister and Buff Skeleton are using v1.0 Descent 2 data. No one
with more recent data has reported a problem.
Add a try { ... } catch { ... } block to trap the partial_range
exception, log it, and then ignore paging in that vclip. This should
approximate what 0.58.1 did, assuming that the crash is due to a
negative `num_frames`. If the crash is due to a large positive
`num_frames`, this change will still prevent the previously reported
crash, but may only move the crash elsewhere. Various other subsystems
react badly to using a vclip that has not been paged in.
This change is tested not to break an otherwise working game. It is not
tested to confirm that it produces a working game for users who
experienced a crash without it.
Reported-by: Tourmeister <https://forum.dxx-rebirth.com/showthread.php?tid=943&pid=12159#pid12159>
Reported-by: Buff Skeleton <https://forum.dxx-rebirth.com/showthread.php?tid=943&pid=12290#pid12290>
References: <https://github.com/dxx-rebirth/dxx-rebirth/issues/371>
Currently, DXX_USE_SDL_REDBOOK_AUDIO is an alias for !SDL2. However,
this patch enables interested users to readily patch out Redbook even
for SDL1.
References: <https://github.com/dxx-rebirth/dxx-rebirth/issues/82>
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")
User jcotton42 suggested copying a D2X-XL feature: preserving the
directory structure of the user's missions area when showing a New Game
dialog. This was substantially more trouble than it should have been,
but the result is good.
Previously, the dialog presented all missions at any depth below the
starting point, and sorted them as if they were all in the root
directory.
Now:
- Empty directories are hidden entirely. There is nothing for the user
to do in them, so there is no point showing them.
- A directory with exactly one entry has that entry promoted into the
parent, since there is no ambiguity about what the user would want.
If the parent in turn has only that one promoted element when the scan
of the parent finishes, then the element can be promoted up again.
This continues until the root is reached or until a level has more
than one entry. For this purpose, both missions and directories count
as entries.
- Directory entries are decorated to inform the user how many
immediate subdirectories are present, how many missions are present
immediately in the directory, and how many missions total are present,
counting all subdirectories. If there are zero immediate
subdirectories, then the directory count is not shown. For this
purpose, directories that were hidden due to a lack of missions are
not counted.
- Sub-dialog boxes for inner directories use a title that reminds the
user of the path so far, and recaps the directory/mission statistics.
- On entry to the New Game dialog, if the last played mission is in a
sub-dialog, appropriate sub-dialogs are opened so that the last played
mission can be pre-selected.
Currently, there is no in-game override to return to the prior rollup
rules.
Requested-by: jcotton42 <https://github.com/dxx-rebirth/dxx-rebirth/issues/392>
Even when empty and default-constructible, const members must be
explicitly constructed. Add a default constructor to do this.
Fixes: 893e8cde06 ("Combine hud gauge parameters")
