Several ai_info fields remember which order set them up (enemy_wing,
guard_objnum/signature/wingnum) or which mode a dynamic goal interrupted
(previous_mode, resume_goal_time), but nothing cleared them when a new
order took over. A ship that once guarded a wing would be yanked back
into AIM_GUARD whenever a wingmate died; a stale previous_mode of
AIM_GUARD could make a former guard abandon a counterattack; a stale
enemy_wing biased every find_enemy() toward a long-ago target; and a
resume_goal_time left over from a dynamic chase suppressed target
acquisition for up to 20 seconds into the next order.
Add ai_reset_order_state(), called when ai_process_mission_orders()
takes up a goal and when ai_do_default_behavior() runs. Each order's
own setup repopulates the fields it needs. enemy_wing is reset under
either $fix stale ai order state or $fix ai target recovery, since it
affects target selection; the rest only under the new flag.
maybe_set_dynamic_chase() wrote aip->mode into previous_submode, a
retail typo that left previous_mode untracked for dynamic chases. Under
the flag, save both fields properly, but not when a dynamic goal is
already in progress, so a nested interruption does not clobber the mode
to return to. Apply the same guard to the strafe/chase transitions in
aibig.cpp.
Co-Authored-By: Claude Fable 5.1 <[email protected]>
The giveOrder scripting function now accepts optional DockerPoint and
DockeePoint arguments for ORDER_DOCK, each given as a dockpoint name or
one-based index (matching the setDocked convention). DockerPoint refers
to a dockpoint on the ordering ship and DockeePoint to one on the target.
If only one point is specified, a compatible point is chosen automatically
on the other ship using the same dock-type precedence as the existing
automatic dockpoint negotiation. When neither is specified, behavior is
unchanged.
Fixes#7119
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
* avoid undefined behavior from UNREACHABLE on reachable code paths
UNREACHABLE expands to __assume(false) / __builtin_unreachable() in
release builds, which is undefined behavior if the code is ever actually
reached. Many UNREACHABLE sites only guard against unexpected-but-
possible runtime states (values from tables, mission/save files, network
packets, scripting, or user input), so reaching them should degrade
gracefully rather than invoke UB.
Replace UNREACHABLE with Assertion(false, ...) at all such sites. It
still pops an error in debug builds, but in release it is a harmless
no-op that simply falls through to the following code. Sites that are
genuinely unreachable by local invariants (e.g. a switch over a bounded
random number) are left as UNREACHABLE.
A few converted sites had no safe fall-through -- an uninitialized read,
null dereference, or null call would have followed -- so they also get a
minimal recovery: initialize the value, guard the dereference, return
early, or skip the offending item.
Where the converted site was a simple guard of the form
if (x) { Assertion(false, ...); return; }
hoist the check onto the invariant instead:
Assertion(!x, ...);
if (x) { return; }
This states the expectation directly and shows the offending value in
the debug popup, while keeping the same graceful runtime degradation.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
* addendum
After consultation with Lafiel, use UNREACHABLE in situations that are effectively impossible even if not provably impossible. UNREACHABLE = "this shouldn't ever happen"; Assertion = "if this ever happens, it's a coding error". Also fix a few other sites and change a couple to Warnings.
---------
Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>
Fix a few items flagged by Coverity and Claude:
- properly check for an unassigned ship node, rather than just Asserting, since this could be affected by mission data
- properly check for null containers for the same reason
- rename `ai_goal_find_empty_slot` to `ai_goal_allocate_slot` and edit comments to reflect its not-quite-side-effect-free role
By default, the player's wing will be issued a form-on-wing order at the beginning of the mission if the wing has no other initial goals. Wings in the "starting wings" list that are not the player's wing will also be issued this goal if they arrive after mission start and have no other initial goals. This PR adds a flag to disable this behavior.
* Back in business hopefully
* Refactor
* Resolve CI issue in aigoals.cpp
* Updating order.cpp
* Whoops
* Updated objecttypes.tbl to incorporate attack ship type
* Fix clang-tidy issue
* Forgot one other instance
---------
Co-authored-by: wookieejedi <[email protected]>
Allow waypoint orders to target a specific waypoint and go in reverse. Add script access to waypoints and waypoint lists belonging to orders.
Fix assignment of targets and goal start times. Assign targets when they are different (and thus need to be assigned), not when they are the same.
1. Allow the goal priority to be an evaluated number, not just a literal number. This allows the goal priority to be obtained from operators or calculations.
2. Since the priority can now be evaluated, it can be NaN. In that case, default to the max priority.
3. Consolidate `CDR(CDR(...))` to `CDDR()` in the aigoal implementation
4. Clarify that `remove-goal` removes the first goal of a given type and priority, not necessarily the first fully matching goal.
5. Enhance `remove-goal` to remove all goals matching the constraints, not just the first goal.
6. Allow `remove-goal` to disregard priority when checking for matching goals.
Several locations in the code (notably `ai_info` and `waypoint_h`) previously stored pointers for waypoints and waypoint lists that need to be valid for more than one frame. Since both waypoint lists and waypoint paths may change size, resulting in vector reallocation, the information should be stored as indexes instead.
Fixes#6353.
This refactors the ship registry and the OSWPT struct to store data as indexes rather than pointers. This is a prerequisite for making these collections dynamic in the future. See also #5800.
A mission in Mantle caused an invalid vector access when checking whether to vaporize a ship whose ship type was -1. Turns out having "no type" is actually allowed, so I audited all `class_type` accesses throughout the code for potential -1 accesses. Most were properly checked but a few were not. A few others needed to be tweaked.
(I suspect this is something that would be on the Coverity list if it isn't already.)
Add "tactical" versions of disable and disarm goals which only perform the specific task without implying anything else. The target ship is not protected and attack goals are not cleared from other ships.
There is also a new AI profiles flag to make this the default hud squad message command instead of the classic disable/disarm.
The ai-fly-to-ship goal was added early in FSO's development but never worked right. Until recently it could crash; and even aside from the crash the AI would try to reach the target's exact position, causing repeated collisions. This folds ai-fly-to-ship into ai-stay-near-ship, since the behaviors are conceptually the same except for fly-to being satisfied as soon as the target is in range.
This also adds a new "escort" mode usable by both behaviors. The default Volition behavior is to swoop toward the target in a long, graceful arc; the new mode generates a goal point and flies directly to that point. These can be toggled via an optional sexp parameter.
There is also some minor cleanup and a new vec3d operator for convenience.
Needed for Scroll; depends on #5667.
Adds a `DEATH_ROLL` status to specifically cover the state after ships have been destroyed but before they exit. I've also audited all code that interacts with the ship registry to use the correct state with this new state taken into consideration.
This fixes access to exploding ships that was broken when the ship registry framework was introduced. In particular, the `when destroyed X beam-lock-all X` QoL feature now works again.
As reported by Woutersmits, the GTT Nova did not dock with the GTD Hall in the first mission of Black Horizon. Attempting to run a debug build caused a CTD. Investigation showed that there was an invalid dockpoint triggering an Int3(). The dockpoint has been fixed, but to aid troubleshooting in the future, the Int3()'s in this function have been changed to more appropriate functions.
Three fixes:
1. In commit fa0878b6b3 for PR #5338, the optional distance parameter was inadvertently changed to a bool. This fixes the parameter.
2. The distance parameter was previously parsed using `atoi` which works for number literals, so change to `eval_num` to allow sexp return values.
3. Make `eval_num` consistent with `eval_sexp` in the sense that evaluating an empty list is valid and returns false or 0.
Also takes advantage of the fact that `is_sexp_true` works for empty lists to streamline optional arguments in `ai_add_goal_sub_sexp`.
Follow-up to #5040. This extends the checks to most iterations over `obj_used_list` in FSO. There are a few exceptions such as `waypoint_add` and `waypoint_remove`, plus a few others. And none of the object iterations in FRED have the guard because FRED does not use the same object life cycle as FSO.
When iterating through `Ship_obj_list`, check the `Should_be_dead` flag before using the object. This fixes#4990 and adds defense-in-depth checks to guard against similar errors elsewhere.
I considered moving the `Ship_obj_list` removal from `obj_delete` to `ship_cleanup`, but decided it would be prudent to avoid fundamental changes to lifecycle management.
