3151 Commits
Author SHA1 Message Date
Goober5000 b483d30b20 Merge pull request #7804 from Goober5000/fix/misc
four miscellaneous fixes
2026-10-04 13:08:13 -04:00
Goober5000andClaude Fable 5.1 112ca2fc4b handle submodel repair/destruction consistently
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]>
2026-09-29 02:58:02 -04:00
Goober5000andClaude Opus 5.5 8c8620ef0f don't look up the operator of an empty SEXP list
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]>
2026-09-25 02:28:24 -04:00
Goober5000 33c4bb0222 make is_special_sender static 2026-09-25 01:52:15 -04:00
Goober5000andClaude Opus 5 c10df878ba remove Current_filename_sub and tidy up stop_parse
Current_filename_sub dates back to the use of *_ex() parsing functions for parsing .eff files, which were superseded by the Bookmarks feature.  Today it is entirely redundant to Current_filename.  A read that throws now leaves Current_filename naming the file that failed, rather than the file parsed before it, as it should.

Also clear Current_filename in stop_parse, so that a warning issued between the end of one parse and the start of the next no longer blames the last file parsed, and null out Mp there so it stops dangling into the freed parse buffer.  Also delete the vestigial Mp_save.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
2026-09-25 01:52:15 -04:00
Goober5000andClaude Opus 5.5 04ba1b2015 type SEXP variable references by the variable's declared type (#7799)
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]>
2026-09-24 13:21:47 +09:00
Goober5000andClaude Opus 5 0dcee2a6be Add scripting access for per-guarder guard ranges (#7794)
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]>
2026-09-21 05:12:47 +02:00
Goober5000andClaude Fable 5.1 04e698c29b fix weapon substitution bank attribution and turret ammo accounting
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]>
2026-09-05 19:22:45 -04:00
Goober5000 b08ab184ff Merge pull request #7757 from Goober5000/fix/flagset_64bit
Fix 32-bit truncation in the flagset u64 round-trip
2026-08-30 23:36:54 -04:00
Goober5000andClaude Opus 4.7 ba7aa70794 Fix 32-bit truncation in the flagset u64 round-trip
Mission flags are saved to the mission file as the full 64-bit flagset value, but three issues prevented anything larger than 32 bits from being saved as intended:

 * `stuff_flagset` parsed the value with `stuff_long`, where `atol` clamps anything above `LONG_MAX`
 * `flagset::from_u64` cast the incoming value to `unsigned long`, truncating bits 32-63
 * `flagset::to_u64` used `bitset::to_ulong` where it should use `bitset::to_ullong`

Fix these issues and add `static_assert` in the flagset functions to defend against future issues.  Also use the proper UINT64_T_ARG where it should be used.

