Rename symbol WORDS_BIGENDIAN to DXX_WORDS_BIGENDIAN to show that it is a DXX
symbol, not one inherited from a library.
This is a mostly automated transform, but the changes to SConstruct were
manual.
git grep -wl WORDS_BIGENDIAN -- '*.h' '*.cpp' | xargs sed -i -e 's/^\s*#ifdef \(WORDS_BIGENDIAN\)\>/#if DXX_\1/' -e 's/\s*#\(el\)\?if \(.*\)defined(\(WORDS_BIGENDIAN\))/#\1if \2DXX_\3/' -e 's/^\s*#ifndef \(WORDS_BIGENDIAN\)\>/#if !DXX_\1/'
The *_specific_type structures have a static member `nm_type`. In -O0
builds, an out-of-line definition of this member is required, but was
not present. In -O2 builds, the optimizer happened to remove all
references to the out-of-line definition, so the link succeeded despite
not having a definition. Add that definition.
Save unique_ptr's to UI_GADGET_BUTTON's in similar/editor/med.cpp and similar/editor/medwall.cpp. Fixes compile errors and resolves a crash when attempting to access both dialogs' gadgets (input event processing is the first attempt).
_g3_draw_poly uses type cg3s_point which is const in OGL and non-const
in SDL. gcc-6 with LTO reports a One Definition Rule violation for this
mismatch:
typedef const int cint;
void f(cint *);
void f(const int *) {}
The code ran correctly as it was, but the fix is trivial and has no
effect on the generated code, so fix it to satisfy the compiler.
Optimizing compilers tend to factor out color automatically. Move it up
to the caller to ensure that it is computed once even if the compiler
does not optimize aggressively.
Rename symbol MAX_HATS_PER_JOYSTICK to DXX_MAX_HATS_PER_JOYSTICK
to show that it is a DXX symbol, not one inherited from a library.
git grep -lzw MAX_HATS_PER_JOYSTICK -- SConstruct '*.h' '*.cpp' | xargs -0 sed -i -e 's/\<MAX_HATS_PER_JOYSTICK\>/DXX_&/g'
Rename symbol MAX_BUTTONS_PER_JOYSTICK to DXX_MAX_BUTTONS_PER_JOYSTICK
to show that it is a DXX symbol, not one inherited from a library.
git grep -lzw MAX_BUTTONS_PER_JOYSTICK -- SConstruct '*.h' '*.cpp' | xargs -0 sed -i -e 's/\<MAX_BUTTONS_PER_JOYSTICK\>/DXX_&/g'
Rename symbol MAX_AXES_PER_JOYSTICK to DXX_MAX_AXES_PER_JOYSTICK to show
that it is a DXX symbol, not one inherited from a library.
git grep -lzw MAX_AXES_PER_JOYSTICK -- SConstruct '*.h' '*.cpp' | xargs -0 sed -i -e 's/\<MAX_AXES_PER_JOYSTICK\>/DXX_&/g'
Rename symbol MAX_JOYSTICKS to DXX_MAX_JOYSTICKS to show that it is a DXX
symbol, not one inherited from a library.
git grep -lzw MAX_JOYSTICKS -- SConstruct '*.h' '*.cpp' | xargs -0 sed -i -e 's/\<MAX_JOYSTICKS\>/DXX_&/g'
Rename symbol OGL to DXX_USE_OGL to show that it is a DXX
symbol, not one inherited from a library. Move it to dxxsconf.h to
shorten the command line.
This is a mostly automated transform, but the changes to SConstruct were
manual.
git grep -lzw OGL -- '*.h' '*.cpp' | xargs -0 sed -i -e 's/\(\s*#\s*if\)def\s*OGL/\1 DXX_USE_OGL/' -e 's/\(\s*#\s*if\)ndef OGL/\1 !DXX_USE_OGL/' -e 's/\(\s*#\s*if !\?\)defined(OGL)/\1DXX_USE_OGL/'
Rename symbol OGLES to DXX_USE_OGLES to show that it is a DXX
symbol, not one inherited from a library. Move it to dxxsconf.h to
shorten the command line.
This is a mostly automated transform, but the changes to SConstruct were
manual.
git grep -lzw OGLES -- '*.h' '*.cpp' | xargs -0 sed -i -e 's/\(\s*#\s*if\)def\s*OGLES/\1 DXX_USE_OGLES/' -e 's/\(\s*#\s*if\)ndef OGLES/\1 !DXX_USE_OGLES/'
`std::use_facet` is provided by `<locale>`. x86_64-pc-linux-gnu happens
to provide `std::use_facet` in inferno.cpp without explicitly including
`<locale>`. kreator found that clang on OS X does not implicitly
provide `<locale>`. `<locale>` must be included to make
`std::use_facet` available on OS X. Although it worked without this on
Linux, it is the right thing to do everywhere.
Unlike many uses of index_sequence<N...>, this set of N... is not the
result of make_tree_index_sequence. Some callers pass sequences with
`Nn != n` for some n, so vp is not initialized as a copy of all elements
of ovp in the same order as ovp.
Fixes: fa5475aa54 ("Remove unnecessary copy of vertex points")
clang crashes with a segmentation fault if asked to implicitly convert
1u to std::size_t in partial_range inline chain. Add a conversion
outside the inline chain, which seems to avoid the bug.
Rename symbol EDITOR to DXX_USE_EDITOR to show that it is a DXX
symbol, not one inherited from a library. Move it to dxxsconf.h to
shorten the command line.
This is a mostly automated transform, but the changes to SConstruct were
manual.
git grep -wl EDITOR -- '*.h' '*.cpp' | xargs sed -i -e 's/^\s*#ifdef \(EDITOR\)\>/#if DXX_USE_\1/' -e 's/\s*#\(el\)\?if \(.*\)defined(\(EDITOR\))/#\1if \2DXX_USE_\3/' -e 's/^\s*#ifndef \(EDITOR\)\>/#if !DXX_USE_\1/'
Rename symbol IPv6 to DXX_USE_IPv6 to show that it is a DXX
symbol, not one inherited from a library. Move it to dxxsconf.h to
shorten the command line.
This is a mostly automated transform, but the changes to SConstruct were
manual.
git grep -wl IPv6 -- '*.h' '*.cpp' | xargs sed -i -e 's/^\s*#ifdef \(IPv6\)$/#if DXX_USE_\1/' -e 's/\s*#\(el\)\?if \(.*\)defined(\(IPv6\))/#\1if \2DXX_USE_\3/'
Rename symbol USE_UDP to DXX_USE_UDP to show that it is a DXX
symbol, not one inherited from a library. Move it to dxxsconf.h to
shorten the command line.
This is a mostly automated transform, but the changes to SConstruct were
manual.
git grep -l USE_UDP -- '*.h' '*.cpp' | xargs sed -i -e 's/^\s*#ifdef \(USE_UDP\)$/#if DXX_\1/' -e 's/\s*#\(el\)\?if \(.*\)defined(\(USE_UDP\))/#\1if \2DXX_\3/'
Rename symbol USE_TRACKER to DXX_USE_TRACKER to show that it is a DXX
symbol, not one inherited from a library. Move it to dxxsconf.h to
shorten the command line.
This is a mostly automated transform, but the changes to SConstruct were
manual.
git grep -l USE_TRACKER -- '*.h' '*.cpp' | xargs sed -i -e 's/^#ifdef \(USE_TRACKER\)$/#if DXX_\1/' -e 's/#\(el\)\?if \(.*\)defined(\(USE_TRACKER\))/#\1if \2DXX_\3/'
Rename symbol USE_SDLMIXER to DXX_USE_SDLMIXER to show that it is a DXX
symbol, not one inherited from a library. Move it to dxxsconf.h to
shorten the command line.
This is a mostly automated transform, but the changes to SConstruct and
inferno.cpp were manual.
git grep -l USE_SDLMIXER -- '*.h' '*.cpp' | xargs sed -i -e 's/^#ifdef \(USE_SDLMIXER\)$/#if DXX_\1/' -e 's/#\(el\)\?if \(.*\)defined(\(USE_SDLMIXER\))/#\1if \2DXX_\3/'
It was a convenient transition macro, but its presence was always
intended to be temporary. Expand it to ease the conversion of usage
sites that already have access to local player data through a local
variable.
In past releases, D1X-Rebirth played the title song even when the user
specified `-notitles`. Commit 29f79f0a6b
("Enable -notitles for Descent 2") fixed this. Unfortunately, despite
being called SONG_TITLE, users believe this to be main menu music, not
title screen music. This belief is reinforced by other code sites that
play the title song on returning to the main menu. Reintroduce the bug
by moving the call above the test for `-notitles`. This restores the
semantics that users expect.
It was a convenient transition macro, but its presence was always
intended to be temporary. Expand it to ease the conversion of usage
sites that already have access to local player data through a local
variable.
Descent demos do not record the time remaining on cloak/invulnerability,
so the demo system tries to fake it by forcing any cloaked player to
have a time remaining of 50%. Extend that hack to do the same for
invulnerability. The local player needs that hack. Remote players do
not need it, but the logic is simpler with it in the loop than with a
special case for just the local player.
Per comment, MK meant for this test to match the Helix cannon, but the
implementation was wrong. Primary weapon indexes cannot be compared to
weapon ID types. The two use different number spaces. Using proper
enum types for each causes the compiler to report this mistake. Fix the
test.
Define it where it is first used.
Skip adjusting damage in cases where it will not be used.
Fix obvious copy&paste error from Parallax.
if (condition)
statement; statement; statement;
is parsed as:
if (condition)
{
statement;
}
statement;
statement;
In context, all three statements were supposed to be guarded by the
condition. Add braces accordingly.
Using "type\nfunction(args)" confuses some parsing scripts. Switch to
the standard "type function(args)" style. Fix sites where keyword
"static" was used after the return type.
No caller passes a key value other than [KEY_F9, KEY_F12]. If one did,
an invalid array reference would occur. Restructure the code to return
on invalid inputs. This fixes a flow control analysis warning.
Mako88 reports that an unspecified environment fails to build
playsave.cpp. The failure is because PRIuFAST32 is undefined. In most
environments, SDL automatically includes inttypes.h, which provides
PRIuFAST32. In this unspecified environment, SDL does not include
inttypes.h. Include it explicitly to ensure the definition is
available.
As described in commit 674a921 ("Add workaround for Win32 wrong type for
size_t"), std::array on Win32 returns type std::size_t, which is
`unsigned int` instead of the `unsigned long` seen on Linux. Calling
std::min requires both arguments to have the same type. On Linux, this
required casting the result of size() to `unsigned` to match the other
input. On Win32, that cast provokes a -Wuseless-cast warning. Add a
workaround by removing the cast and instead explicitly setting the type
of std::min's arguments to `unsigned`, then relying on the compiler to
perform an implicit conversion from `unsigned long` to `unsigned int`
on Linux. The value is always small enough to fit in an `unsigned int`,
so no precision loss will occur.
Field width conversion `.*` always takes an `int`.
On Win32, casting `long` to `int` triggers a `-Wuseless-cast` warning.
Omitting the cast works correctly.
On Linux/amd64, casting `long` to `int` works correctly.
Omitting the cast triggers a `-Wformat` warning.
Add a macro that conditionally expands to `static_cast<int>` or to ``,
as necessary for the target platform.
Most 64-bit systems use `unsigned long` for `uint_fast32_t`. Some
32-bit systems use `unsigned int` for `uint_fast32_t`. To handle this,
write_netgame_profile used casts to `unsigned` and a format string of
`%u`. Switch to inttypes format macros so that the format strings are
correct without requiring a cast to handle systems where `uint_fast32_t`
is not `unsigned`.
Most 64-bit systems use `unsigned long` for `uint_fast32_t`. Some
32-bit systems use `unsigned int` for `uint_fast32_t`. To handle this,
multi_process_bigdata used casts to `unsigned long` and a format string
of `%lu`. Switch to inttypes format macros so that the format strings
are correct without requiring a cast to handle systems where
`uint_fast32_t` is not `unsigned long`.
Both Primary_weapon and Secondary_weapon should use the underlying enum
type. Primary_weapon uses its enum type now. Secondary_weapon will use
its enum type eventually. Add appropriate casts to newdemo.cpp to keep
it working when those changes are made.
The compiler may or may not recognize that the temporary T{} passed to
make_unique can be omitted. Help it by passing nothing, then explicitly
clearing the returned data as a separate step.
Mako88 reports that using `-pilot Mixed` on Windows causes crashes
during multiplayer setup, but `-pilot mixed` does not crash. This is
not reproducible on case sensitive filesystems, probably because case
sensitive systems recognize that `Mixed.plr` and `mixed.plr` are not the
same file. In both cases, the underlying plr file name was all
lowercase. Coerce the pilot name to lowercase when it is converted to a
plr file name. This also fixes a minor bug where the check for a
user-specified extension of `.plr` would recognize `-pilot alice.plr`,
but not recognize `-pilot alice.PLR`.
This is a workaround for a bug in the network code, which becomes
confused and crashes when the player's callsign contains mixed case.
Reported-by: Mako88 <https://github.com/dxx-rebirth/dxx-rebirth/issues/188>
Callers are expected to filter out invalid numbers. Even if they do
not, the only use of the number is to match entries in Stuck_objects.
An invalid match could cause a call to vobjptr() with an invalid index,
but that would be diagnosed by the valptridx checking, so no invalid
memory access will occur.
Many callers of kill_stuck_objects call it twice, once for each of two
walls. Move the flush_fcd_cache up to occur only once, rather than once
per wall.
After fixing the bogus lifeleft assignment, the D2 version is equivalent
to a more thorough version of the D1 version. Delete the D1 version and
use the D2 version for both games.
This assignment looks bogus. If the signature saved when the object
became stuck is different from the signature of the object currently
using that slot, then remove_obsolete_stuck_objects kills the object
using that slot. Signatures are defined to change when the object slot
is freed and reused. Therefore, this statement kills the new object
that took the slot of the obsolete object. This new object may be
important, such as a spawned robot or a weapon.
GCC std::array uses std::size_t for its size_type. On Linux/amd64,
`std::size_t` is `unsigned long`. On Win32, `std::size_t` is
`unsigned int`. This provokes format string warnings because an
unsigned int is passed where the format string declares an unsigned
long. On Win32, `unsigned int` and `unsigned long` are the same size,
so `unsigned long` could have been used to avoid this problem by
maintaining consistency with Linux, but it was not. This may have been
an attempt to achieve bug compatibility with Microsoft's types.
Add a workaround by defining a macro DXX_PRI_size_type in the style of
inttypes.h PRI* macros. Use an SConf test to determine the correct
value by inspecting which compilation runs succeed. Currently, Linux
needs "l", Win32 needs "", and Win64 needs "I64".
Previously, if the player was invulnerable, it was impossible to grab an
invulnerability powerup. Relax this by allowing a spawn-induced fake
invulnerability not to count as invulnerable for this test. Players
still cannot grab an invulnerability powerup if they are still under the
influence of a prior invulnerability powerup.
An optimizing compiler should inline these tests. Moving them out makes
the code easier to read, since the statements can focus on what is
updated, rather than how it is done.
Change the clearing of MultiLevelInv during initialization to reduce
wasted stores. MultiLevelInv has three fields.
Field ::Initial is not accessed by MultiLevelInv_CountLevelPowerups, and
is overwritten immediately afterward, so it does not need to be cleared
first.
Field ::Current is unconditionally cleared by
MultiLevelInv_CountLevelPowerups, so it does not need to be cleared
first.
Field ::RespawnTimer is not accessed by
MultiLevelInv_CountLevelPowerups, but needs to be cleared. Add a clear
of field .RespawnTimer and remove the clear of the entire structure.
All callers of MultiLevelInv_Count passed a constant value. Factor
out the top level blocks of MultiLevelInv_Count into helper functions,
then create two new functions corresponding to MultiLevelInv_Count(0)
and MultiLevelInv_Count(1), implemented by calling the appropriate new
helper functions.
Uses of `(short*)` usually want exactly a 16-bit signed integer. Most
platforms provide that as `short`, but that is not guaranteed.
s/(short \(*\+\)\s*)/(int16_t \1)/g
C casts do not require parentheses. C++ casts require grouping around
the target. Prepare for conversion to C++ casts by adding otherwise
unnecessary parentheses around the target of simple C casts.
s/\((\w\+\s*\*\+)\)\s*\(&\w\+\(\[[[:alnum:]+-]*\]\)*[]);]\)/\1(\2)/g
Grant processing code sets `Primary_weapon` to the user's preferred
weapon, but the delayed autoselect code then changed it back to
something else. Call select_primary_weapon instead of directly updating
`Primary_weapon`, so that `Delayed_primary` is updated too.
This also fixes a bug where the demo always recorded the player
switching to lasers/concussion, instead of the values that the player
actually received from the grant.
Reported-by: Mako88 <https://github.com/dxx-rebirth/dxx-rebirth/issues/184>
C casts do not require parentheses. C++ casts require grouping around
the target. Prepare for conversion to C++ casts by adding otherwise
unnecessary parentheses around the target of simple C casts.
s/\((\s*\(\(un\)\?signed\|int\|char\|short\|long\|float\|double\|s\?size_t\|\(u\?int[[:digit:]]\+_t\)\)\s*\**\s*)\s*\)\([&+-]\?\)\([[:alnum:]_.]\+\s*->\s*\)*\([[:alnum:]_.]\+\)\(\s*\[[^][]*\]\)*\(\s*\([];+>)*\/^%,|&<>]\)\|$\|\(\s*-\s*[^>]\)\)/\1(\5\6\7\8)\9/g
This pass only targets commonly used standard types.
s/(\(\s*\(\(un\)\?signed\|int\|char\|short\|long\|float\|double\|s\?size_t\|\(u\?int[[:digit:]]\+_t\)\)\)\s*)\s*(/static_cast<\1>(/g
C casts do not require parentheses. C++ casts require grouping around
the target. Prepare for conversion to C++ casts by adding otherwise
unnecessary parentheses around the target of simple C casts.
Exclude float|double from the second substitution. Including it
rewrites medmisc.cpp in an incorrect way, and excluding it does not
exclude any valid rewrites.
s/(\(int\|signed\|float\|double\|long\|short\|unsigned\))\s*\(\w\+\s*(\s*\w\+\s*)\)/(\1)(\2)/
s/(\((\(int\|signed\|long\|short\|unsigned\))(\w\+\s*(\s*\w\+\s*))\))/\1/g
C casts do not require parentheses. C++ casts require grouping around
the target. Prepare for conversion to C++ casts by adding otherwise
unnecessary parentheses around the target of some C casts.
This pass attempts to process expressions that involve parenthesized or
bracketed subexpressions, but only if those subexpressions do not
themselves contain parenthesized or bracketed subexpressions.
(int) f(1); // changed
(int) f(g()); // not changed
perl -p -i -e 's/(\(\s*((?:un)?signed|int|char|short|long|float|double|s?size_t|(?:u?int[[:digit:]]+_t))\s*\**\s*\)\s*)([&+-]?)([[:alnum:]_.]+\s*->\s*)*([[:alnum:]_.]+)((?:\s*(?:\[[^][]*\])*|(?:\([^()]*\))*))(\s*([;+>*\/^%,|&<>])|$|(\s*-\s*[^>]))/\1\(\3\4\5\6\)\7/g'
Object numbers are unsigned short, not signed short. If anyone ever
raises the object limit high enough, testing for negative values would
blacklist valid objects.
Remove unnecessary test for objnum!=object_none. If it were none,
constructing `obj` would have failed.
This pass only targets commonly used standard types.
s/(\(\s*\(\(un\)\?signed\|int\|char\|short\|long\|float\|double\|s\?size_t\|\(u\?int[[:digit:]]\+_t\)\)\s*\*\)\s*)\s*(/reinterpret_cast<\1>(/g
This pass only targets commonly used standard types.
s/(\(\s*\(\(un\)\?signed\|int\|char\|short\|long\|float\|double\|s\?size_t\|\(u\?int[[:digit:]]\+_t\)\)\)\s*)\s*(/static_cast<\1>(/g
C casts do not require parentheses. C++ casts require grouping around
the target. Prepare for conversion to C++ casts by adding otherwise
unnecessary parentheses around the target of simple C casts.
This pass does not attempt to process expressions that involve
any subexpression that can nest arbitrarily, such as parentheses or
brackets. It also works only on commonly used standard types.
(int) a->b; // changed
(int) a[b]; // not changed
s/\((\s*\(\(un\)\?signed\|int\|char\|short\|long\|float\|double\|s\?size_t\|\(u\?int[[:digit:]]\+_t\)\)\s*\**\s*)\s*\)\([&+-]\?\)\([[:alnum:]_.]\+\s*->\s*\)*\([[:alnum:]_.]\+\)\(\s*\([];+>)*\/^%,|&<>]\)\|$\|\(\s*-\s*[^>]\)\)/\1(\5\6\7)\8/g
Both callers of ReadIniArgs need the string removed afterward. Prior to
8fb9a0f, only the parser for `-ini` needed the string removed. The
caller for the automatic ini file ended and destroyed the container. As
of 8fb9a0f, the container persists. Leaving the string in the container
causes a bad error message if the command line is rejected.
It is bad form to have ReadIniArgs pop, but the caller push. However,
the two callers use different signatures for emplace_back(), so unifying
the push would require calling a suboptimal emplace_back. Choose the
lesser evil of unbalanced push/pop.
Reported-by: zicodxx <https://github.com/dxx-rebirth/dxx-rebirth/issues/198>
get_chunks_state has a member named `data` and its constructor took a
parameter named `data`. This is unambiguous in the language, but causes
a warning from gcc's -Wshadow when building with WORDS_NEED_ALIGNMENT.
Rename the parameter to eliminate the warning.
Reported-by: derhass <https://github.com/dxx-rebirth/dxx-rebirth/pull/196>
Descent for DOS supported paging content on demand. Rebirth has no
support for paging content out, but retained piggy_page_flushed
to track whether anything had been paged out. Commit 3c20c24 ("Disable
piggy_bitmap_page_out_all") removed the last site that could set
piggy_bitmap_page_out_all to a non-zero value. All remaining code
either tests it for non-zero or sets it to zero.
Remove the statements that set it to zero.
Remove assertions that the value is zero.
Remove conditional blocks that execute only when it is non-zero.