feat: simplify feature loading#115
Merged
Merged
Conversation
alandtse
approved these changes
Oct 1, 2023
alandtse
left a comment
alandtse
left a comment
Contributor
There was a problem hiding this comment.
Awesome. Thanks for handling. We're probably not merging the new features for a while so not sure we need to wait. Can you also update the wiki developer section explaining the new overrides?
Collaborator
Author
|
Ah gotcha! Do you think I should include the drawdeferred or not? Due to it not being called yet |
Contributor
|
Fine to include. We can PR the features to use it when they come in. |
Collaborator
Author
|
Nice, I'll update the wiki as soon as this is merged then! |
alandtse
referenced
this pull request
in alandtse/open-shaders
Jul 20, 2025
feat: simplify feature loading
ParticleTroned
pushed a commit
to ParticleTroned/skyrim-community-shaders
that referenced
this pull request
Jun 11, 2026
…pect) (community-shaders#115) ## What Adopt devbench **1.5.0**'s generalized `RegisterToolExtension` to expose the Open Shaders menu and non-feature reads as extensions of the base `menu` / `inspect` tools, instead of bespoke registrations. **No fallback** — gated on `GetBuildNumber() >= 10500` for ABI safety; older hosts simply don't get these (the other `openshaders.*` tools still register). - **Menu** → `RegisterToolExtension("menu", "CommunityShaders", …)`; same `menu invoke name='CommunityShaders'` routing, now via the generalized API (was the 1.4.0-only `RegisterMenuHandler`). op: `open|close|toggle` (default toggle). **Returns `{op, queued:true}`** — the change is applied on the **render thread** next frame (open is a no-op while first-time setup is pending). - **Inspect** → drop the top-level `openshaders.inspect` tool; register two base-`inspect` extensions: `inspect kind=openshaders` (engine state `{plugin,frame_count,vr}`) and `inspect kind=shadercache` (cache status). `openshaders.shadercache` (clear/deleteDisk) stays distinct (mutate vs read). - **Port** → devbench-api bumped to 1.5.0 (REF `5bc4ad7`). ## Threading `SetVisible` is render/UI-thread-only (touches the ImGui context + `IsEnabled`). Off-thread callers use `Menu::RequestVisibility`, which records an atomic request consumed on the render thread in `ProcessInputEventQueue`. Storage is **last-absolute + toggle-count** (private atomics) so rapid sub-frame toggles aren't collapsed; `ClearInputKeys` fires only on the closed→open transition. ## Testing Built clean (ALL). Runtime-tested on SE + VR against devbench 1.5.0: `menu invoke` open/close/toggle and `inspect kind=openshaders`/`kind=shadercache` over REST — see PR comments for results. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ParticleTroned
pushed a commit
to ParticleTroned/skyrim-community-shaders
that referenced
this pull request
Jun 11, 2026
VR runtime smoke (SteamVR null-driver headless rig) caught a real CTD the DLL/SE-smoke gates couldn't: an EXCEPTION_ACCESS_VIOLATION in BSShaderRenderTargets::Create (id 100458) +0xF4B. - Hooks.cpp: upstream's new CreateRenderTarget_Water1/Water2 hooks (community-shaders#2484, water RT fp16 promotion) shipped 2-arg REL::Relocate (no VR offset). On VR they wrote a CALL at the SE offset inside Create -> corrupted the function -> AV on the render thread. RE'd the VR call sites in SkyrimVR.exe Create (targets 0x59/0x5a): Water1 VR +0x12C2, Water2 VR +0x12D8 (cross-checked monotonic vs UnderwaterMask 0xE06 < Water 0x12C2/0x12D8 < PrecipitationMask 0x1917). Now 3-arg, works on VR. - Upscaling.cpp: drop the duplicate "Depth VR Propagate" block the community-shaders#2475 revert re-added beside our existing one (dev had exactly one, gated isVR && depthUpscaleActive). The sacred propagate stays; only the redundant copy removed. A parallel fork-feature-loss audit (dev 6728765 vs HEAD) found NO genuine fork losses: all removed capabilities were either relocated/preserved or upstream PR removals (e.g. SkySync Sun Position Offsets removed by upstream community-shaders#2408 -- evolution, not loss). All fork features (VR, foveated, LLF superset, branding, UAF hooks, community-shaders#115, sacred propagate) confirmed intact. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Simplify feature loading by adding optional virtual functions in Feature.h that can be overriden by features. This will reduce the need to modify State.cpp, XSEPlugin.cpp & Menu.cpp when adding new features etc
Added to
Feature.h:DataLoaded<-- Called by SKSEkDataLoadedeventPostPostLoad<-- Called by SKSEkPostPostLoadeventClearShaderCache<-- Called by imgui clear shader cache buttonDrawDeferred<-- This one is not called yet, that is in the subsurface-scattering branchFixed almost all tasks of #97
Link to build for this PR -> https://github.com/FlayaN/skyrim-community-shaders/releases/tag/untagged-626eeecf0c4665d55ff6
Created this as a draft due to this should maybe wait until subsurface-scattering is in dev (Or maybe remove the DrawDeferred part of the PR), would like some feedback if this is what you meant on your issue @alandtse