When a destroyed subsystem was repaired, its submodels were only restored if the repair was done through a SEXP. A subsystem repaired by a support ship, by automatic repair, by a script, or through the debug console regained its hitpoints but not its submodels, so a ship could have a working turret with no model. Any -destroyed form also remained visible.
All of these now call restore_subsystem_submodels, which brings back the subsystem's submodels and hides the -destroyed submodel linked to a special-point subsystem. The -destroyed form of an ordinary submodel is hidden when the submodels are next replicated. The SEXP code uses the same function, with no change in its behavior.
Destruction outside do_subobj_destroyed_stuff now goes through a new blow_off_subsystem_submodels function, which do_subobj_destroyed_stuff also uses. It blows off both of the subsystem's submodels and shows any linked -destroyed submodel, unless the subsystem has "no disappear". This handles the cases where a subsystem appears destroyed without going through do_subobj_destroyed_stuff:
- the subsystem is destroyed at the start of the mission
- the subsystem was destroyed in the previous mission of a red-alert sequence
- the ship changes class while the subsystem's submodels are blown off
Previously the first two blew off only the first submodel, ignored "no disappear" for it, and never showed a linked -destroyed submodel; the third did nothing. For red alert, subsystems with no hitpoints at all are not treated as destroyed. For a class change, the visible state is carried over rather than inferred from hitpoints, so a subsystem set to zero without being blown up, or repaired without its submodels, keeps its appearance.
Red alert also restores the submodels of a subsystem that the mission file starts destroyed if the previous mission's data gives it hitpoints. Ship-copy-damage now shows or hides the -destroyed submodel linked to a special-point subsystem to match the copied state.
As before, the link between special-point subsystems and -destroyed submodels requires "$Link special-point subsystems to -destroyed submodels:" in game_settings.tbl.
The multiplayer hitpoint-update path is not changed. A client receives subsystem hitpoints separately from the notification that a subsystem was destroyed, so a hitpoint update sent just before the destruction could arrive after it and wrongly restore the submodels.
Co-Authored-By: Claude Fable 5.1 <[email protected]>
An empty list such as `( )` leaves get_sexp() with no start node, but it still passed that -1 to get_operator_const() for the ai-chase-any-except compatibility check and, outside FRED, for preloading. This tripped the out-of-range assertion in debug builds. Return the empty list directly instead.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
When parsing a SEXP, a variable reference's node type was decided solely by whether it was quoted: "@Var" became a string variable and @Var a numeric variable, regardless of how the variable was declared. A hand-edited SEXP with the wrong quoting therefore produced either a misleading "Not a number" error or a silently mistyped node that was saved back with the wrong quoting.
get_sexp now takes the node type from the variable's declared type, and flags a reference whose quoting contradicts it. check_sexp_syntax reports such references (or any variable node whose type disagrees with its variable) as the new recoverable error SEXP_CHECK_VARIABLE_TYPE_MISMATCH: FRED flags it, and the game warns but still loads the mission, which runs correctly because the node type has been corrected. Saving from FRED writes the corrected quoting.
Co-authored-by: Claude Opus 5.5 (1M context) <[email protected]>
Follow-up to #7606. Restore the MaxGuardRadius property, restore the ability to set specific ranges using set-guard-range, and add scripting functions for setting the specific ranges.
Fixes#7756
Co-authored-by: Claude Opus 5 (1M context) <[email protected]>
Follow-up to #7635.
Pass the firing bank explicitly through weapon_create() and beam_fire()
instead of searching the ship's banks by weapon class, which could
attribute substitution patterns to the wrong bank. Factor the
substitution logic into get_weapon_substitution_tuple() and only apply
linked/dual-fire pairing to ship-level fire, not turret fire. Skip
substitution on multiplayer clients, since the server already resolved it.
Derive turret bank indices from the weapon slot being fired rather than
the turret's bank cursor, so ammo and substitution state are tracked on
the bank that actually fired. Previously a single-selection turret could
consume ammo from a bank it never fired, and non-ammo turrets (whose
cursors stay at -1) would have skipped substitution entirely.
Use the Player_ship object flag rather than Player_obj in
ship_secondary_bank_can_dual_fire() so the server and clients agree on
dual fire for remote players, and guard ship_secondary_has_ammo() against
a negative bank index.
Co-Authored-By: Claude Fable 5.1 <[email protected]>
- Clean up `check_sexp_syntax` to use `eval_ship` (and incidentally `eval_wing` and `eval_prop`) rather than the old lookups.
- Disable SEXP node caching while FRED is running. Also use smart pointers and fix a cache memory leak.
`sexp_percent_ships_arrive_depart_destroy_disarm_disable_scan` could have an edge case where the total value (ie the denominator) is 0 and thus crashes FSO. This could happen if `Cancel_future_waves_of_any_wing_launched_from_an_exited_ship` is used and the death or departure of a mothership triggers the total of a wing to change that is later checked in a sexp such a 'percent-destroyed` or other similar sexps.
Fix is just to add an early out if the total value is 0.
Following PR #7429, ships belonging to wings were hashed the same way as every other ship, instead of the original hackish format where the hash was juggled around the number. This was a mostly transparent upgrade, and ship names were automatically converted from the legacy format to the new format. However, missions that referenced ships *in subsequent waves* broke; the converter only handled ships in the first wave.
This PR adds compatibility checks to `eval_ship` and `ship_registry_get` to accommodate legacy names.
* New guard-range sexp
New guard-range sexp which allows different ranges for different guarders
* cast value
* Clang
* bugfixes
* fix CI
* Address Feedback
* address feedback
* typo
An operator that isn't enclosed in parentheses could trigger an UNREACHABLE, which crashes in Debug and is undefined behavior in Release. Catch it in a more appropriate way.
* Prioritize items that begin with search string
Items that begin with the search string should be prioritized before those that simply contain the search string.
* Add Search Operators option
* Add Safety to Action Handler
I didn't see any crashes, but this matches the pattern of other keyboard shortcuts.
* Allow search for numeric nodes
The working code existed already
* Limit matches on operator search
This limits matches on operator searches by setting a minimum size that a string match has to meet. Function returns -1 on failure, and the warning has been removed to allow this behavior.
* Try this option name better
* Add keyboard shortcut for search
This allows ctrl+s (which can be listed instead of spacebar) to initiate search. There is a new menu item added as well.
* Put this in the correct place
* Address Feedback
* Wrong state
We want the item to be inserted here, not ignored.
Due to a long-standing bug, the groups-of-4 argument count check for
send-message-list and send-message-chain never actually worked, so
missions through 25.0 could includes sexps with truncated argument lists
that recovered gracefully at runtime. Now that the check works, don't
punish those missions: introduce SEXP_CHECK_BAD_ARG_COUNT_BENIGN, which
warns but does not prevent the mission from loading.
To make sexp tree validation more robust, check_sexp_syntax now defers
recoverable errors rather than returning them immediately; otherwise
a recoverable error could mask a fatal error deeper in the tree.
Also fix a stale-variable bug in FRED's fred_check_sexp that could
produce a bogus empty dialog.
Co-Authored-By: Claude Fable 5 <[email protected]>
Fix a regression from #7393: `last_targeted_subobject` must be updated when the player's target and target subsystem are changed, or the subsystem will be retargeted due to `hud_target_change_check`.
Also fix related issues:
- the `ship.Target` and `ship.TargetSubsystem` scripting setters now use the standard `set_target_objnum` function and also correctly handle the new target subsystem
- fix an indexing error in `order.TargetSubsystem`
Co-Authored-By: Claude Fable 5 <[email protected]>
* Fix C4709 Warnings on latest VS
* more
* suppress false positive
* Switch to using any/none_of
* Address Feedback
* Fix Line endings?
* Fix commits and strip out flag_combinator
* more fixes
* nolint
* 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]>
Create a `ship_level_close()` function and call it from `game_level_close()`. Among other tasks, this clears out any PRESENT or DEATH_ROLL statuses to prevent SEXPs or scripts from obtaining references to ships that have been deleted.
Follow-up to #7522.
The numeric branch of the OPF_GAME_SND check in check_sexp_syntax never
actually ran until the type2 variable-overload bug was fixed, so missions
using a sound index of -1 ("no sound" / "default") loaded fine. Now that
the check is live, explicitly allow -1, which the runtime already treats as
an invalid (unset) sound. Also return the more descriptive
SEXP_CHECK_INVALID_GAME_SND / SEXP_CHECK_INVALID_FIREBALL error codes, and
let nested-operator (SEXP_ATOM_LIST) arguments fall through since their type
is already validated upstream.
Also route the Lua SEXP soundentry argument through sexp_get_sound_index so
it accepts a table index, an operator, or a name like the core SEXPs do,
instead of only a name.
Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>
Fix#4089, part 2:
- properly link special-point subsystems with -destroyed submodels, if such a relationship exists and a game_settings.tbl flag is set
- add some helper functions for -destroyed variants of submodels (and now subsystems)
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
* use std::move() for local variables that are copied but not reused
Addresses ~140 Coverity "Variable copied when it could be moved"
defects across 74 files. Every std::move() targets a local variable
whose value is consumed by push_back, emplace, assignment, lambda
capture, or a function call and is never read again afterward.
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
* clang: use std::make_shared
* clang: use std::make_unique
* address feedback
---------
Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>
* draft set-target-player-sexp
* Missed a few entries and duplicates
* Missed another entry
* Yet another missed entry
* Fix subsystem not showing up being targeted
* Fix subsystem targeting in set-player-target SEXP
* remove reduntant return
* change sexp category to better suit feature
Clean up the syntax checker, using clearer variable names for each check, and most importantly unique variable names. Fixes several bugs where syntax checking failed because the same variable was overwritten with incompatible type information.
Also fixes a bug in `check_operator_argument_count` where the count check never ran due to a mixup of `op_index` and `op_const`.
* Implement Photo Mode
* fix gr string errors
* return raw char array
* control conflict groups and better photo param defines
* better param identification in real time
* make photo mode hude dynamically sized
* fix xstrs after all changes
* missed a few
* use modern loop
* Fix multiplayer standalone crash: client stats race + host accept code
Fixes a crash in multiplayer PXO/tracker games hosted on a standalone
server where clients crash at debrief close after scoring a kill:
pilotfile.cpp:295: Ship kills of '<ship class>' not found,
should have been added by pilotfile::update_stats
Two independent root causes were found, both of which must be fixed for
correct behavior:
--- Root cause 1: race condition in multi_sexp_end_mission() (sexp.cpp) ---
The previous code called send_debrief_event() before
multi_handle_end_mission_request() as a "hack" to skip the warp-out
sequence when an end-mission SEXP fires. On standalone servers this is
a race condition: scoring_eval_kill only runs on the master, and the
master hasn't broadcast m_okKills to clients yet at the point
multi_sexp_end_mission() is called. Entering debrief this early causes
scoring_level_close() to run with zeroed m_okKills, so
Pilot.update_stats() creates no kill entries for that session.
When debrief_close() later calls scoring_backout_accept() it cannot
find those entries and hits the UNREACHABLE.
Fix: Remove the send_debrief_event() call entirely. The normal flow
via multi_handle_end_mission_request() has the server broadcast stats
first and then send MISSION_END, which causes the client to enter
debrief with correct kill data already populated. The master enters
debrief through sexp_end_mission() -> send_debrief_event() as before.
The behavioral side-effect of removing this hack: clients that go
through the normal warp-out sequence will no longer skip it on
end-mission. Clients in a state where they cannot warp still receive
immediate debrief via multi_handle_sudden_mission_end().
--- Root cause 2: host accept code never set on PXO standalone (multiui.cpp) ---
In multi_debrief_accept_hit() and multi_debrief_esc_hit(), when the
game is a PXO/tracker game and the local player is the game host
(NETINFO_FLAG_GAME_HOST), the code calls send_store_stats_packet() to
tell clients to record their stats. It then sets
Multi_debrief_stats_accept_code = 1 via the per-client response path
inside the NETINFO_FLAG_AM_MASTER block.
On a standalone server the game host is NOT the master
(NETINFO_FLAG_AM_MASTER is false for the client-side host). The
NETINFO_FLAG_AM_MASTER block is skipped entirely, and
send_store_stats_packet() uses multi_io_send_to_all_reliable() which
skips the local player — so the host never receives its own packet.
Multi_debrief_stats_accept_code stays at -1 (unset).
When debrief_close() checks this value and sees it is not 1, it
assumes stats were tossed or the window was closed without accepting,
and runs scoring_backout_accept() unconditionally. With fix 1 in place
this no longer crashes (entries exist), but silently strips all session
kill credit from the host's pilot record.
Fix: After the send_store_stats_packet() block, call
multi_debrief_stats_accept() unconditionally for the game host so the
accept code is set locally regardless of whether the host is also the
standalone master.
Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
* Preserve end-mission warp-out skip via deferred debrief entry
The previous fix removed send_debrief_event() from
multi_sexp_end_mission() entirely, which was correct for the stats
race condition but had a side effect: clients would no longer skip the
warp-out sequence when an end-mission SEXP fires. The warp-out skip
was an intentional feature — process_endgame_packet() checks whether
the client is already in debrief state and, if so, bypasses
multi_warpout_all_players().
Restore the original behavior by deferring the send_debrief_event()
call to process_endgame_packet() rather than eliminating it.
multi_sexp_end_mission() now sets Multi_sexp_end_mission_pending=true
before calling multi_handle_end_mission_request(). When the client
receives the MISSION_END packet, process_endgame_packet() checks the
flag, calls send_debrief_event() there, then falls through to the
debrief-state check which skips the warp-out exactly as before.
This is safe because send_endgame_packet() calls multi_broadcast_stats()
before sending MISSION_END. Both use reliable delivery, so stats are
guaranteed to be received and applied before MISSION_END is processed,
meaning scoring_level_close() now sees correct kill data when
send_debrief_event() fires.
Alt-J-initiated mission ends are unaffected: the flag is never set,
so the warp-out proceeds normally for those paths.
Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
* Revert multiui.cpp accept code change — tracker master owns stats
The previous change added multi_debrief_stats_accept() calls for the
game host in multi_debrief_accept_hit() and multi_debrief_esc_hit() to
work around Multi_debrief_stats_accept_code staying -1 on a standalone
server. This was based on incorrect analysis.
In PXO tracker games the standalone master is intentionally the sole
authority for stat storage (via multi_fs_std_tracker_store_stats() in
multi_standalone_postgame_close()). The host and clients must not save
stats to their local pilot files independently. Multi_debrief_stats_
accept_code remaining -1 and triggering scoring_backout_accept() on the
host is correct behavior: kills are backed out of the local pilot file
because the tracker holds the authoritative record, not the local file.
The race condition fix (deferred send_debrief_event()) is sufficient to
prevent the crash: Pilot.update_stats() now creates kill entries with
correct m_okKills data, so scoring_backout_accept() can find and remove
them cleanly rather than hitting the UNREACHABLE.
Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
* Move Multi_sexp_end_mission_pending from multiutil to multi and reset in multi_vars_init()
Addresses review feedback: the flag has no relation to multiutil, and
resetting it in multi_vars_init() ensures it is always cleared between
missions regardless of which code path was taken.
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
---------
Co-authored-by: Claude Sonnet 4.6 <[email protected]>
The waypoint class stored its position in m_position separately from the game object's pos field, requiring callers to manually keep both in sync. Now get_pos() and set_pos() delegate to the object position when the game object exists, falling back to the parsed position only if the object does not exist.
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
1. Add optional `start_index` and `num_items` parameters to the find/count functions in utility.h.
2. Fix an accidental recursion bug.
3. Change appropriate parameters to `size_t` and cast appropriately.
4. Add two forms of `SCP_vector_contains_lcase`.
5. Enhance `sprintf` and add `sprintf_concat` for `SCP_string`.
Testing in FotG exposed a bug that dates back to retail. During mission parsing, arrival and departure anchors were represented by indexes into `Parse_names`. After parsing, they were converted to ship indexes -- but only in FRED. Since some functions are shared between FRED and FS2, this can produce inconsistent results, which can be hard to detect since parse indexes and ship indexes are usually the same.
This refactors arrival and departure anchors to be represented by ship registry indexes in all cases. It also adds conversion functions for the benefit of FRED and QtFRED since those dialogs were designed with ship indexes in mind.
The existing `cannot-perform-scan` flag prevents the cargo from being shown on the HUD, but some modders would like to see the cargo while still preventing a ship from being able to perform a scan. So, this renames the existing flag to `cannot-perform-scan-hide-cargo` and adds a new `cannot-perform-scan-show-cargo` flag. Compatibility code in both mission parsing and flag evaluation ensures that missions using the old flag will still work.
Note that if both `cannot-perform-scan-hide-cargo` and `cannot-perform-scan-show-cargo` are set, the former will take precedence.
Add a useful function for getting a ship_info pointer from a ship registry entry. Also rename `ship_has_dock_bay` to `ship_has_hangar_bay` for clarity, since docking bays and docking points are not the same thing. Finally, add a `model_has_hangar_bay` function.
* First draft of prop object type
* Add prop editor dialog in FRED
* prop sexps
* lua table prop support
* lab support
* lua mission access to props
* prop collision support
* use std::optional so the object->instance id is always valid
* allow props to block suns
* minor cleanup and remove arbitrary limit. Still limited to 500 objects.
* Sort props by category
* cleanup
* implement collision hooks
* parse prop flags and further ensure props are inited and closed correctly
* Add Cyborg's multi suggestions
* fix missing header
* add props to empty slots if available
* add multi signature
* fix rebase issues and match collisions to the new multithread style
* appease Clang the Conquerer
* save/parse mission prop classes by name
* Clean up comments and finish TODOs
* fix string errors
* clang wants this to be static now
* address feedback
* address feedback
* Use model num from prop info instead
* merge prop and ship collision group sexps together
* fix rebase bug
* address feedback
* fix name shadowing
* ship heavy submodel
* clang
---------
Co-authored-by: Asteroth <[email protected]>
Just as special tokens are replaced in debriefings, replace them in command briefings and briefings as well.
Change `string_replace_tokens_with_keys` to `message_translate_tokens` (since it's the same thing) and add a `replaceContainers` script API function to complement `replaceVariables` and `replaceTokens`.
Also replace every category of token in `sexp_modify_variable` and add comments to places where token replacement might be expected but happens elsewhere.
The `play-sound-from-file` sexp did not have any way to automatically recycle audio handles, instead relying on other sexps to explicitly close them. It is more modder-friendly to implicitly free any handles of streams that have finished playing, so this PR adds that capability. It also adds a Warning in the event that an unused audio handle cannot be found.
Fixes#7108.
Infamously, in FSO, a wing whose mothership is destroyed will not, itself, be considered destroyed if it has more waves remaining, because wing cleanup and logging typically only happen when the last ship in the last wave exits the mission. Instead, with no mothership to arrive from, waves will simply no longer arrive. Events that check to see if the wing is destroyed or departed will never fire.
This PR fixes that behavior, ensuring that the wing is cleaned up and logged properly, by using the same logic as the enhanced `cancel-future-waves` SEXP operator. This is tied to a new `$cancel future waves of any wing launched from an exited ship:` AI profiles flag which is enabled by default on 25.0+.
Goober5000