Newer gcc has become more effective at propagating values and now proves
that certain tests will always throw. This proof triggers a
compile-time error, which is desirable in the main program, but not in
the unit test. Add an indirection to hide the value from the compiler
so that it is forced to emit runtime code, which can then throw and
catch as intended.
PF_OVERFLOW is part of p3_flags, not p3_codes. Testing p3_codes meant
that the disk would be drawn when CC_OFF_RIGHT was unset. This seems
unlikely to be what was intended, so fix the mismatch by switching to
use p3_flags instead of switching the symbolic constant.
gcc libstdc++ built with -D_GLIBCXX_DEBUG includes in
std::array::operator[] an assertion of the form:
```
assert(index < size());
```
The valptridx allow_end_construction constructor, when given a
completely full array, will call operator[](size()), which causes the
assertion to fail. Rework the constructor to compute an end iterator
without using operator[]. valptridx allow_end_construction expects that
the resulting iterator may be the end iterator and shall not be
dereferenced.
Introduce a helper to obtain both the magnitude of a vector and the
corresponding normalized vector. Use it to capture both values as const
when possible.
`Multi_is_guided` is only enabled in one place. One code path set up
Guided_missile[] for the local player, and a different path handled
network players. Remove `Multi_is_guided` and rearrange such that a
single site handles both local and network players.
This is only used in the file where it is defined, so it can be inlined
even though the definition is not in a header. Enabling inlining
encourages gcc to see that the `operator new`/`operator delete` calls
are not mismatched.
clang-14 is detected as being able to optimize out unreachable paths,
but then triggers a build error reporting an unspecified invalid use
somewhere in multi.cpp.
Remove the static check, and rely on -Wsuggest-attribute=noreturn to
report any functions which are guaranteed to fail. This is a weaker
check, but over the course of development, the static check has been hit
rarely, if ever, so keeping it provides little value.
```
In file included from similar/main/multi.cpp:38:
In file included from common/main/game.h:32:
In file included from common/main/robot.h:34:
In file included from common/main/object.h:40:
common/include/valptridx.h:229:2: error: call to unsigned int valptridx<dcx::player>::check_index_range_size<valptridx<dcx::player>::index_range_exception, std::__1::less>(char const*, unsigned int, unsigned long, valptridx<dcx::player>::array_managed_type const*)::DXX_ALWAYS_ERROR_FUNCTION::dxx_trap_handle_index_range_error() declared with 'error' attribute: invalid index used in array subscript
DXX_VALPTRIDX_CHECK(Compare<std::size_t>()(s, array_size), handle_index_range_error, "invalid index used in array subscript", a, s);
^
common/include/valptridx.h:37:3: note: expanded from macro 'DXX_VALPTRIDX_CHECK'
DXX_VALPTRIDX_STATIC_CHECK(dxx_valptridx_check_success_condition, dxx_trap_##ERROR, FAILURE_STRING); \
^
common/include/valptridx.h:20:5: note: expanded from macro 'DXX_VALPTRIDX_STATIC_CHECK'
(DXX_ALWAYS_ERROR_FUNCTION(FAILURE_FUNCTION, FAILURE_STRING), 0) \
^
build/ulinux-clang++-14-64b10d04-ogl/dxxsconf.h:84:2: note: expanded from macro 'DXX_ALWAYS_ERROR_FUNCTION'
DXX_ALWAYS_ERROR_FUNCTION::F(); \
^
1 error generated.
```
Given this code:
```
enum class X : unsigned;
X a{1};
```
gdb has an unfortunate behavior of reporting "incomplete type" and
refusing to show the underlying value of `a`, even when `a` is in scope.
Adding `enum class X : unsigned {}` fixes this and allows gdb to show
the value.
Compiler error messages are generally better when reporting a misuse
that fails a requires() versus reporting a misuse that fails a
std::enable_if. In some cases, this also makes the code clearer, and
avoids the need for dummy template parameters as a place to invoke
std::enable_if.
The caller has the player object, and can provide the orientation matrix
from it. Use that instead of letting multi_send_fire recompute the
object in order to get the matrix.
When the player fires a generic secondary, the choice of proximity bomb
versus super-mine is never read. When the player specifically drops a
bomb, only that choice needs to be read. Pass the chosen weapon as an
argument, and avoid computing the choice on the path where it is never
read.
Two paths are always local. One path is always remote. Split the
handler function to remove the locality test and rely on the caller's
knowledge of whether the affected player is local.
Instead of sliding the entire queue down one element at a time, move the
non-expired elements in one step, and wipe all the trailing elements
in a single pass.
The only values for any given player are true/false, so use a bitset
instead of a byte per player. Some implementations may use a uint64_t
internally, negating the space savings. Even so, this version is
an improvement for requiring the use of playernum_t as an index.
The function had one caller, which always passed `0`. Inline that.
Next, observe that `sent` is only ever modified `if (!(Game_mode &
GM_NETWORK))`. As a multiplayer function, `multi_send_robot_frame`
should never be called in single-player, so this test should always be
false. Delete it, and observe that `sent` is now `const`.
Remove unused return value of `multi_send_robot_frame`.
Instead of passing a bare `int` named `secret_flag`, define it as an
`enum class : uint8_t` to name the two special values.
Rework the passing of this value, to deal with some confusing
inconsistencies when reading the code.
Before this change:
In D1:
- Multiplayer will always go to the secret level, regardless of which
exit door the player used.
In D2:
- Flying through a D1 secret exit in multiplayer shows the on-HUD error
"Secret Level Teleporter disabled in multiplayer!", and does not exit
the level. This is at best confusing, and at worst dangerous, since
D1 secret exits are only available during the countdown, so the player
has little time to realize that the normal exit must be used instead.
- Like D1, multiplayer will request to go to the secret level regardless
of the exit used. Unlike D1, the caller ignores the flag and always
advances to the next regular level.
After this change:
- No observable differences for the player in-game. The questionable D2
secret exit handling for D1 is retained.
- The code makes clearer that secret exits do not work in D2
multiplayer, by way of `#if defined(DXX_BUILD_DESCENT_I)` guarding the
existence of the parameter and all updates to it.
