Due to original game bugs, which Rebirth intentionally replicates in
order to match the balance of the original game, the logic looks odd.
Add comments explaining that the oddity is intentional.
object_create_muzzle_flash delegated to
object_create_explosion_without_damage, adding one parameter that
callers ought to provide instead. Inline object_create_muzzle_flash into
callers and change them to provide `Vclip`.
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.
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.
Resolve the object to a vmobjptridx once, and use it as needed. Replace
some uses of ConsoleObject with the local player, since those should be
the same, and local player is already computed.
Commit 81dd23b151 ("Only charge player for weapons that fire successfully")
added an early return when `objnum == object_none`, so if execution
reaches the call to `multi_send_fire`, then the object is guaranteed to
exist. Access the track_goal without rechecking whether the object
exists.
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.
- 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.
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.
Previously, rendering the preferred bomb type could also change it if
the active type was exhausted. This is undesirable, since it depends on
the user to have a HUD mode which causes the bomb type to render.
Switch to only change the active type if the user tries to drop a bomb
while the active type is unavailable. Some call sites already switch
bomb types automatically on depletion. Those sites will still do so.
gcc and clang disagree about how to disambiguate when an identifier is
both a typename and a member. Avoid the disagreement by renaming the
member.
Reported-by: Kreeblah <https://github.com/dxx-rebirth/dxx-rebirth/issues/532>