This was first triggered by `Limited_support_rearm_pool`, the first mission flag to occupy bit 31.  Only mission flags are affected; the other flagset serialization paths were audited and are safe.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
2026-08-30 16:46:11 -04:00
Goober5000 87189525e0 clean up check_sexp_syntax and improve caching
- 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.
2026-08-27 14:21:30 -04:00
wookieejedi e0e3e9dff2 Fix CTD with ship-percent sexp edge case (#7738)
`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.
2026-08-25 15:11:40 -04:00
Goober5000 41884cb1a3 wing name format compatibility and other cleanup
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.
2026-08-23 21:38:52 -04:00
Goober5000 5a67e8e952 Merge pull request #7711 from Goober5000/fix/sexp_parsing
patch a hole in SEXP syntax checking
2026-08-18 00:52:21 -04:00
The Force 54caf6ec25 New guard-range sexp (#7606)
* 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
2026-08-16 17:53:41 +01:00
Kestrellius 1967daffd3 Vectorize MsgItems (#7687)
Convert MsgItems from an array to a vector, and remove the associated limit value and counter.

* vectorize msgitems

* appeasement

* leave me alone clang

* remove optional

* cleanup

* changes requested

* clang

* clang what is WRONG with you

* feedback

* remove flag in .h
2026-08-16 02:04:50 +00:00
Goober5000 da4f4e70ac patch a hole in SEXP syntax checking
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.
2026-08-15 15:21:03 -04:00
Goober5000 3bfe71d834 Merge pull request #7670 from Goober5000/fix/msg_arg_count
send-message-list arg-count forgiveness, plus better error checking
2026-08-03 22:21:13 -04:00
John Fernandez 92384b3c60 More Search UI/UX Enhancements (#7612)
* 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.
2026-07-30 12:59:33 -04:00
Goober5000andClaude Fable 5 5fa6674688 send-message-list arg-count forgiveness, plus better error checking
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]>
2026-07-30 01:03:06 -04:00
Goober5000andClaude Fable 5 d40fcd16c0 fix player subsystem targeting from sexps and scripting
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]>
2026-07-28 23:09:24 -04:00
The Force da05081405 Fix C4709 Warnings In modern VS (#7576)
* 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
2026-07-26 14:33:00 +01:00
b26b9cf743 Add Vulkan rendering backend, with HDR10 output and raytraced shadows (#7554)
* Shadow overhaul round 2: PCSS, dynamic cascades, cockpit/instanced shadows

Squashed snapshot of lafiel/shadow_overhaul_2's continuation work
(d3922aa87..010f9d2d9), which has no separate upstream PR of its own.

* Add Vulkan rendering backend

Squashed snapshot of the Vulkan backend as vendored into this branch
(ce2d1e85f..6ce539463, authored by Mara van der Laan with build/CI
fixes by Taylor Richards). This work has its own upstream PR #7553
(notimaginative:vulkan-pr-new) and is not otherwise original to true-hdr.

* Add true HDR output, RT shadows, and PCSS shadows for Vulkan

Squashed snapshot of true-hdr's own integration work: true HDR
framebuffer output, wiring the shadow overhaul into the Vulkan
backend (raytraced + cascaded shadow maps), logging refactors, log
filter UI, and CI/build fixes.

* refactor: update Vulkan logging calls to use `nprintf` for scoped logging

* fix: correct shadow cascade count in Vulkan logging calls

* fix: fall back to PATH-resolved ccache when CCACHE_PATH is stale

CI hardcodes /usr/local/bin/ccache, but macOS runners on Apple Silicon
hosts install Homebrew (and ccache) under /opt/homebrew instead,
causing all four Mac configs to fail with "No such file or directory".

* Fix vulkan texture readback to correctly handle transparent pipelines

* Put the pixel swizzles into a util file

* OOB safety check

* Fix fog handling

* Fix MSAA in Vulkan

* fix: enable SMAA for vulkan

* adapt the post-processing AA shaders to hdr

* fix clang-tidy warnings: concatenate nested namespaces, static methods

pixel_swizzle.{h,cpp}: collapse `namespace graphics { namespace util {`
into `namespace graphics::util {` (modernize-concat-nested-namespaces).

VulkanState.h/VulkanDraw.h: getClearColor()/setClearColor() only touch
gr_screen and other globals, no instance state, so make them static
(readability-convert-member-functions-to-static) and drop the
top-level const on getClearColor()'s return type
(readability-const-return-type).

* fix clang-tidy warning

* Extend raytraced shadows to all directional lights

RT shadows previously only shadowed the first directional light in the
scene, even though multi-sun missions push more than one directional
light into the engine. The TLAS raytraced shadows query against is
already light-agnostic, so this extends both the deferred lighting
pass and the forward MODEL shader to shadow every directional light
(capped by a new Max Raytraced Shadow Lights option, configurable via
the in-game Options menu or the Lab's Scene rendering options).

Cascaded shadow maps are untouched: they're built for a single light
only, so the deferred pass still limits itself to one shadowed
directional light whenever CSM (not raytracing) is the active method.

Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_016DA6JHJ3twz2oHGQcZ1Udc

* refactor: improve shader flag handling and render target finalization in Vulkan backend

- Consolidate shader flag setup logic in VulkanPostProcessingLighting to simplify configuration.
- Fix dead field usage in VulkanDraw by ensuring correct image view selection for environment maps.
- Add mip chain regeneration in VulkanRenderer::endRenderTarget to ensure correct mip levels for cubemaps.

* fix: ensure correct mip chain initialization in cubemap rendering

- Add mip chain generation to VulkanDraw for cubemaps with multiple mip levels.
- Fix potential issue with uninitialized mip levels above level 0 in deferred-f.sdr sampling paths.

* fix: don't crash on startup when environment locale is unavailable

std::locale("") pulls in the process's environment locale categories
(LC_CTYPE, LC_MONETARY, LC_PAPER, etc.), and throws std::runtime_error
if any of them names a locale not installed on the host (e.g.
mismatched regional-format overrides). Since SCP_default_locale is
constructed during static initialization, before main() and before any
of our own exception handling exists, an uncaught throw here aborted
the whole process before the game even started. Fall back to the
classic "C" locale instead, which is sufficient since this locale is
only ever used for ASCII case-folding (SCP_toupper/tolower/isdigit).

Co-Authored-By: Claude Sonnet 5 <[email protected]>

* `feat: add raytraced shadows for local lights and optimize TLAS management`

* Make the Commandline authoritative for -res and -vulkan / -opengl

* address BMagnu's comments on the original PR

* update prebuilt version hash to 62c89ef6

* refactor: standardize graphics API handling and document Vulkan limitations in qtFRED

- Updated `AboutDialogModel.cpp` to use `GraphicsAPI` enum for better clarity.
- Ensured qtFRED falls back to OpenGL when Vulkan is requested.
- Added detailed documentation in `qtfred/README.md` regarding Vulkan backend limitations and requirements.

* chore: include cmdline library in main and mission management modules

* docs: add known issue for blank render viewport under Wayland in README.md

* fix CI issues

* chore: add `GR_DEBUG_SCOPE` markers across Vulkan post-processing and rendering paths for improved debugging

* feat: improve Vulkan HDR pipeline with separate encode paths for SDR and HDR10

- Added an HDR10-specific output encode pass in `VulkanRenderer` and `VulkanPostProcessor` that applies PQ/BT.2020 transfer.
- Introduced an optimized SDR fallback path with direct blit support for devices that can skip shader draws.
- Updated `m_sceneLdrCompressed` to provide [0,1]-clamped proxies for HDR edge detection.
- Refactored `copyImageToImage` and added `blitImageToImage` to allow format conversions and optimized transfers.
- Replaced `hdr_mode` with explicit HDR10 flags and data structures for clearer state handling.

* refactor: encapsulate SMAA processing and refactor VulkanPostProcessor

- Moved SMAA-specific logic into `VulkanSMAA` class for better separation of concerns.
- Simplified `initSMAA`, `shutdownSMAA`, and `executeSMAA` logic by leveraging `VulkanLDR` and `PostProcessContext` abstractions.
- Improved initialization and cleanup code with clearer resource lifecycle management.
- Updated debug markers and messages to reference `VulkanSMAA`.
- Enhanced flexibility of fullscreen triangle rendering API with additional parameters (`sampleCount`, `bindGlobalSet`).

* Update Vulkan porting credits in AboutDialogModel

* fix CI issues again

* fix: correct Y-flip in viewport setup for Vulkan light volumes

- Adjusted viewport height to negative value to account for G-buffer render pass's negative Y-axis convention.
- Ensured proper alignment of point and tube light volumes with actual light positions.

* feat: add user-adjustable gamma correction to SDR and HDR10 output paths

- Added gamma correction to HDR10 path, applying user settings before PQ/BT.2020 encode.
- Replaced the SDR blit-only path with an encode pass that applies gamma correction, ensuring consistent behavior across devices.
- Refactored VulkanPostProcessor and associated shaders to support gamma slider adjustments for both SDR and HDR output.

* fix: ensure proper uniform and cull mode setup for Vulkan sphere rendering

- Added uniform buffer setup and default material uniforms in `vulkan_draw_sphere`.
- Disabled face culling in material definition for sphere rendering to align behavior with OpenGL.

* Ensure memory flushes after UBO updates and add explicit pipeline barrier for MoltenVK compatibility.

* fix: add clear pass for SMAA edge detection to prevent persistent artifacts

* feat: add Vulkan debug-utils messenger, opt-in sync validation, frame diagnostics

Groundwork for the renderer-review fix series (.agents/plans/
vulkan-renderer-review-fixes.md, Phase 0):

- Register a VK_EXT_debug_utils messenger (preferred) instead of only the
  deprecated VK_EXT_debug_report callback, which is kept as a fallback for
  loaders without debug-utils. The messenger config is also chained into
  instance creation so vkCreateInstance-time messages are captured.
  Validation errors/warnings now go to the main log via mprintf.
- New -gr_sync_validation cmdline flag (implies -gr_debug): chains
  VkValidationFeaturesEXT with synchronization validation into instance
  creation when VK_LAYER_KHRONOS_validation is available. This is the
  regression net for the synchronization fixes in the following commits.
- Frame stats now report descriptor sets/writes allocated per frame and the
  total pipeline count (descriptor-manager counters; DescriptorWriter::flush
  moved out-of-line to report into the manager).

Co-Authored-By: Claude Fable 5 <[email protected]>

* refactor: unify Vulkan render-pass begins, clamp in-pass clears, add per-frame UBO ring

Phase 1 of the renderer-review fix series: shared helpers that the
correctness fixes build on.

- New VulkanRenderer::beginTrackedRenderPass(PassBeginDesc): every render-pass
  begin on the frame command buffer now records vkCmdBeginRenderPass and
  updates the state tracker (pass, color attachment count, sample count,
  render area, viewport) in one place. Converted: setupFrame,
  beginSceneRendering (both branches), resumeSceneRendering,
  endSceneRendering, beginRenderTarget (viewport kept for gr_set_viewport),
  resumeSwapChainPass, plus new resumeScenePassAfterCopy() shared by
  copyEffectTexture/copySceneDepthForParticles and
  rebeginSwapChainPassAfterReadback() shared by both readbackFramebuffer
  exits (~200 lines of duplicated begin/clear/viewport blocks removed).
- The state tracker records the active render area; all four
  vkCmdClearAttachments sites clamp their rects against it and skip empty
  results. Previously gr_screen-sized clear rects could exceed a smaller
  off-screen render target's render area, which is invalid API usage.
- New PerFrameUboRing utility (VulkanPerFrameUbo.h/.cpp): a persistently
  mapped uniform buffer with one slot region per frame in flight and a
  self-flushing alloc(). Used by the next commit to fix the cross-frame
  UBO races.

Deliberate behavior deltas (all safe-direction): the helper always sets
colorAttachmentCount/sampleCount (some sites previously inherited stale
values), and resumeSwapChainPass/resumeSceneRendering now also reset the
standard flipped viewport instead of leaving the previous one.

Co-Authored-By: Claude Fable 5 <[email protected]>

* fix: eliminate Vulkan cross-frame UBO/buffer races, make gr_sync honest

Phase 2 of the renderer-review fix series: with two frames in flight, several
host-visible resources were rewritten by the CPU while the previous frame's
GPU work could still be reading them.

- The post-processing scratch UBO is now a PerFrameUboRing (per-frame-in-
  flight slot regions, persistently mapped, self-flushing alloc). All the
  per-pass map/unmap/cursor bookkeeping in bloom/LDR/fog/SMAA is deleted;
  the cursor resets exactly once per frame via the new
  VulkanPostProcessor::beginFrame() hook in setupFrame.
  This also fixes two latent bugs: (a) fog consumed scratch slots mid-scene
  and bloom's later same-frame cursor reset overwrote them before submission
  whenever fog+bloom were both active; (b) volumetric_fog_data (288 bytes)
  overflowed the 256-byte slots - the memcpy spilled into the neighboring
  slot and the shader's bound range ended before the aspect/fov/noise
  fields. Slots are now 512 bytes (static_assert at the fog call site) and
  the per-frame budget is 32 slots.
- The dedicated tonemap, output-encode, and MSAA-resolve UBOs (each a single
  instance rewritten per frame, i.e. racy; the MSAA one also never flushed)
  are deleted entirely - those passes build their structs on the stack and
  allocate from the scratch ring.
- Static-buffer rewrites orphan instead of racing: getVkBuffer() stamps a
  last-used frame number, and a full-content updateBufferData() on a Static
  buffer used within the last MAX_FRAMES_IN_FLIGHT frames defers the old
  VkBuffer to the deletion queue and writes a fresh one. Offset writes
  (GPU-heap appends to fresh regions) and PersistentMapping buffers (the
  engine holds their mapped pointer and syncs via gr_sync) are exempt.
- gr_sync_wait now honors its contract: VulkanRenderFrame::waitForFinish
  takes a timeout and reports expiry without consuming the fence;
  VulkanRenderer::waitForFrame returns false for fences taken during the
  still-recording frame (previously it waited on a stale or idle slot and
  reported success) and resolves the frame->slot mapping correctly. This
  makes UniformBufferManager's segment-fence throttling actually protective
  on Vulkan for the first time.

Co-Authored-By: Claude Fable 5 <[email protected]>

* Refactor framebuffer and target lifecycle: centralized creation/resizing logic, improved resource management across Vulkan post-processing subsystems.

* Improve Vulkan cross-frame sync with explicit access masks and validation updates

- Unified Vulkan validation flag handling in setup, separating engine debug output from synchronization validation.
- Specified access masks and stage flags to address cross-frame WRITE_AFTER_WRITE vulnerabilities, notably in G-buffer, post-processing, and MSAA resolve paths.
- Adjusted subpass dependencies and image layout transitions to prevent read-after-write and write-after-write hazards, detected using `-gr_sync_validation`.
- Fixed uniform buffer and barrier transitions to ensure proper ordering in render passes across frames.

* Refactor Vulkan render-pass dependencies and sync stages for clarity and future testing

- Clarified dependency logic for cross-pass and cross-frame synchronization, ensuring explicit stage and access flag comments match observed behavior.
- Adjusted subpass dependencies to decouple render-pass compatibility definitions from dependency assumptions.
- Highlighted MoltenVK-specific issues and retained workarounds with notes for re-testing on macOS hardware.
- Updated synchronization barriers and validation notes to address historical inconsistencies across platforms.

* Remove transfer queue support; consolidate all operations into the graphics queue

- Removed transfer queue handling, simplifying buffer and queue logic to use the graphics queue for all transfer operations.
- Updated buffer sharing modes and removed related code paths and parameters for transfer queue families.
- Introduced exclusive buffer access, noting async transfer and queue ownership transfers are future work.
- Limited enabled device features to only those actively used by the renderer, reducing unnecessary feature requests (e.g., robustBufferAccess).

* Refactor frame advancement and descriptor management in VulkanRenderer

- Removed `endFrame()` from `VulkanDescriptorManager`, replacing it with `setCurrentFrame(frameIndex)` for explicit frame synchronization.
- Centralized frame index advancement within `VulkanRenderer` to ensure consistency across components.
- Invalidated cached descriptor set bindings at render-pass boundaries to handle raw recorder staleness correctly.
- Optimized pipeline descriptor rebind logic by removing redundant clears during pipeline changes.

* Refactor `PipelineConfig::hash` and Global descriptor set management for improved hashing and memoization efficiency

- Replaced inefficient shift-based hashing in `PipelineConfig::hash` with a robust `hashCombine` strategy to minimize bucket collisions.
- Introduced `maskBits` helper to streamline bitmask creation for boolean fields.
- Implemented memoization for Global (Set 0) descriptor sets, reducing redundant rebuilds across frames and improving performance.
- Added dirty state tracking (`m_globalSetDirty`) and invalidation logic for Global set updates following relevant input changes (e.g., TLAS or UBO modifications).

* Refactor Vulkan post-processing output logic: extracted shared `encodeToSwapChainPass` method to streamline HDR and SDR output paths.

* Optimize Vulkan descriptor set management with per-frame memoization

- Added memoization for Material (Set 1) and PerDraw (Set 2) descriptor sets to reuse cached sets across frames if inputs remain unchanged.
- Implemented precise input tracking for Material and PerDraw descriptor sets to detect and rebuild only when necessary.
- Reduced redundant Vulkan descriptor allocations and writes, improving performance on repeated or batched draw calls.

* Fix Vulkan access masks for G-buffer and scene pass transitions

- Added `eColorAttachmentRead` to access masks in multiple barriers to ensure proper synchronization for load operations (e.g., `loadOp=eLoad`) across render passes.
- Prevented `READ_AFTER_WRITE` synchronization validation issues in deferred and multi-pass rendering scenarios.
- Updated comments to clarify the reasoning behind the access mask changes and load dependencies.

* Refactor Vulkan resource management and indexed draw logic

- Consolidated destruction queue methods in `VulkanDeletionQueue` using a shared `queueHandle` template to reduce redundancy.
- Introduced `ModelDrawParams` structure and `computeModelDrawParams` function to unify indexed draw parameters for models and shadows.
- Simplified draw logic by eliminating repeated calculation of indexed draw parameters and improving code reuse.

* Add `releaseAnimationSlotRef` for ref-count handling of shared animation texture arrays

- Introduced `releaseAnimationSlotRef` to manage reference counting for shared animation texture-array slots, preventing premature destruction of shared resources.
- Updated `flushTextures` and related methods to integrate `releaseAnimationSlotRef` for shared slot cleanup.
- Refactored staging buffer creation into `createStagingBuffer` for 3D texture uploads and shared path consistency.
- Enhanced texture upload logic with improved tracking and logging for re-upload cases, aiding preloading gap analysis.
- Added instrumentation for on-demand mid-frame texture uploads in `FrameStats`.

* Refactor Vulkan resource and texture management, extend debug logging

- Updated smart pointer usage for consistency across Vulkan resource managers (`std::make_unique`).
- Removed redundant `cubeImageView` member and related cleanup logic; unified sampling view management.
- Added per-instance debug log counters (avoiding static state) for Vulkan texture uploads and bindings, improving visibility during initialization and run-time.
- Optimized `VulkanRenderer` property/feature getters by caching physical device properties and features during device creation.
- Improved memory region flushing logic for out-of-date offsets, aligning with Vulkan Memory Allocator behavior.
- Refactored render pass state tracking to reapply dynamic state reliably after mid-frame render passes.
- Enhanced validation and error handling for mismatched update texture formats.

* fix CI build warning

* Fix on-demand YUV texture locking and set alpha uniform for movie rendering in Vulkan

- Corrected bitmap lock type from `BMP_TEX_OTHER` to `BMP_AABITMAP` for YUV textures during on-demand loading.
- Updated movie rendering to bind alpha from the `MovieData` UBO (set 2, binding 4) for Vulkan, aligning behavior with OpenGL implementation.

* revert inadvertent changes introduced by a squash commit

* address ci issues

* remove wip stuff from code comments that make no sense to people who aren't me

* Remove unused LD_LIBRARY_PATH export and fix shader shadow map sampling.

* Refactor batched rendering uniform setup in Vulkan

- Removed unused fields from `genericData` uniform block, simplifying shader definitions.
- Updated Vulkan material uniform handling to align with OpenGL's batched rendering logic.
- Introduced `batched_data` struct for streamlined UBO updates, improving maintainability and clarity.

* Add `-hdr` command-line option for HDR nits configuration

- Introduced `-hdr` parameter to set paper white and peak luminance in nits, overriding in-game HDR settings.
- Added input validation with error feedback for incorrect format.

* Remove unused `MODEL_SDR_FLAG_SHADOW_MAP` macro from shader flags

* clean up unrelated changes

* Invalidate Vulkan state tracker bindings after external command buffer modifications

- Added `invalidateExternalBindings()` to `VulkanStateTracker` to handle stale pipelines, descriptors, and dynamic states caused by external Vulkan commands (e.g., ImGui renders mid-pass).
- Updated ImGui rendering flow to call `invalidateExternalBindings()` post-draw, ensuring consistent pipeline state for subsequent engine renders.
- Adjusted image usage flags to account for `eTransferSrc` in texture readbacks.
- Fixed barrier `dstAccessMask` and pipeline stage flags to ensure proper synchronization for image transitions.

* Add a few more generically useful lines to the log

* Simplify Qt deployment logic and improve handling for system-installed Qt on Linux

* add cmdline headers to make qtFred compile

* use VK_KHR_synchronization2 for sync and barrier purposes

* Simplify and clean up Qt deployment logic in CMake for qtfred

* update get_bitmap_from_texture to match the new API

* Enhance `-hdr` command-line option to force HDR output and support optional nits configuration with improved input validation and overrides.

* Improve Vulkan robustness, add device feature checks, and refine memory/logging.

- Added depth clamping support gated by device feature checks, with fallback where unsupported.
- Improved deferred lighting UBO overflow handling with user-accessible warnings and buffer size adjustments.
- Enhanced Vulkan logging for validation, allocation, and debugging:
  - Automatic logging for descriptor pool exhaustion/fragmentation.
  - Vertex format and resource destruction checks.
  - Notifications for generic layout transitions and volumetric texture uploads.
- Consolidated shader logic to ensure Vulkan and OpenGL parity.

* Remove HDR10 encode shader and consolidate gamma correction logic for SDR and HDR10. Replace `Verify` assertions with `Assert`

* Add braces to conditional in object lighting logic for clarity

* Refine Vulkan memory allocation checks with `Verification` for improved error handling and messaging.

* Refine VulkanBuffer resize logic to preserve contents with CPU-side copy and add safeguards for non-coherent memory scenarios.

* Add detailed comments explaining descriptor array capacity constants in VulkanDescriptorManager

* Gate shadow support on depth clamp feature availability and clarify its necessity with improved comments.

---------

Co-authored-by: BMagnu <[email protected]>
Co-authored-by: Mara van der Laan <[email protected]>
Co-authored-by: Claude <[email protected]>
2026-07-21 16:58:21 +00:00
Goober5000andClaude Opus 4.8 bed228d264 avoid undefined behavior from UNREACHABLE on reachable code paths (#7555)
* 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]>
2026-07-09 11:20:25 +09:00
Goober5000 598c5c71a3 clean up ship status after mission (#7523)
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.
2026-07-05 04:13:06 +02:00
Goober5000 d15cadc47b Merge pull request #7537 from Goober5000/fix/defensive_unicode
more graceful degradation of invalid unicode
2026-06-20 20:50:27 -04:00
Goober5000andClaude Opus 4.8 fd2ebaf941 allow -1 sound index in OPF_GAME_SND syntax check (#7540)
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]>
2026-06-21 01:59:08 +02:00
Goober5000 43b4724240 more graceful degradation of invalid unicode
If invalid UTF-8 is encountered, return the standard Unicode replacement character 0xFFFD.  When incrementing or decrementing, make a best effort to follow the code's instructions.  It might produce one or more � characters, but that's better than an Error and a hard crash.

Also move character constants to standardized locations.
2026-06-20 16:22:21 -04:00
Goober5000andClaude Opus 4.7 876a1d36e9 fix CTD when LuaSEXP evaluates a subsystem on an absent ship
The OPF_SUBSYSTEM argument-conversion path in LuaSEXP only verified
has_shipp() before calling ship_entry->objp().  A ship registry entry
retains its shipnum after the ship is destroyed or departed, but its
objnum is reset to -1, so objp() either asserts (debug) or dereferences
Objects[-1] (release) when the entry is looked up by Lua scripting
during, for example, an OnMissionAboutToEnd hook.

Tighten the check to has_objp() so absent ships short-circuit to an
empty handle, and correct the early-return type from l_Ship to
l_Subsystem.  Apply the matching type correction to OPF_DOCKER_POINT,
which has the same return-type mismatch but no CTD because it never
dereferences objp().

Fixes #7206.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
2026-06-03 16:40:59 -04:00
BMagnu b70657a190 Moveable Instant Finish (#7476)
* Expose functions to instantly finish current moveable

* Fix IK crash
2026-05-25 01:31:53 +02:00
Goober5000 4bc45b3481 Merge pull request #7459 from Goober5000/fix/comm_node
fix the Shivan Comm Node
2026-05-15 22:17:34 -04:00
Goober5000 bed223c5c7 second part of comm node fix
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)
2026-05-14 00:04:31 -04:00
Goober5000 f8fa1e4775 a bit of cleanup
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
2026-05-13 23:29:31 -04:00
Mike Nelson 76c00dbc06 Add set-guard-range sexp (#7251)
* set-guard-range

* make sure old path remains unchanged

* address feedback again
2026-05-11 07:53:58 -04:00
Goober5000andClaude Opus 4.6 3f85d6172a use std::move() for local variables that are copied but not reused (#7308)
* 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]>
2026-05-07 14:40:38 +09:00
LuytenKy 09a848a224 Refactor player-set-target SEXP (#7393)
* Refactor of logic based on feedback

* resolve requested changes
2026-05-03 12:26:45 -04:00
Goober5000 f0ed5c88b8 Merge pull request #7407 from Goober5000/fix/coerce_to_utf8
fix coerce_to_utf8
2026-04-28 22:39:14 -04:00
Goober5000 abc5e87914 Merge pull request #7365 from Goober5000/fix/sexp_type_checking
improve SEXP syntax checking
2026-04-28 22:37:10 -04:00
Goober5000 66eb4702e1 fix coerce_to_utf8
Fix a bug that Claude caught: conversion from Latin-1 would fall through, truncate, and emit a spurious warning.
2026-04-28 01:26:11 -04:00
LuytenKy 2894ddb1fa Feature: Implements the SEXPs that set the player's target and or clear it if necessary (#7349)
* 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
2026-04-15 11:57:40 -04:00
Goober5000 56341002c8 improve SEXP syntax checking
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`.
2026-04-08 23:09:20 -04:00
Goober5000 219f692660 Merge pull request #7339 from Goober5000/claude/waypoint_position
remove redundant waypoint position by delegating to the game object
2026-04-08 21:12:04 -04:00
Mike Nelson 36d05ba9d8 Implement Photo Mode (#7287)
* 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
2026-04-08 10:33:51 -04:00
Cliff GordonandClaude Opus 4.6 cfd9e0087c Fix multiplayer standalone crash: client stats race + host accept code (#7319)
* 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]>
2026-04-03 09:26:23 -04:00
Goober5000 cc26887774 Merge pull request #7300 from Goober5000/cleanup/enhance_utility_h
enhance utility functions
2026-03-31 21:22:23 -04:00
Goober5000andClaude Opus 4.6 3c6df26da2 remove redundant waypoint position by delegating to the game object
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]>
2026-03-31 20:08:59 -04:00
Goober5000 6a20c0f549 Merge pull request #7117 from Goober5000/anchor_refactor
refactor ship anchors
2026-03-22 02:59:47 -04:00
Goober5000 05054f89f6 Merge pull request #7222 from Goober5000/text_replacement_updates
consistent handling of text token replacement
2026-03-20 15:00:16 -04:00
Goober5000 49f00019a7 enhance utility functions
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`.
2026-03-20 02:22:20 -04:00
Goober5000 ce31a33029 fix persistent variables and containers (#7192) 2026-03-13 08:16:28 -04:00