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.
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.
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>
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.
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.
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.
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.
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.
