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.
Using `netplayer_info` in `UDP_sequence_packet` defined many unused
fields. Replace this with per-message types that carry only the
required fields. Make these fields `const` where possible.
Remove the definition of FQ_CHECK_OBJS and all uses of it. Add a new
fvi_query member d_level_unique_object_state *LevelUniqueObjectState.
If object checking is enabled, pass &LevelUniqueObjectState in that
member. If object checking is disabled, pass nullptr in that member.
Change fvi_sub to use this member to decide whether to perform object
checking.
If FQ_CHECK_OBJS is used, a valid object is required. Copy the
icobjptridx_t from fvi_query into a local vcobjptridx_t to force a
validation, then use that validated copy for later work.
- Make all members constant, and pass an anonymous temporary fvi_query
to find_vector_intersection.
- Change `p0`/`p1` to `const vms_vector &`, since the positions are
mandatory. Callers can no longer pass `nullptr` or an uninitialized
value here.
- Change `thisobjnum` to `icobjptridx_t`. Calls to fvi_sub built an
objptridx at need, so moving it to the caller allows it to be
constructed once per find_vector_intersection call.
- Move `flags` and `rad` out of fvi_query, since calls to fvi_sub may
use other values than the ones in fvi_query. This prepares for
passing fvi_query to fvi_sub.
Embedding it in d_level_shared_robot_info_state is reasonable from a
relational perspective, but interferes with referencing
d_robot_info_array when only a forward declaration of
d_level_shared_robot_info_state is available.
clang-13 needed an operator==(const exact_type<T>&) defined to avoid an
ambiguity. Now that C++20 mode is enabled, switch to a
compiler-generated operator== instead of manually defining the
equivalent function.
In C++ 20 mode, clang-13 gets confused about how to handle operator==.
Rewrite the test to encourage it to pick the correct version.
```
common/include/gr.h:129:12: error: use of overloaded operator '==' is ambiguous (with operand types 'dcx::grs_main_bitmap *' and 'exact_type<dcx::grs_bitmap>')
if (this == &r)
~~~~ ^ ~~
common/include/pack.h:31:17: note: candidate function (with reversed parameter order)
constexpr bool operator==(const T *rhs) const { return p == rhs; }
^
common/include/gr.h:129:12: note: built-in candidate operator==(struct dcx::grs_bitmap *, struct dcx::grs_bitmap *)
if (this == &r)
```
clang-13 in C++20 mode fails:
```
similar/main/newmenu.cpp:202:18: error: ISO C++20 considers use of overloaded operator '==' (with operand types 'exact_type<const dcx::grs_font>' and 'exact_
type<const dcx::grs_font>') to be ambiguous despite there being a unique best viable function [-Werror,-Wambiguous-reversed-operator]
return &cv_font == &game_font ? "\202" : "\207"; // 135
~~~~~~~~ ^ ~~~~~~~~~~
common/include/pack.h:31:17: note: ambiguity is between a regular call to this operator and a call with the argument order reversed
constexpr bool operator==(const T *rhs) const { return p == rhs; }
^
```
Add an operator== that exactly matches the input types so that clang
does not attempt a conversion.
gamedata_read_tbl is only called if (D1 || (D2 && EDITOR)). Exclude
defining it for (D2 && !EDITOR). From there, also exclude defining or
reading the alias list, which is only written by a function
called by gamedata_read_tbl.
Reported-by: heftig <https://github.com/dxx-rebirth/dxx-rebirth/issues/642>
This produces a better error message than a static_assert failure, since
gcc points directly to the call which resolved to the deleted overload.
When the caller is correct, this produces the same result as the prior
version.
splitpath_t is designed for MS-DOS paths, even though Rebirth now runs
on many platforms that never used DOS conventions. Most of the members
of splitpath_t are unused on all platforms. Remove them, and switch to
returning an initialized version of the structure.
Previously, the supplied pointer was converted to an array index, then
passed to valptridx::idx for validation. If the index_type is smaller
than std::size_t, this would truncate the value before validation.
Certain out-of-range indexes would be in-range after truncation, and
incorrectly not be reported.
Reorder the check to validate the index against the array size before
truncation.
Players are not robots, but the show-path cheat tried to pretend they
are, and triggered a BUG warning[1] when looking up robot information
from the player object. Fix this by passing in the robot_info when the
caller is providing a robot, and a named `nullptr` value (as
`create_path_unused_robot_info`) when the caller is providing a
non-robot object.
[1] As below, but repeated many times
```
similar/main/ai.cpp:1905: BUG: object 0x555555858550 has type 4, expected 2
similar/main/ai.cpp:1974: BUG: object 0x555555858550 has type 4, expected 2
```
Zero the entire array, then overwrite the leading portion with the
received data. This permits the compiler to do a fixed number of large
stores, instead of a variable number of small stores.
The type aliases are sufficient. Individual bytebuffer_t
implementations can define a `static constexpr` member `endian` from the
type alias and rely on `std::integral_constant<T, V>::operator()`
instead of defining a `static` method just to return an instance of the
`std::integral_constant`.
Use `object &` instead of `vmobjptr_t`. This should generate equivalent
code, but produce smaller debug information and may require less
inlining by the compiler.
Every caller passes `1`, so remove the parameter and always use `1`.
For backward compatibility with the previous network protocol, continue
to send a `1` in the network message.
This allows taking the input by-value instead of by-reference, while
still protecting against unwanted type conversion.
Add some basic static_assert tests that the swapped values are correct.
Commit f1606f7747 ("Simplify test for
__builtin_bswap16") changed the SConstruct test to either define both
DXX_HAVE_BUILTIN_BSWAP and DXX_HAVE_BUILTIN_BSWAP16 or to define neither
of them. Follow up that commit by removing the definition of
DXX_HAVE_BUILTIN_BSWAP16 and redirecting uses of it to
DXX_HAVE_BUILTIN_BSWAP.
The conditional definition of a D2X flag causes a technical ODR
violation. It is legal, though not useful, to define this flag in D1X
and in common code. Remove the preprocessor guard so that D2X and
common code use the same definition for the enum.
Split check_index_range to check_index_range+check_index_range_size.
Redirect check_explicit_index_range_ref to check_index_range_size, so
that the index_type is not truncated and then extended.
Use a single constructor that accepts anything convertible to both of
the required types, rather than special constructors for:
- Accepting a qualified_segment
- Accepting a variant of susegment with compatible const qualifiers
- Accepting a type T that converts to qualified_segment
This reduces the number of constructors to consider, which improves
error messages when an invalid input is used.
v29_trigger and v30_trigger define a field `time`. v29_trigger never
initializes it. v30_trigger initializes it from the uninitialized
v29_trigger in legacy mode, and from a file field otherwise. No program
logic ever reads this member, so remove it.
When a file name is available, use SDL_RWFromFile + Mix_LoadMUSType_RW,
rather than calling Mix_LoadMUS(filename). This allows the calling code
to be more consistent.
SDL1 defines Mix_LoadMUS_RW as:
```
Mix_LoadMUS_RW(SDL_RWops *rw)
Mix_LoadMUSType_RW(rw, MUS_NONE, SDL_FALSE);
```
SDL2 defines Mix_LoadMUS_RW as:
```
Mix_LoadMUS_RW(SDL_RWops *src, int freesrc)
Mix_LoadMUSType_RW(src, MUS_NONE, freesrc);
```
The version with freesrc is preferable, and Rebirth used it to set
freesrc=SDL_TRUE in SDL2 mode. For SDL1, Rebirth used special logic
in the reset() call to emulate setting freesrc=SDL_TRUE. SDL1 also
exposes Mix_LoadMUSType_RW, which allows the caller to set freesrc in
both SDL1 and SDL2. Switch to that, so that the same code is used in
SDL1 and SDL2.
If the file is accessible via PhysFS, the next branch, based on
PHYSFS_openRead, will find it and use it. If it is not accessible via
PhysFS, then attempting to resolve its path would fail. Thus, this
attempt is redundant regardless of whether the file is reachable by
PhysFS. Remove it.
The contents of the output buffer are undefined if PHYSFSX_getRealPath
fails, so mark the function as [[nodiscard]] and modify all callers to
check that the function succeeded.
Rework the error paths to return path-specific status codes so that the
caller can report exactly which step caused an HMP file to be rejected.
On error, print this reason numerically and, if the reason was a PhysFS
error, also print the PhysFS error code numerically and symbolically.
If the type is not None, an earlier statement will have already
returned. Thus, the type can be assumed to be None if these test
statements are reached.
Previously, if the filename had no dots, then mix_play_file would refuse
to play it at all. This was excessive, as only the check for hmp needs
a filename with a dot in it. Change the logic so that a dot-less file
still is assumed not to be an hmp, but instead of returning, will fall
through and try the other load types.
Return the type for compatibility, but set the music type immediately.
Change callers to return without using a switch to set the music type
based on the returned value. This allows the callers to exit early on a
successful load.
Remove the `switch` in the caller, since now every path that can set
`current_music_type` will either (a) set it to None because the load
failed or (b) set it to non-None and return before reaching the
`switch`.
Return the type for compatibility, but set the music type immediately.
Change callers to return without using a switch to set the music type
based on the returned value. This allows the callers to exit early on a
successful load.
- Centralize the default value for the hook function.
- Move some static functions to the anonymous namespace.
- Define functions for setting the mixer parameters to ADLMIDI mode and
SDL_mixer mode, so that these modes can be set from more places.
AlumiuN reports that mingw32-w64-gcc-8.1.0 incorrectly reports
`ascending` as unused-but-set. This is clearly not true. Reorder the
code to avoid saving `ascending`, and instead use the result of
`detail::get_xrange_ascending` directly. This also improves the error
message in the DXX_ALWAYS_ERROR_FUNCTION path.
Reported-by: AlumiuN <https://github.com/dxx-rebirth/dxx-rebirth/issues/626>
If two or more events are delivered in the same loop, the previous
implementation would count joystick motion multiple times. Fix this by
moving the joystick interpretation to occur once, after all the events
have been processed.
- Use std::integral_constant instead of a static function that returns
the value
- Remove unused protocol_family
- Replace the enum with a typedef for the one type that the enum was
used to define
