GCC std::remove_if overwrites removed elements using:
*dstiter = move(*srciter);
This is fine for normal containers, but produces incorrect results when
*dstiter returns a proxy object instead of a reference. In that case,
the proxy object is move-assigned from the source, then goes out of
scope. If the move assignment did not write to underlying storage, as
valptridx proxy objects do not, then incorrect results occur. This
broke ActiveDoor handling (fixed in 4a01fab66d98[1]) and has been a trap
waiting to recur. Apply reference-qualifiers to valptridx objects so
that move-assignment requires an lvalue for the left-hand side. This
permits normal use of move-assignment, but forces a compile error if
std::remove_if or similar are used on valptridx proxy objects.
[1]: 4a01fab66d
Ill-formed levels can provoke this exception. Downgrade the error from
a fatal exception to a CON_URGENT message.
Increase the array size to 32 from D1:7, D2:20, since each element is
only an int.
Switch from an assertion on bitmask overflow to a CON_URGENT warning.
Fixes: f7f416c3cb ("Trap blown bitmap overflow")
Many gauge functions take the same parameters, and pass those parameters
on to child functions in turn. Bundle common parameters into a few
standard structures, so that adding new context does not need to involve
every function in the chain for every new context value.
Replace magic strings with sentinel objects, so that mistakes are
reported as missing variables, rather than being legal at parse time and
incorrect at runtime.
If user_settings.sharepath is configured to be blank in SConstruct, omit
the C preprocessor macro SHAREPATH instead of defining it to expand to
an empty string. Adjust the C++ code that uses the macro SHAREPATH to
handle its absence:
- Clearer output in help text
- Skip adding blank SHAREPATH to the PhysFS search path.
As a nice side effect, this enables SHAREPATH on Windows, which could be
helpful for builds shipped with an installer that places game files in a
well-known location (such as "C:\Games\Descent"). Previously,
!defined(__unix__) systems did not add SHAREPATH to the PhysFS search
path, even when one was defined.
Commit 131c1b9f4d added support for PNG screenshots and made this support a default build option. Set screenshot=legacy in the project file for macOS's Xcode for all out-of-the-box builds. PNG support has been dropped in Xcode for several reasons:
- macOS creates PNG screenshots via Command-Shift-3. This works in DXX-Rebirth, even in full screen mode.
- The macOS screenshots are put on the desktop, as opposed to in the user's Library (which is normally hidden from view and can only be accessed via 'Go' menu holding down Option)
- Macs have no Print Screen key
- There is no simple way to include libpng with the macOS DXX-Rebirth binaries, and the average Mac user is unlikely to install it from source
When building with scons, the Mac user compiling can do one of two things:
- Install libpng (which is much easier than packaging with a binary)
- Pass screenshot=legacy to scons
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")
Define global copies of the user's chosen host address, host port, and
local port. When these are non-empty, prefer them to the values from
-udp_hostaddr, -udp_hostport, and -udp_myport. When the user attempts a
connection, update the global copies from the user's data. If the user
abandons the connect dialog, do not update the globals.
Requested-by: Mako88 <https://github.com/dxx-rebirth/dxx-rebirth/issues/73>
Slide left/right: A/D
Slide up/down: C/X
Accelerate/reverse: W/S
Afterburner (D2 only): Left-Shift
This produces the more FPS-typical WASD layout for
forward/left/back/right movement, and maps the crouch/stand bindings to
slide up/down. This ticket sat for longer than it should have (though
it never missed a release). Several competing designs were suggested,
but only one could be made active. After consideration, I used my own
bindings on the basis that, while some other bindings may be better,
every other configuration repurposed a classic weapons-fire key to
movement. Returning players who get the "new defaults" on a newly
created pilot profile might be very surprised by having their fire keys
move. Since these are only defaults, and can be rebound by the player
with a few minutes work, these defaults do not need to be perfect. They
just need to be an improvement over original Descent.
Delete unexpand-cpp-kconfig-key.py. It will likely never be needed, and
was added in the prior commit solely to have a file to recover if it
ever is needed.
Requested-by: Mako88 <https://github.com/dxx-rebirth/dxx-rebirth/issues/214>
Future work is simpler without this, and the stringified name is only
visible in debug output, which is in turn rarely used due to its
verbosity and lack of filtering controls.
Per C99, this code was always wrong. The results of `snprintf` are
undefined if the target string array and one of the supplied input
arguments refer to the same buffer. In practice, this particular usage
happened to work by wastefully copying a string onto itself, then adding
more content at the end. Switch to appending properly, which also fixes
the gcc-8 -Wrestrict warning.
References: <https://github.com/dxx-rebirth/dxx-rebirth/issues/388>
Various files included compiler-static_assert.h to use the compatibility
macros for compilers that lacked a working C++11 static_assert.
However, some source files used static_assert without this inclusion,
and no one ever reported problems. From this, assume that no one uses a
compiler which lacks C++11 static_assert. Remove the inclusions that
were only for the compatibility macro. Keep the inclusions that use the
assert_equal helper.
- Remove the one test that clang-5 still fails.
- Require all remaining tests to pass using only C++11 native
static_assert.
- Remove preprocessor-based alternative static_assert implementations.
gcc-8 adds calls to static_assert with a comma inside its first
argument in the implementation of std::vector. This is legal within the
standard, but conflicts with Rebirth's use of static_assert as a
two argument preprocessor macro. The macro is not substantially useful,
and was only present to compensate for the clang limitation removed in
the previous commit. Remove the macro.
References: <https://github.com/dxx-rebirth/dxx-rebirth/issues/388>
valptridx contains `static_assert` statements of the form:
static_assert(var.m, "");
where `var` is a non-`constexpr` reference and `m` is a `static
constexpr` member of a base type of `var`. gcc recognizes that a
`static constexpr` member is a constant expression and permits this.
clang rejects this, presumably because `var` is not a `constexpr`
variable. In the almost 3 years since this was added, clang has not
improved to permit this usage. Rather than continuing to suppress
static_assert in clang, rewrite this expression to be less clear, but be
compatible with clang.
gcc-8 has special cases in its std::advance that require the target to
support `operator++` and `operator--`. These are easy to support, so
add them.
References: <https://github.com/dxx-rebirth/dxx-rebirth/issues/388>
gcc-8 warns that static_cast<dt &>(d) is useless. In general, it is
useless. It is present to force a compile error in cases when `d` is
not convertible to `dt &`. Switch to a compound statement that declares
a local reference to `d` of type `dt &`. This achieves the same
checking effect, but does not provoke the -Wuseless-cast warning.
References: <https://github.com/dxx-rebirth/dxx-rebirth/issues/388>
Python 2 permitted defining __new__, then deleting it once it had been
used. Python 3 does not directly reject that pattern, but instead
always fails with an error about wrong number of arguments to object().
Switch to a simple classmethod that is explicitly invoked by the first
__init__().
In Python 3, filter returns an object that can be iterated to get the
filtered content. However, there is no way to reset the object, so it
is useless for persistence. Spend the one iteration copying the data
into a tuple, which can be iterated multiple times.
Python 3 `dict.keys()` returns an object that cannot be added to other
instances of itself. Pass the returned objects as a tuple, then let the
callee use itertools.chain to combine them during iteration.
Python 3 removed `dict.keys` and `dict.items`, then renamed
`dict.iterkeys` and `dict.iteritems` to `dict.keys` and `dict.items`,
respectively. The iteration semantics are preferable here, so add shims
to use the iteration API under a version-appropriate name.
Change PreservedEnvironment.__getitem__ back to a class method, instead
of an instance variable. Python 3 refuses to search the instance
dictionary for __getitem__, so the optimization of copying
`self.flags.__getitem__` to `self.__getitem__` breaks in Python 3.
Python 3 changed the rules for list comprehensions, so the comprehension
can no longer see a class-scope variable without a class qualifier. The
class qualifier is not legal until the class is finished. Work around
this limitation by expanding the use inline.
Materialization centers ("Matcens") have a governor to prevent
overrunning the level in robots. However, due to a logic ordering
error, multiplayer games failed to record how many robots had been
destroyed, so for the purpose of detecting whether there were currently
"too many" robots in play, the game pretended that no robots had ever
been destroyed. Therefore, once enough robots had been created to reach
the "too many" threshold, matcens stopped generating robots, regardless
of how efficiently the player(s) had been destroying robots.
Reported-by: Mako88 <https://github.com/dxx-rebirth/dxx-rebirth/issues/132>
Reported-by: Sirius-TR <https://github.com/dxx-rebirth/dxx-rebirth/issues/386>
If no character matches the typed character, the loop should exit when
`lb->citem == cc`, which should happen after every entry is visited
once. However, if no item is selected, `lb->citem == -1`, and `cc` is
never `-1`, so the exit path never triggers. Rewrite the loop to be
bounded by number of steps, rather than bounded by returning to a
particular offset. This also protects against undefined behavior if the
menu had zero elements.
