If a menu was all text except for the last element, the menu would
incorrectly be classifed as all text, and citem would be pinned to the
first element.
Fixes: 14a9657136 ("Use partial_range to store newmenu_item pointer/length")
The index and the value were transposed, and since both were integer
types, the type system was unable to warn that the new code was
incorrect. Swap the variables to bind as intended.
Reported-by: Donkyhotay <https://github.com/dxx-rebirth/dxx-rebirth/issues/544>
Fixes: de4efc4f46 ("Qualify more uses of shared_segment members")
Support for ORTHO_VIEWS has been broken since commit
db514a5ded in December 2004.
`git blame HEAD -L872,+4 -- similar/editor/meddraw.cpp`:
```
db514a5ded main/editor/meddraw.c (Bradley Bell 2004-12-19 13:54:27 +0000 872) }
db514a5ded main/editor/meddraw.c (Bradley Bell 2004-12-19 13:54:27 +0000 873) } else
db514a5ded main/editor/meddraw.c (Bradley Bell 2004-12-19 13:54:27 +0000 874) #if ORTHO_VIEWS
db514a5ded main/editor/meddraw.c (Bradley Bell 2004-12-19 13:54:27 +0000 875) else if ( screen_canvas == TopViewBox->canvas )
```
When ORTHO_VIEWS is true, this code expands to `} else else if (...)`; a
double-else is not legal, so the code cannot have been compiled with
ORTHO_VIEWS enabled. All 4 of the relevant lines date to db514a5ded,
which itself is an import of files from the D1X editor.
Rebirth built with OpenGL has a hard dependency on the GL utility
library, but did not have a configure time test to report this. Add
one.
Reported-by: shoober420 <https://github.com/dxx-rebirth/dxx-rebirth/issues/468>
When joystick support is not present, `event_joystick_get_button` is not
declared in joy.h.
Fixes: c24864b180 ("make menus controllable with joystick")
The same logic is used in two places. Move it to a helper function, and
switch from repeated use of snprintf for one character at a time to
instead use fill_n.
Check if the cockpit window textures are still valid when reusing the
previous cockpit data in cockpit_decode_alpha.
This fixes a crash when leaving the graphics options menu while in
the game with cockpit visible, since that calls
ogl_smash_texture_list_internal which invalidates all textures.
For historical reasons, the kcm_* arrays are smaller than the player's
configuration settings. Move the kcm_ arrays to be the first range
passed to zip, so that traversal does not exceed the array.
Fixes: d959eeca8f ("Use zip in kconfig to allow use of range-based for")
When AddressSanitizer is enabled, gcc-10 will warn that strncpy will
truncate the message. This truncation is valid, since it only loses the
terminating null byte, and the next line explicitly supplies a
terminating null. Switch to memcpy to eliminate the warning and produce
simpler code.
Define separate enum values for rotation data in both the high bits,
where it is usually kept, and the low bits, where it is sometimes used
for math or comparisons.
Define an enum value to represent the composite of the index and the
rotation, since the composite is not suitable for use as an array
subscript. Add helper functions to extract the component pieces.
tmap1 cannot be rotated, so checking for it inside
convert_to_d1_tmap_num is wrong. tmap2 can be rotated, and checking for
it outside convert_to_d1_tmap_num is cleaner.
Add a comment in the #include directive to hide it from the PCH scanner,
since this file should not be included in the PCH. It is only used in
one file, and it is included under special circumstances. In
particular, it is included while inside a namespace.
segment inherits unique_segment, so unique_segment members can be
referenced from an instance of segment. Eventually, segment will be
removed. Adjust references to go through unique_segment to support this
change.
segment inherits unique_segment, so unique_segment members can be
referenced from an instance of segment. Eventually, segment will be
removed. Adjust references to go through unique_segment to support this
change.
segment inherits unique_segment, so unique_segment members can be
referenced from an instance of segment. Eventually, segment will be
removed. Adjust references to go through unique_segment to support this
change.
segment inherits unique_segment, so unique_segment members can be
referenced from an instance of segment. Eventually, segment will be
removed. Adjust references to go through unique_segment to support this
change.
gcc and clang disagree about how to disambiguate when an identifier is
both a typename and a member. Avoid the disagreement by renaming the
member.
Reported-by: Kreeblah <https://github.com/dxx-rebirth/dxx-rebirth/issues/532>
This was a leftover from an earlier prototype. gcc-7 warns about it,
but gcc-8 and later silently allow it. Remove it to fix the gcc-7
build, since it is not needed.
Reported-by: Kieltux <https://github.com/dxx-rebirth/dxx-rebirth/issues/531>
Fixes: ec6a78c481 ("Use enum class for marker index types")
Various sites assign GM_GAME_OVER to Game_mode, but no sites test for
it. Its only value would be to clear out other flags, but that can be
done by clearing Game_mode.
In the original code there was a 'Hack to keep sounds from building up...'
which changed a non-forever linked sound to a non-linked sound. Therefore
the forever flag wasn't needed in the demo files. The commit
4a98e796ab removed the hack, which caused
non-forever sounds to start looping on demo playback.
This commit restores the hack as far as demo files are concerned.
Reported-by: zicodxx <https://github.com/dxx-rebirth/dxx-rebirth/issues/477>
Fixes: 4a98e796ab ("Prevent stacking weapon rotation sounds")
Descent tracked hoard orbs by borrowing the player's proximity bomb
slot. Commit 829e95b6f8 moved proximity
bomb tracking to its own slot, but failed to update the player
death/deres logic accordingly. This caused multiple inconsistencies
when a player was killed in hoard mode:
- The killed player saw the orb drop as expected.
- The killed player _also_ kept the orb in inventory after respawn,
because the counter was not reset.
- Other players saw no orb drop.
Fix the inappropriate retention by resetting the orb count in
init_player_stats_new_ship. Fix the inappropriate failure to drop by
adding a new unconditional field to the player death/deres message. In
hoard games, use it to pass the orb count. In other games, ignore it.
Fixes: 829e95b6f8 ("Separate hoard/proximity tracking")
Reported-by: snytek <https://github.com/dxx-rebirth/dxx-rebirth/issues/526>
The keys are always enabled, so there is no need to show them here.
Non-fullscreen users already dedicate space to the key graphic, so
nothing can be saved by adding a special case for those viewing modes.
Instead of a generic "-no title-", insert the time of the save. This
conveys some information to users, who may find it helpful for
distinguishing individual saves.
Save menus have a non-selectable first element used to allocate extra
space for the save game's embedded screenshot. The memorized save game
slot failed to account for this, causing the game to suggest a slot one
less than the last used slot. Attentive users could override the
suggestion, but the game should suggest the correct slot automatically.
Reported-by: ziplantil <https://github.com/dxx-rebirth/dxx-rebirth/issues/524>
Gauges aligned to the left side of the screen try to move up in
multiplayer, and move farther in large multiplayer games. However, the
logic for this was inconsistent, and could draw shields and afterburner
state on the same line. Fix this by computing the starting point once,
and passing it down to the functions that need it.
Reported-by: snytek <https://github.com/dxx-rebirth/dxx-rebirth/issues/521>
Testing for buttons||hats||axes is equivalent to testing for
joysticks!=0, since SConstruct forces joysticks=0 when
buttons == 0 && hats == 0 && axes == 0. Remove tests of
buttons||hats||axes when those occur inside a test for joysticks!=0.
This block previously was used to clear TRIGGER_ON, but that change was
removed because TRIGGER_ON was a write-only flag. This block is now
useless, so remove it. This should also fix an obscure crash reported
by AlumiuN.
AlumiuN proposed adding a test that seg->children[side] is still valid,
but since the logic to execute would be useless even when the test
succeeds, it is simpler to remove this code entirely.
Reported-by: AlumiuN <https://github.com/dxx-rebirth/dxx-rebirth/issues/515>
Suggested-by: AlumiuN <https://github.com/dxx-rebirth/dxx-rebirth/pull/518#issue-443277182>
This reverts commit 60edaa7dce.
This commit was brought in through a merge to obtain a different change,
and was supposed to be suppressed in preference to the change from
dfcb0fcdc. Revert it here, since reloading the variable suffices to fix
the problem, and this commit forces a reload on all platforms, rather
than only reloading on platforms that need it.
This ensures that an array is indexed by the appropriate type. Indexing
a mouse array with a joystick index is undefined behavior, and with this
change, such undefined behavior is now a compilation error.
PHYSFSX_fsize opens the file, reads its length, and closes it. Calling
PHYSFSX_fsize in a loop is wasteful, since the file size should not
change while the game is playing.
This makes the alignment data structure `chunk` private to interp.cpp,
and allows most files to ignore whether DXX_WORDS_NEED_ALIGNMENT is
enabled, thus improving the ccache hit rate.
kajott reports that on Raspberry Pi systems, "Medium Hulk and Heavy Hulk
models were missing their heads". This impacts all systems with
words_need_alignment=1, and was introduced by logic that tried to
truncate invalid polygon models, which was added because such models
often crashed the game. The validation logic used the original size of
the polygon model data, which is correct on systems with
words_need_alignment=0, but can be too small on systems with
words_need_alignment=1 if the polygon model was enlarged as part of
aligning it. Such systems would refuse to read past the original,
shorter, length, and then declare the model to be broken and truncate
it. Fix this by reloading the length from the polymodel structure after
the alignment logic is applied.
Reported-by: kajott <https://github.com/dxx-rebirth/dxx-rebirth/pull/519>
Fixes: 42a2e3ab0b ("Avoid crash loading polymodels with invalid subcalls")
lengthof was added to provide the size of C arrays. C++17 std::size is
now available, and can serve the same purpose. Remove the custom
lengthof.
The generated code is the same in both cases, other than the change in
line numbers caused by removal of the #include directives.
Use an enum class to prevent implicit conversion between trigger
behavior flags and other integers. Fix up various resulting breaks,
which look like bugs:
- Descent 2 editor mode could modify trigger::flags, but used
TRIGGER_FLAG_* values, which specify the actions for a Descent 1
trigger when it executes, not the behavior properties for a trigger.
- Adding a trigger set its flags to 0, then cleared all flags except
TRIGGER_ON. Since the flags were just set to 0, the mask operation is
useless. Remove it.
- trigger_turn_all_ON cleared all flags except TRIGGER_ON. This seems
to be completely wrong. Change it to remove
trigger_behavior_flags::disabled. Descent 1 has no (working) support
for disabling triggers, so make trigger_turn_all_ON exclusive to
Descent 2.
- wall_restore_all would enable TRIGGER_ON in both games. Descent 1
never reads TRIGGER_ON. Descent 2 uses this field for trigger
behavior flags, and TRIGGER_ON is not a behavior flag.
- For Descent 1 builds, remove the modification of the field.
- For Descent 2 builds, change it to clear
trigger_behavior_flags::disabled.
Normally, this shouldn't matter, and on almost all
OpenGL drivers out there, it really doesn't, but
the Raspberry Pi 4's "V3D" Mesa driver behaves
differently: It simply won't draw any lines as long
as GL_CULL_FACE is enabled, so we need to disable
it.
If compiled with words_need_alignment=1, the
model data was correctly converted and expanded,
but the old size was still used for validating
and initializing the model.
This caused funky effects on affected platforms,
e.g. some robots missing their heads on Raspberry Pi.
It's possible to enter empty savegame descriptions,
which is no problem itself, but it looks quite
confusing in the save/load menus: you can't see
the currently selected item, for instance.
This commit replaces an empty description with a
placeholder text ("no title") upon saving.
This is particularly useful when controlling
the game only with a joystick, where entering
a proper description isn't possible.
A new configurable joystick button binding is
introduced that opens the ingame menu, i.e. the
menu that appears when pressing the Escape key.
Fortunately, there was still enough space left
in the button binding maps for both games. The
configuration screen, however, needed to be
extended by a row to make room for the new
binding on d2x.
One known issue: When using an old player
configuration file, the menu button bindings
are initially set to J1 B1, because the unused
slots in DefaultKeySettings used to be
initialized that way. There's not much we can
do about that; the player needs to remove
these bogus assignments by hand. We can (and
do) prevent that from happening again, though,
by initializing new player files with correct
unmapped buttons.
In most menus, keyboard commands are synthesized
from controller buttons, leveraging the existing
axis-to-button translation.
Menu controls are currently fixed:
- button 0 (A) is confirm (Enter)
- button 1 (B) is cancel (Esc)
- button 2 (X) is switch (Space)
- button 3 (Y) is delete (Delete)
- axes 0 and 1 (main analog pad) maps to cursor keys
- all hats (D-pads) map to cursor keys
Title screens and credits can be confirmed
with any joystick button or axis motion too.
Similar to 09eff19a66, _g3_draw_poly can
store the control information on the stack to avoid an allocation, since
the total size is bounded and relatively small.
Similar to 09eff19a66, _g3_draw_tmap_2 can
store the control information on the stack to avoid an allocation, since
the total size is bounded and relatively small.
This was once a compatibility shim, but compiler support for std::begin
has been required since 5e434cbe95 and no
issues have been reported. Flatten the include tree by removing this
header and using the STL names directly.
A declaration is useful if it declares an external function. A static
inline declaration, if not followed by the definition later in the same
translation unit, will not be useful. Remove such declarations, and
rely on using the definition as a declaration for those files that
actually call partial_range() or similar functions.
This fixes PCX loading in the non-editor build. PCX loading broke in
3114874713 because it switched from PHYSFSX_openReadBuffered to a call
which did not handle the presence of the marker byte in the filename.
The from_hog_only flag has not been properly supported since the
introduction of PHYSFSX_openReadBuffered in
8b323e7214. Editor builds ignored it on
purpose, and non-editor builds pretended to respect it, only to then
ignore it down inside PHYSFSX_openReadBuffered. For consistency with
code paths that do not use PHYSFSX_openReadBuffered, disable the bypass
in PHYSFSX_openReadBuffered and remove the code in show_title_screen
that applied the flag.
Reported-by: Jayman2000 <https://github.com/dxx-rebirth/dxx-rebirth/issues/510>
Fixes: 3114874713 ("Delegate PCX loading to SDL_image")
The automap code keeps a mostly-private secondary copy of control_info.
The change to reset control_info before the event loop reset the main
copy of control_info for the automap, not the copy that the automap
actually used. Fix this by passing the desired control_info to relevant
functions instead of always using the global variable Controls.
Fixes: c621a970c6 ("Reset kconfig counters once, before the event loop")
This field is used in the SDL build for rendering. In the GL build, its
only purpose is for the editor to write it to a PIG file. Change that
one use to compute the value as needed. Move all other references
behind a preprocessor test for !DXX_USE_OGL. This shrinks the size of
grs_bitmap by 1 pointer, since the compiler added padding after
avg_color up to the size of the next aligned pointer.
This adds a new dependency, but most systems likely already have
SDL_image installed. Use of SDL_image can be disabled, but this is
discouraged, because various in-game interfaces assume the use of the
original background.
The old implementation automatically corrected for filename case. The
new implementation expects that the supplied filename can be passed to
PYHSFS_openRead as-is. All known uses in-game have been corrected to
satisfy this requirement. If the new stricter match requirement becomes
a problem, a variant of PHYSFSRWOPS_openRead that adjusts filename case
could be created for use here.
- Update install instructions
- Update ebuild
- Update Arch PKGBUILD
gcc-10 warns that the game information text may be truncated, because it
overestimates the maximum possible length of some strings. Handle the
warning by changing the format string to explicitly truncate these
strings at their maximum legal lengths.
gcc-10 warns that if immediate_directories were very large, the sprintf
result would be truncated. This is true, but a user is extremely
unlikely to have enough directories to trigger this code path. Handle
the warning by explicitly returning an empty string if truncation would
otherwise occur. This is not strictly correct, but it can be handled
with very little code, eliminates the warning, and is unlikely to matter
in practice.
As reported in pull #507, the D1 implementation of the reduction was
incorrect and never reduced laser power. Therefore, when D2 emulates
D1, D2 should emulate the effect of the bug, by not reducing laser
power.
The code that provides the 0.75x multiplier to laser bolts when they're fired
using the quad laser is present in D1 as in D2, however it is incorrect and
results in it not being applied. As such, it is more accurate to exclude the
multiplier when compiling D1.
Thanks to SaladBadger for the tip.
Commit 2bcc7bb371 restricted
Laser_create_new to only create lasers for known weapon types. This
seemed like a good idea, but broke custom levels that use undocumented
laser types. It also replaced logic that coerced invalid weapon numbers
into a type 1 laser with logic that prevented firing such weapons at
all. This rendered robots with unknown or broken weapon data unable to
fire.
Commit 0c30fa7cf3 relaxed the logic to
prevent firing only in editor builds, but allow firing unknown or
invalid weapon types in release builds. Firing unknown weapon types may
work. Firing invalid weapon types may cause the game to crash when data
is accessed beyond the end of the defined weapon data. This caused the
crash reported in issue #506.
Revert to the 0.58.1 rule that invalid weapons are coerced to be a
player level 1 laser. Add a diagnostic when such a weapon is fired,
since the level author should have requested that weapon explicitly if
that was intended.
Reported-by: heftig <https://github.com/dxx-rebirth/dxx-rebirth/issues/506>
Fixes: 2bcc7bb371 ("Only create lasers for known weapon types")
newpath is a std::array and does not decay to char* automatically.
Use data() method to get char[], which does decay to char*.
This compile-time issue only affected Mac OS X.
The minimum supported compiler versions now provide a depth-efficient
implementation of std::make_index_sequence, which removes the last
reason to carry a private implementation. In the case of clang, it
appears to have a special compiler intrinsic used to implement its
std::make_index_sequence.
Switch to the compiler-provided version for both gcc and clang.
gcc-4.9 support is now difficult to test due to system libraries linking
to newer symbols. gcc-4.9 is unsupported upstream, as are gcc-5 and
gcc-6. Raise the minimum required gcc version to the minimum version
supported upstream.
Debian Jessie shipped gcc-4.9.2, and support for this target was the
primary motivator for retaining gcc-4.9 support. Jessie ended regular
support in June 2018, and will end Long Term Support in June 2020. It
seems unlikely that Jessie would receive a snapshot build of Rebirth in
the months it has left.
Debian Stretch shipped gcc-6, but is currently considered "oldstable"
and has been superseded by Debian Buster. Further, Debian Stretch
provides a package for gcc-7, so Stretch users can still build Rebirth
using only packages available from the package manager.
Commit d355ef4030 removed a seemingly unnecessary modification of the
global variable grd_curcanv->cv_font, after eliminating all local reads
of it. However, a non-local read, buried in listbox_mouse, depended on
grd_curcanv->cv_font being set to a MEDIUM font. After that commit,
grd_curcanv->cv_font retained its prior value, which is not a MEDIUM
font. This caused listbox_mouse to compute an incorrect height of the
lines in the listbox, which manifested as the game choosing the wrong
line when the mouse is clicked in the listbox.
Fix the problem by explicitly using MEDIUM3_FONT, since that was
usually the value left in grd_curcanv->cv_font prior to that commit. In
some cases, a different MEDIUM font would be left there, but all the
MEDIUM fonts have the same height, so they are interchangeable for this
purpose.
Reported-by: Q3BFG10K <https://github.com/dxx-rebirth/dxx-rebirth/issues/498>
Fixes: d355ef4030 ("Pass font to various drawing functions")
andrew-strong reported that using SDL2, with the window set to full
screen, but a windowed size less than full screen, caused letterbox
sequences, such as the player ship destroyed sequence, to render in a
subwindow sized to the dimensions game would have when unmaximized, even
if the game window is maximized at the time of the sequence.
tycho suggested a change that resolves this issue, and basic testing
showed no unwanted side effects.
Reported-by: andrew-strong <https://github.com/dxx-rebirth/dxx-rebirth/issues/399>
Suggested-by: tycho <https://github.com/dxx-rebirth/dxx-rebirth/issues/399#issuecomment-583688998>
ziplantil reported that in Descent 1, if a player has an empty Vulcan
cannon, and no energy, then attempting to fire a weapon would play the
SOUND_ALREADY_SELECTED clip every frame until the player ceased trying
to fire. The problem does not impact Descent 2 since Descent 2 does not
play this sound sample when trying to activate a weapon that is already
active.
If the current weapon is already LASER_INDEX, then
select_primary_weapon(LASER_INDEX) simplifies to:
```
// Pointless, player already using this weapon
newdemo_record_player_weapon(LASER_INDEX)
if (Primary_weapon != LASER_INDEX) {
// skipped, path is false
}
else
{
if (wait_for_rearm) // true for this path
// Bad, plays every frame
digi_play_sample(SOUND_ALREADY_SELECTED);
}
// Pointless, Primary_weapon already is LASER_INDEX
Primary_weapon = LASER_INDEX;
if (weapon_name)
{
// skipped, path is false
}
```
Skipping the call avoids two pointless statements, one bad statement,
and nothing useful. Therefore, the simplest fix for the problem is to
call select_primary_weapon(LASER_INDEX) only if the primary weapon is
not currently LASER_INDEX.
Reported-by: ziplantil <https://github.com/dxx-rebirth/dxx-rebirth/issues/499>
Read the entire buffer at once, then remove the carriage returns and set
a null terminator. This reduces the number of calls to PHYSFS_read from
len to 1.