build: add cmake install commands#1372
Conversation
The project is currently spending 5 minutes every time I press the build button, after linking CommunityShaders.dll, without any prompt. Turns out lot of this was the cost from all the `POST_BUILD`` commands attached to CommunityShaders, with `ZIP_TO_DIST` default to `ON`. This change splitted the expensive zip packaging command into seperate Package-XXX targets, where developers can manually build before release. Beside, this change also added proper `cmake --install` support. You can use it to install the AIO package into any folder with `cmake --install ./build/ALL --prefix <TARGET>` command. With all these change, the default preset can now finish an incremental build in about 10 seconds instead of 5 minutes.
WalkthroughRefactors CMake packaging and presets: introduces stamp-based incremental AIO and shader-prepare flows, explicit packaging targets (Core, per-feature, AIO, manual AIO), new deploy/copy targets, updated CMake presets, and expanded build/packaging documentation. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Developer/CI
participant CMake as CMake
participant Stamp as Filesystem (stamps)
participant Stage as Install Staging
participant Pack as Packager (7z/tar)
participant Deploy as Deploy/Copy
Dev->>CMake: configure (preset ALL/Dev/Package/Shaders)
Dev->>CMake: build target (PREPARE_AIO / COPY_SHADERS / prepare_shaders / AIO / Package-*)
CMake->>Stamp: run prepare steps -> produce/compare `*.stamp`
Stamp-->>CMake: stamp indicates prepared state (skip or proceed)
CMake->>Stage: install() into AIO/Core/Feature staging prefix
Stage->>Pack: create archives (Core / Feature / AIO) with stable names
Pack->>Dev: place archives in `dist` (stamp-driven outputs)
alt AUTO_PLUGIN_DEPLOYMENT enabled
CMake->>Deploy: copy artifacts to CommunityShadersOutputDir
Deploy-->>Dev: deployed plugin files
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Points to check:
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (1 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (5)📓 Common learnings📚 Learning: 2025-08-17T18:37:35.839ZApplied to files:
📚 Learning: 2025-07-05T05:20:45.823ZApplied to files:
📚 Learning: 2025-06-24T07:17:36.604ZApplied to files:
📚 Learning: 2025-10-02T14:20:33.454ZApplied to files:
🔇 Additional comments (6)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Automated formatting by clang-format, prettier, and other hooks. See https://pre-commit.ci for details.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
CMakeLists.txt (1)
2-3: Policy set is silenced – lose early-warning signalImmediately switching CMP0116 to NEW and then disabling its warning defeats the purpose of declaring the policy. If an older CMake version is used (e.g. 3.21 ≤ v < 3.26) and the generator behaviour changes again, you will not notice.
-cmake_policy(SET CMP0116 NEW) -set(CMAKE_POLICY_WARNING_CMP0116 OFF) +# Prefer seeing the warning while still opting-in. +cmake_policy(SET CMP0116 NEW) # behaviour we want +# set(CMAKE_POLICY_WARNING_CMP0116 OFF) # keep this *on* unless the noise is overwhelmingUnless there is a very specific reason to suppress the warning (and that reason should be documented), leave it enabled.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
CMakeLists.txt(2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: alandtse
PR: doodlum/skyrim-community-shaders#0
File: :0-0
Timestamp: 2025-07-05T05:20:45.823Z
Learning: In the skyrim-community-shaders repository, file deletion error handling improvements that replace existence checks and try-catch blocks with std::filesystem::remove error-code-based approaches are considered bug fixes rather than refactoring, as they address inadequate error handling and misleading log messages.
Learnt from: alandtse
PR: doodlum/skyrim-community-shaders#0
File: :0-0
Timestamp: 2025-06-24T07:17:36.604Z
Learning: When reviewing PRs, always clarify the scope if there are multiple related features or dependencies. WeatherPicker was a separate PR that was already merged, while this PR focuses specifically on WetnessEffects climate preset system enhancements.
📚 Learning: in the skyrim-community-shaders repository, file deletion error handling improvements that replace e...
Learnt from: alandtse
PR: doodlum/skyrim-community-shaders#0
File: :0-0
Timestamp: 2025-07-05T05:20:45.823Z
Learning: In the skyrim-community-shaders repository, file deletion error handling improvements that replace existence checks and try-catch blocks with std::filesystem::remove error-code-based approaches are considered bug fixes rather than refactoring, as they address inadequate error handling and misleading log messages.
Applied to files:
CMakeLists.txt
📚 Learning: default parameter values are supported in the hlsl compiler used by the skyrim-community-shaders pro...
Learnt from: alandtse
PR: doodlum/skyrim-community-shaders#577
File: features/Wetness Effects/Shaders/WetnessEffects/WetnessEffects.hlsli:57-61
Timestamp: 2025-06-17T05:40:22.785Z
Learning: Default parameter values are supported in the HLSL compiler used by the skyrim-community-shaders project, contrary to standard HLSL (FXC/DXC) limitations.
Applied to files:
CMakeLists.txt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build plugin and addons
🔇 Additional comments (1)
CMakeLists.txt (1)
282-304: Name mangling breaks dependencies & Windows pathsInside the feature-packaging loop you mutate
FEATUREthrough twostring(REPLACE …)calls to drop spaces and dashes, then reuse the mutated value in subsequent iterations.
Example:1st iter:
"Wet Snow"→"WetSnow"
2nd iter now receives already-mangled value and mangles again →"WetSnow"(OK) but original name lost for later use (e.g. path construction).Create a scratch variable instead:
- string(REPLACE " " "" FEATURE ${FEATURE}) - string(REPLACE "-" "" FEATURE ${FEATURE}) - add_custom_target("Package-${FEATURE}" DEPENDS ${FEATURE_PACKAGE}) + set(_feature_target "${FEATURE}") + string(REPLACE " " "" _feature_target "${_feature_target}") + string(REPLACE "-" "" _feature_target "${_feature_target}") + add_custom_target("Package-${_feature_target}" DEPENDS ${FEATURE_PACKAGE})This avoids accidental mutation and keeps
FEATUREusable for path operations later in the loop.⛔ Skipped due to learnings
Learnt from: alandtse PR: doodlum/skyrim-community-shaders#1157 File: src/Feature.cpp:42-49 Timestamp: 2025-06-17T09:27:49.594Z Learning: In src/Feature.cpp, when an obsolete feature's INI file is deleted, the feature should be silently disabled without surfacing any issues to the user. This is the intended behavior because a deleted INI file for an obsolete feature indicates that the user has properly cleaned up the obsolete feature.Learnt from: ThePagi PR: doodlum/skyrim-community-shaders#1369 File: src/Features/SnowCover.cpp:277-293 Timestamp: 2025-08-05T18:22:40.542Z Learning: In the skyrim-community-shaders SnowCover feature, the wstrtostr and strtowstr utility functions defined in src/Features/SnowCover.cpp are unused and should be removed rather than fixed, as confirmed by the original author ThePagi.
- Set `AIO_DIR`` earlier before `prepare_shaders`` - Ensure `dist` folder exists
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
CMakeLists.txt (1)
252-254:dist/still not created – first-time packaging will fail
(Previously raised, still unresolved)
cmake -E tardoes not create parent directories, so the first archive command errors out whendist/is absent. Add the directory creation once near the top of the packaging section:set(DIST_PATH "${CMAKE_SOURCE_DIR}/dist") +file(MAKE_DIRECTORY "${DIST_PATH}")
🧹 Nitpick comments (1)
CMakeLists.txt (1)
2-3: Avoid globally suppressing CMP0116 warnings
set(CMAKE_POLICY_WARNING_CMP0116 OFF)mutes all future diagnostics related to this policy, which may hide legitimate issues in downstream modules or future CMake upgrades.
Prefer one of the following:-cmake_policy(SET CMP0116 NEW) -set(CMAKE_POLICY_WARNING_CMP0116 OFF) +# Keep the NEW behaviour but only silence this project’s own +# warnings – not those of dependencies. +cmake_policy(SET CMP0116 NEW) # adopt NEW behaviour +set_property(DIRECTORY PROPERTY + CMAKE_POLICY_WARNING_CMP0116 OFF) # suppress locally
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
CMakeLists.txt(2 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: alandtse
PR: doodlum/skyrim-community-shaders#0
File: :0-0
Timestamp: 2025-07-05T05:20:45.823Z
Learning: In the skyrim-community-shaders repository, file deletion error handling improvements that replace existence checks and try-catch blocks with std::filesystem::remove error-code-based approaches are considered bug fixes rather than refactoring, as they address inadequate error handling and misleading log messages.
Learnt from: alandtse
PR: doodlum/skyrim-community-shaders#0
File: :0-0
Timestamp: 2025-06-24T07:17:36.604Z
Learning: When reviewing PRs, always clarify the scope if there are multiple related features or dependencies. WeatherPicker was a separate PR that was already merged, while this PR focuses specifically on WetnessEffects climate preset system enhancements.
📚 Learning: in the skyrim-community-shaders repository, file deletion error handling improvements that replace e...
Learnt from: alandtse
PR: doodlum/skyrim-community-shaders#0
File: :0-0
Timestamp: 2025-07-05T05:20:45.823Z
Learning: In the skyrim-community-shaders repository, file deletion error handling improvements that replace existence checks and try-catch blocks with std::filesystem::remove error-code-based approaches are considered bug fixes rather than refactoring, as they address inadequate error handling and misleading log messages.
Applied to files:
CMakeLists.txt
📚 Learning: when reviewing prs, always clarify the scope if there are multiple related features or dependencies....
Learnt from: alandtse
PR: doodlum/skyrim-community-shaders#0
File: :0-0
Timestamp: 2025-06-24T07:17:36.604Z
Learning: When reviewing PRs, always clarify the scope if there are multiple related features or dependencies. WeatherPicker was a separate PR that was already merged, while this PR focuses specifically on WetnessEffects climate preset system enhancements.
Applied to files:
CMakeLists.txt
📚 Learning: in the skyrim-community-shaders snowcover feature, the wstrtostr and strtowstr utility functions def...
Learnt from: ThePagi
PR: doodlum/skyrim-community-shaders#1369
File: src/Features/SnowCover.cpp:277-293
Timestamp: 2025-08-05T18:22:40.542Z
Learning: In the skyrim-community-shaders SnowCover feature, the wstrtostr and strtowstr utility functions defined in src/Features/SnowCover.cpp are unused and should be removed rather than fixed, as confirmed by the original author ThePagi.
Applied to files:
CMakeLists.txt
📚 Learning: default parameter values are supported in the hlsl compiler used by the skyrim-community-shaders pro...
Learnt from: alandtse
PR: doodlum/skyrim-community-shaders#577
File: features/Wetness Effects/Shaders/WetnessEffects/WetnessEffects.hlsli:57-61
Timestamp: 2025-06-17T05:40:22.785Z
Learning: Default parameter values are supported in the HLSL compiler used by the skyrim-community-shaders project, contrary to standard HLSL (FXC/DXC) limitations.
Applied to files:
CMakeLists.txt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build plugin and addons
🔇 Additional comments (1)
CMakeLists.txt (1)
207-208: I couldn’t locate anyfile(REMOVE_RECURSE…)orinstall(CODE…)invocation in your CMake files. Could you confirm whether those lines still exist (and if so, where exactly in CMakeLists.txt), or share the updated snippet so I can verify the potential risk?
|
This will break a lot of the ci if it changes the behavior of the presets. While I think overall the change is good, why couldn't you disable the package variable? |
|
Please also read the spec for conventional commits. Your title is invalid. |
alandtse
left a comment
alandtse
left a comment
There was a problem hiding this comment.
This breaks the ci as there is expectation that when the presets are set files are copied to certain locations for packaging. Please make the necessary changes to address those scripts and tools or do not alter core behavior.
|
Thanks, will take some time to test and fix CI later today :) |
alandtse
changed the title
|
Please make sure you handle the copy_shaders action and that your changes don't break the CI expectation. |
Automated formatting by clang-format, prettier, and other hooks. See https://pre-commit.ci for details.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
CMakeLists.txt (2)
207-208: Directory still removed withfile(REMOVE)instead ofREMOVE_RECURSEThis was flagged previously and is still unresolved:
file(REMOVE …)cannot delete a directory, so${CMAKE_INSTALL_PREFIX}/Coreremains and pollutes every install.-install(CODE "file(REMOVE \${CMAKE_INSTALL_PREFIX}/Core)" COMPONENT Shaders) +install(CODE "file(REMOVE_RECURSE \${CMAKE_INSTALL_PREFIX}/Core)" COMPONENT Shaders)
262-266: Literal quote characters breakcopy_directoryon Windows
list(APPEND CORE_FEATURE_PATHS "\"${FEATURE_PATH}/\"")stores the quotes in the value, so the resulting argument starts with"and the path is invalid.- list(APPEND CORE_FEATURE_PATHS "\"${FEATURE_PATH}/\"") + list(APPEND CORE_FEATURE_PATHS "${FEATURE_PATH}/")Same issue occurs again at Lines 291-292 – please fix both spots.
🧹 Nitpick comments (3)
CMakeLists.txt (2)
222-228:copy_shaders.stamprule never rebuilds when shader install scripts changeThe command only depends on
${HLSL_FILES}. IfCMakeLists.txtor install-related code changes, the stamp is still considered up-to-date and shaders are not re-copied.
Add the script itself (or simply${CMAKE_CURRENT_LIST_FILE}) toDEPENDSso edits invalidate the stamp.
197-200: Appending “/” assumes forward slashes – may produce mixed separators
list(TRANSFORM … APPEND /)forces a Unix separator on Windows paths that already use back-slashes, yieldingC:\path\to\feature/.
While CMake copes in most cases, tools that read the list later might not.
Usefile(TO_CMAKE_PATH …)orcmake_path(NATIVE_PATH …)for portability.README.md (1)
70-87: Command-line example uses both--presetand manual-B– redundant
cmake -S . --preset=ALL -B ./build/ALLoverrides the preset’s ownbinaryDir.
Use either--preset ALLor-S/-B, not both, to avoid confusion:cmake --preset ALL # preferred # or cmake -S . -B build/ALL -D<cache-vars…>
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/build.yaml(2 hunks)CMakeLists.txt(2 hunks)CMakePresets.json(1 hunks)README.md(1 hunks)features/Extended Translucency/Shaders/ExtendedTranslucency/ExtendedTranslucency.hlsli(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- features/Extended Translucency/Shaders/ExtendedTranslucency/ExtendedTranslucency.hlsli
- .github/workflows/build.yaml
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: alandtse
PR: doodlum/skyrim-community-shaders#0
File: :0-0
Timestamp: 2025-06-24T07:17:36.604Z
Learning: When reviewing PRs, always clarify the scope if there are multiple related features or dependencies. WeatherPicker was a separate PR that was already merged, while this PR focuses specifically on WetnessEffects climate preset system enhancements.
Learnt from: alandtse
PR: doodlum/skyrim-community-shaders#0
File: :0-0
Timestamp: 2025-07-05T05:20:45.823Z
Learning: In the skyrim-community-shaders repository, file deletion error handling improvements that replace existence checks and try-catch blocks with std::filesystem::remove error-code-based approaches are considered bug fixes rather than refactoring, as they address inadequate error handling and misleading log messages.
Learnt from: alandtse
PR: doodlum/skyrim-community-shaders#0
File: :0-0
Timestamp: 2025-06-29T00:27:42.717Z
Learning: When reviewing PRs, the committer's description should inform analysis of changes and may take precedence to conclusions when reasonable, but the choice of commit type should still be independently evaluated for compliance with conventional commits. Both committer intent and conventional commit standards should be considered to determine if the classification is defensible.
Learnt from: jiayev
PR: doodlum/skyrim-community-shaders#0
File: :0-0
Timestamp: 2025-08-03T18:37:19.690Z
Learning: ISReflectionsRayTracing.hlsl and ISWorldMap.hlsl in the skyrim-community-shaders repository are image-space post-processing shaders that perform color sampling and blending operations that need proper linear color space handling for the linear lighting system. ISReflectionsRayTracing handles screen-space reflections and should use conditional Color::IrradianceToLinear/Gamma conversions similar to ISCompositeLensFlareVolumetricLighting.hlsl. ISWorldMap performs 7x7 color accumulation that should be done in linear space similar to the pattern used in ISSAOComposite.hlsl.
📚 Learning: default parameter values are supported in the hlsl compiler used by the skyrim-community-shaders pro...
Learnt from: alandtse
PR: doodlum/skyrim-community-shaders#577
File: features/Wetness Effects/Shaders/WetnessEffects/WetnessEffects.hlsli:57-61
Timestamp: 2025-06-17T05:40:22.785Z
Learning: Default parameter values are supported in the HLSL compiler used by the skyrim-community-shaders project, contrary to standard HLSL (FXC/DXC) limitations.
Applied to files:
README.mdCMakePresets.jsonCMakeLists.txt
📚 Learning: in the skyrim-community-shaders repository, file deletion error handling improvements that replace e...
Learnt from: alandtse
PR: doodlum/skyrim-community-shaders#0
File: :0-0
Timestamp: 2025-07-05T05:20:45.823Z
Learning: In the skyrim-community-shaders repository, file deletion error handling improvements that replace existence checks and try-catch blocks with std::filesystem::remove error-code-based approaches are considered bug fixes rather than refactoring, as they address inadequate error handling and misleading log messages.
Applied to files:
README.mdCMakePresets.jsonCMakeLists.txt
📚 Learning: isreflectionsraytracing.hlsl and isworldmap.hlsl in the skyrim-community-shaders repository are imag...
Learnt from: jiayev
PR: doodlum/skyrim-community-shaders#0
File: :0-0
Timestamp: 2025-08-03T18:37:19.690Z
Learning: ISReflectionsRayTracing.hlsl and ISWorldMap.hlsl in the skyrim-community-shaders repository are image-space post-processing shaders that perform color sampling and blending operations that need proper linear color space handling for the linear lighting system. ISReflectionsRayTracing handles screen-space reflections and should use conditional Color::IrradianceToLinear/Gamma conversions similar to ISCompositeLensFlareVolumetricLighting.hlsl. ISWorldMap performs 7x7 color accumulation that should be done in linear space similar to the pattern used in ISSAOComposite.hlsl.
Applied to files:
README.md
📚 Learning: when reviewing prs, always clarify the scope if there are multiple related features or dependencies....
Learnt from: alandtse
PR: doodlum/skyrim-community-shaders#0
File: :0-0
Timestamp: 2025-06-24T07:17:36.604Z
Learning: When reviewing PRs, always clarify the scope if there are multiple related features or dependencies. WeatherPicker was a separate PR that was already merged, while this PR focuses specifically on WetnessEffects climate preset system enhancements.
Applied to files:
CMakePresets.jsonCMakeLists.txt
📚 Learning: in the skyrim-community-shaders snowcover feature, the wstrtostr and strtowstr utility functions def...
Learnt from: ThePagi
PR: doodlum/skyrim-community-shaders#1369
File: src/Features/SnowCover.cpp:277-293
Timestamp: 2025-08-05T18:22:40.542Z
Learning: In the skyrim-community-shaders SnowCover feature, the wstrtostr and strtowstr utility functions defined in src/Features/SnowCover.cpp are unused and should be removed rather than fixed, as confirmed by the original author ThePagi.
Applied to files:
CMakeLists.txt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Validate shader compilation (VR, .github/configs/shader-validation-vr.yaml)
- GitHub Check: Build plugin and addons
- GitHub Check: Validate shader compilation (Flatrim, .github/configs/shader-validation.yaml)
🔇 Additional comments (1)
CMakePresets.json (1)
65-72: Build preset “ALL” description mismatches its behaviourThe description says it “generate(s) files in
<BUILD>/aio”, but the targets list contains"AIO"which currently produces files inside${AIO_DIR}only if the earlier bug is fixed.
After the bug above, the preset silently succeeds without producing an AIO folder, confusing users and CI.Re-test the preset once the
AIO_PACKAGEordering issue is resolved.
| COMMAND ${CMAKE_COMMAND} --install ${CMAKE_BINARY_DIR} --prefix ${FEATURE_PATH} --component SKSE | ||
| COMMAND ${CMAKE_COMMAND} -E copy_directory ${CORE_FEATURE_PATHS} ${FEATURE_PATH} | ||
| COMMAND ${CMAKE_COMMAND} -E rm -f -- ${FEATURE_PATH}/Core | ||
| COMMAND ${CMAKE_COMMAND} -E tar cfv ${CORE_PACKAGE} --format=7zip -- . |
There was a problem hiding this comment.
cmake -E rm -f cannot delete a directory
Core is a directory; use -rf or file(REMOVE_RECURSE …).
- COMMAND ${CMAKE_COMMAND} -E rm -f -- ${FEATURE_PATH}/Core
+ COMMAND ${CMAKE_COMMAND} -E rm -rf -- ${FEATURE_PATH}/CoreCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In CMakeLists.txt around lines 276 to 279, the command uses `cmake -E rm -f` to
delete the `Core` directory, but this only works for files, not directories.
Replace this command with `cmake -E rm -rf` to recursively force-remove the
directory, or alternatively use the CMake command `file(REMOVE_RECURSE
${FEATURE_PATH}/Core)` to properly delete the directory and its contents.
|
CMake's built-in archiving is horribly slow. Is there another cross-platform archiving method using pro-processor directives to select where the solution build is launched from, Windows or Linux? I found that starting with Windows 10 there is a PowerShell Compress-Archive method for archiving, only it archives in .zip format. But much faster than cmake -E tar. I tested the archiving speed and measured the time. To do this, I simply added another line in one .cpp with a message output to the logger so that the build would be rebuilt, and so on for each build target. Here are the archiving tests by time: |
|
Regarding archiving, please look at this. Is it suitable for us to use this generator? I see that it uses a parameter for multithreading and should be fast. |
|
Sorry, I mixed it up with archive formats. It's all about the differences between .7z and .zip formats. Packing in .7z will be slower in speed, but the output will be a smaller archive, with .zip, on the contrary, the archiving process will be faster, but we will get a larger archive. I compiled archives using CPake Archive Generator and for .7z I got an archive weighing 62 MB, for .zip 94 MB. |
|
@zndxcvbn I totally aggrege On another hand, I doubt if Github Action VM have many cores like developer's machine >_< -j may not help much if there are only one VM core. |
Automated formatting by clang-format, prettier, and other hooks. See https://pre-commit.ci for details.
|
Hi, @alandtse, wondering how can I test changes to the CI script? I changed the CI script to use the |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
CMakeLists.txt (4)
277-279: Directory removal usesrm -f– fails for directories
cmake -E rm -fonly removes files. Use-rfor delegate tofile(REMOVE_RECURSE …):- COMMAND ${CMAKE_COMMAND} -E rm -f -- ${FEATURE_PATH}/Core + COMMAND ${CMAKE_COMMAND} -E rm -rf -- ${FEATURE_PATH}/Core
206-208:file(REMOVE …)only deletes files, not directories
file(REMOVE \${CMAKE_INSTALL_PREFIX}/Core)silently fails on theCoredirectory.
Usefile(REMOVE_RECURSE …)instead.-install(CODE "file(REMOVE \${CMAKE_INSTALL_PREFIX}/Core)" COMPONENT Shaders) +install(CODE "file(REMOVE_RECURSE \${CMAKE_INSTALL_PREFIX}/Core)" COMPONENT Shaders)
265-266: Literal quote characters break path handling
list(APPEND CORE_FEATURE_PATHS "\"${FEATURE_PATH}/\"")stores quotes in the variable, producing paths like"C:/repo/…/"thatcmake -E copy_directorytreats as a single invalid argument, especially on Windows.Drop the manual quoting:
- list(APPEND CORE_FEATURE_PATHS "\"${FEATURE_PATH}/\"") + list(APPEND CORE_FEATURE_PATHS "${FEATURE_PATH}/")Apply the same fix in the later loop (Line 291).
203-204: Dangerous: wipes the entire install prefix recursively
install(CODE "file(REMOVE_RECURSE \${CMAKE_INSTALL_PREFIX})")deletes everything under whatever--prefixthe caller supplies, risking catastrophic data loss (e.g. a user passes--prefix "C:/ModOrganizer/mods/SomeOtherMod").Delete this line or restrict it to a project-owned sub-directory:
-install(CODE "file(REMOVE_RECURSE \${CMAKE_INSTALL_PREFIX})") +# DO NOT delete the whole prefix – limit to CommunityShaders subtree +# install(CODE "file(REMOVE_RECURSE \${CMAKE_INSTALL_PREFIX}/CommunityShaders)")
🧹 Nitpick comments (1)
CMakePresets.json (1)
66-101: Consider marking a clear default build presetDevelopers invoking
cmake --build --presetwith no preset name receive an error when multiple presets exist. Adding"default": trueto the recommended “Dev” preset avoids confusion.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
CMakeLists.txt(2 hunks)CMakePresets.json(1 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: alandtse
PR: doodlum/skyrim-community-shaders#0
File: :0-0
Timestamp: 2025-06-24T07:17:36.604Z
Learning: When reviewing PRs, always clarify the scope if there are multiple related features or dependencies. WeatherPicker was a separate PR that was already merged, while this PR focuses specifically on WetnessEffects climate preset system enhancements.
Learnt from: alandtse
PR: doodlum/skyrim-community-shaders#0
File: :0-0
Timestamp: 2025-07-05T05:20:45.823Z
Learning: In the skyrim-community-shaders repository, file deletion error handling improvements that replace existence checks and try-catch blocks with std::filesystem::remove error-code-based approaches are considered bug fixes rather than refactoring, as they address inadequate error handling and misleading log messages.
Learnt from: alandtse
PR: doodlum/skyrim-community-shaders#0
File: :0-0
Timestamp: 2025-06-29T00:27:42.717Z
Learning: When reviewing PRs, the committer's description should inform analysis of changes and may take precedence to conclusions when reasonable, but the choice of commit type should still be independently evaluated for compliance with conventional commits. Both committer intent and conventional commit standards should be considered to determine if the classification is defensible.
📚 Learning: 2025-07-05T05:20:45.823Z
Learnt from: alandtse
PR: doodlum/skyrim-community-shaders#0
File: :0-0
Timestamp: 2025-07-05T05:20:45.823Z
Learning: In the skyrim-community-shaders repository, file deletion error handling improvements that replace existence checks and try-catch blocks with std::filesystem::remove error-code-based approaches are considered bug fixes rather than refactoring, as they address inadequate error handling and misleading log messages.
Applied to files:
CMakePresets.jsonCMakeLists.txt
📚 Learning: 2025-06-24T07:17:36.604Z
Learnt from: alandtse
PR: doodlum/skyrim-community-shaders#0
File: :0-0
Timestamp: 2025-06-24T07:17:36.604Z
Learning: When reviewing PRs, always clarify the scope if there are multiple related features or dependencies. WeatherPicker was a separate PR that was already merged, while this PR focuses specifically on WetnessEffects climate preset system enhancements.
Applied to files:
CMakePresets.jsonCMakeLists.txt
📚 Learning: 2025-08-05T18:22:40.542Z
Learnt from: ThePagi
PR: doodlum/skyrim-community-shaders#1369
File: src/Features/SnowCover.cpp:277-293
Timestamp: 2025-08-05T18:22:40.542Z
Learning: In the skyrim-community-shaders SnowCover feature, the wstrtostr and strtowstr utility functions defined in src/Features/SnowCover.cpp are unused and should be removed rather than fixed, as confirmed by the original author ThePagi.
Applied to files:
CMakeLists.txt
📚 Learning: 2025-06-17T09:27:49.594Z
Learnt from: alandtse
PR: doodlum/skyrim-community-shaders#1157
File: src/Feature.cpp:42-49
Timestamp: 2025-06-17T09:27:49.594Z
Learning: In src/Feature.cpp, when an obsolete feature's INI file is deleted, the feature should be silently disabled without surfacing any issues to the user. This is the intended behavior because a deleted INI file for an obsolete feature indicates that the user has properly cleaned up the obsolete feature.
Applied to files:
CMakeLists.txt
📚 Learning: 2025-08-05T17:40:44.796Z
Learnt from: ThePagi
PR: doodlum/skyrim-community-shaders#1369
File: package/Shaders/Lighting.hlsl:0-0
Timestamp: 2025-08-05T17:40:44.796Z
Learning: In the skyrim-community-shaders repository, ultra trees (object LOD trees) are detected using a compound shader define condition `defined(DO_ALPHA_TEST) && defined(LOD_BLENDING) && defined(RIM_LIGHTING) && defined(SOFT_LIGHTING)` because "they have no define" according to the comment. The `ExtraFlags::IsTree` flag is used for different tree handling (AO removal in skylighting) and may not apply to ultra trees specifically. Before replacing the compound condition with `IsTree`, verification is needed to ensure the flag covers ultra trees.
Applied to files:
CMakeLists.txt
📚 Learning: 2025-06-17T05:40:22.785Z
Learnt from: alandtse
PR: doodlum/skyrim-community-shaders#577
File: features/Wetness Effects/Shaders/WetnessEffects/WetnessEffects.hlsli:57-61
Timestamp: 2025-06-17T05:40:22.785Z
Learning: Default parameter values are supported in the HLSL compiler used by the skyrim-community-shaders project, contrary to standard HLSL (FXC/DXC) limitations.
Applied to files:
CMakeLists.txt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Validate shader compilation (Flatrim, .github/configs/shader-validation.yaml)
- GitHub Check: Build plugin and addons
- GitHub Check: Validate shader compilation (VR, .github/configs/shader-validation-vr.yaml)
🔇 Additional comments (1)
CMakePresets.json (1)
55-63: Single “ALL” configure preset may break consumers relying on version-specific presetsAll previous
AE,SE,VRpresets vanished. If any scripts or CI jobs still invoke those names they will now fail outright.
Keep thin wrapper presets (that simplyinherits: "ALL"and toggle the relevantENABLE_SKYRIM_*flags) or update every downstream reference before merging.
|
✅ A pre-release build is available for this PR: |
It's building using the parameters for the cmake GitHub action. Run-cmake I believe. To test ci you may have to use a fork and not the main repo. |
|
I will test the build with different commands. If you have any recommendations for testing, that would be great. |
|
Is it possible to install 7zip in Github Workflow for archiving? It should be faster than |
Automated formatting by clang-format, prettier, and other hooks. See https://pre-commit.ci for details.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (7)
README.md (7)
71-72: Clarify wording: this produces a staging folder, not a zip package.To avoid confusion with the separate Package-* zip targets, call the output a “staging folder.”
Apply this diff:
-It should generate the AIO package in the `./build/ALL/aio` folder by default. +This will generate an AIO staging folder at `./build/ALL/aio` by default (zip archives are produced only via the `Package-*` targets).
101-101: Quote the install prefix to support paths with spaces.Windows paths commonly include spaces; quoting avoids failures.
Apply this diff:
-cmake --install ./build/ALL --prefix $MOD_FOLDER +cmake --install ./build/ALL --prefix "$MOD_FOLDER"
109-116: Grammar tweaks in package commands section.Minor readability and naming consistency.
Apply this diff:
-You can build zip packages for optional cmake targets. -Currently support `Package-AIO`, `Package-Core`, and `Package-<Feature>`: +You can build zip packages for optional CMake targets. +Currently supports `Package-AIO`, `Package-Core`, and `Package-<Feature>`: @@ -# Create a AIO package in ./dist/ +# Create an AIO package in ./dist/ cmake --build ./build/ALL --config Release --target Package-AIO -# Create a CommunityShaders core package in ./dist/ -cmake --build ./build/ALL --config Release --target Package-Core +# Create a Community Shaders core package in ./dist/ +cmake --build ./build/ALL --config Release --target Package-Core # Create a feature package in ./dist/ cmake --build ./build/ALL --config Release --target Package-GrassLighting
34-36: Fix grammar in the optional tools note.Apply this diff:
-CMake & Vcpkg comes with Visual Studio in Developer Command Prompts already. -Install them manually only if you want them in everywhere. +CMake and vcpkg come with Visual Studio’s Developer Command Prompts. +Install them manually only if you want them available system-wide.
133-137: Tighten wording around custom presets; fix article and punctuation.Apply this diff:
-When using custom preset you can call BuildRelease.bat with an parameter to specify which preset to configure eg: -`.\BuildRelease.bat ALL-WITH-AUTO-DEPLOYMENT` +When using a custom preset, you can call BuildRelease.bat with a parameter to specify which preset to configure (e.g.): +`.\BuildRelease.bat ALL-WITH-AUTO-DEPLOYMENT`
156-158: Improve wording in Docker follow-up step.Apply this diff:
-5. In subsequent builds only run the build step (3.) +5. For subsequent builds, only rerun step 3.
66-77: Confirm CI expectations and document CI usage.Given the refactor to install- and target-based packaging, CI may need to switch from building the default target to invoking
Package-AIOexplicitly. Add a short “CI usage” snippet to avoid breaking downstream automation.Proposed addition (below this subsection):
CI example (GitHub Actions): - name: Configure run: cmake --preset=ALL - name: Build AIO run: cmake --build --preset ALL --config Release --target Package-AIO - name: Artifact run: dir distIf CI must preserve old behavior, consider a transitional preset/target alias (e.g., keep
ALLbuilding Package-AIO in CI only), and document it here.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
CMakeLists.txt(2 hunks)README.md(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- CMakeLists.txt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build plugin and addons
🔇 Additional comments (1)
README.md (1)
40-41: Specify supported CMake versions precisely.“CMake 4.0+ is not supported right now” is speculative and not actionable. Please state the minimum required version and the versions you test against (e.g., “Requires CMake ≥ 3.x; validated with 3.x–3.y”).
Would you like me to scan CMakeLists.txt/CMakePresets.json to infer the minimum CMake version (e.g., via
cmake_minimum_requiredand preset schema version)?
alandtse
left a comment
alandtse
left a comment
There was a problem hiding this comment.
- Please make sure you review the bot suggestions and resolve them after you've thought about it. The bot could be wrong, but sometimes it's actually right.
- Note, not all of us us MO2.
- Some of us deploy to multiple versions of skyrim so need the deployment scripts to work into multiple directories without further copying. I didn't double check copy_shaders, but that is used by some of us to automatically rebuild things in game. If it's not able to copy into a live skyrim folder, it's not working.
- All commands should work from command line including to generate features and the aio we distribute for testing. I never log into Visual Studio so will not see the install targets. The CI always builds via CLI and needs to generate all feature zips and aio zip.
Brings PR community-shaders#1372 up to date with upstream/dev (v1.4.6), integrating install target improvements with upstream's deployment system. Changes from PR community-shaders#1372: - Add CMake install() infrastructure for manual deployments - Add Package-Core, Package-AIO-Manual, and Package-<Feature> targets - Support cmake --install with custom --prefix - Set CMAKE_INSTALL_PREFIX default to build/aio - Add CMake policy CMP0116 settings Changes from upstream/dev (v1.4.6): - Add directx-headers, XeSS-SDK, Detours dependencies - Implement incremental copy system with copy_if_different - Add Windows robocopy integration for efficient deployment - Add PREPARE_AIO target with intelligent dependency tracking - Improve shader copying to preserve timestamps - Multiple feature improvements and bug fixes Conflict resolution: - Unified AIO_DIR definition - Kept ZIP_TO_DIST and AIO_ZIP_TO_DIST options - Added install() commands alongside existing POST_BUILD system - Maintained prepare_shaders target for CI - AUTO_PLUGIN_DEPLOYMENT uses upstream robocopy approach Tested: - cmake --preset ALL configuration - cmake --build build/ALL - prepare_shaders target (CI critical) - Package-Core target - cmake --install with custom --prefix
Create a stable stamp file for the automatic AIO archive so CMake
only rebuilds the archive when inputs change. The AIO tar step now
writes ${CMAKE_CURRENT_BINARY_DIR}/aio_package.stamp and
`AIO_ZIP_PACKAGE` depends on that stamp while preserving the UTC
timestamp in the archive filename.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
README.md (1)
82-102: Resolve CMake invocation issues from previous review feedback.Two critical issues remain unresolved from past review comments:
Lines 82–86: Calling
vcvarsall.batfrom PowerShell runs in a childcmd.exeprocess, so environment changes don't persist to your PowerShell session. Use the pre-configured Developer PowerShell or Native Tools Command Prompt instead.Line 95: Cannot mix a build directory (
./build/ALL) and a preset (--preset ALL) in a singlecmake --buildinvocation. Use the preset-only form.Apply this diff:
-```pwsh -# Same as running `x64 Native Tools Command Prompt` -# Adding cl.exe and cmake.exe into your PATH -& "C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Auxiliary\Build\vcvarsall.bat" amd64 - -# Change to the repository folder -cd skyrim-community-shaders - -# Generate the build files in ./build/ALL -cmake -S . --preset=ALL -B ./build/ALL - -# Invoke build tool to build the C++ source code -# This also generate an AIO folder in ./build/ALL/aio -cmake --build ./build/ALL --preset ALL +```pwsh +# Run from: Developer PowerShell for VS 2022, or x64 Native Tools Command Prompt for VS 2022 +# (These shells already have cl.exe and cmake.exe on PATH.) + +# Change to the repository folder +cd skyrim-community-shaders + +# Generate the build files in ./build/ALL +cmake -S . --preset=ALL -B ./build/ALL + +# Invoke build tool to build the C++ source code +# This also generates an AIO folder in ./build/ALL/aio +cmake --build --preset ALLCMakeLists.txt (4)
289-289: Usefile(REMOVE_RECURSE)to delete the Core directory, notfile(REMOVE).
file(REMOVE)only removes files; it cannot delete directories. The Core directory will remain in the AIO tree and end up in packaged archives.Apply this diff:
-install(CODE "file(REMOVE \${CMAKE_INSTALL_PREFIX}/Core)" COMPONENT Shaders) +install(CODE "file(REMOVE_RECURSE \${CMAKE_INSTALL_PREFIX}/Core)" COMPONENT Shaders)
755-765: Replace target name with file generator expression in build dependencies.Line 755 appends the bare target name
${PROJECT_NAME}toCORE_SOURCES, which is later used inDEPENDSclauses. CMake expects file paths or generator expressions, not bare target names.Apply this diff:
-# Add SKSE plugin dll as dependency -list(APPEND CORE_SOURCES ${PROJECT_NAME}) +# Add SKSE plugin dll as dependency (use generator expression for actual output file) +list(APPEND CORE_SOURCES $<TARGET_FILE:${PROJECT_NAME}>)
277-289: CRITICAL: Prevent dangerous wholesale deletion of install prefix.Line 277 uses
file(REMOVE_RECURSE ${CMAKE_INSTALL_PREFIX})which blindly deletes everything under whatever--prefixthe user passes tocmake --install. If a user inadvertently points--prefixto an existing Mod Organizer folder, a staging directory with unrelated data, or even/usr/local, that entire directory is erased without confirmation.This is a critical security and data-loss risk. Scope deletion to a known subdirectory or remove it entirely:
-install(CODE "file(REMOVE_RECURSE \${CMAKE_INSTALL_PREFIX})") +# Scope cleanup to this project's subdirectory to prevent accidental data loss +install(CODE "file(REMOVE_RECURSE \${CMAKE_INSTALL_PREFIX}/CommunityShaders)" COMPONENT SKSE)Alternatively, if cleanup is not essential, simply remove this line and rely on the user providing a clean or managed prefix directory.
778-780: Userm -rf(orfile(REMOVE_RECURSE)) to delete the Core directory.
cmake -E rm -fonly removes files; it cannot recursively delete directories. The-fflag does not make it recursive.Apply this diff:
COMMAND ${CMAKE_COMMAND} -E copy_directory ${CORE_FEATURE_PATHS} ${FEATURE_PATH} - COMMAND ${CMAKE_COMMAND} -E rm -f -- ${FEATURE_PATH}/Core + COMMAND ${CMAKE_COMMAND} -E rm -rf -- ${FEATURE_PATH}/Core COMMAND ${CMAKE_COMMAND} -E tar cfv ${CORE_PACKAGE} --format=7zip -- .
🧹 Nitpick comments (5)
README.md (3)
33-36: Add language identifier to fenced code block.The code block lacks a language specifier for syntax highlighting and linting compliance.
Apply this diff:
-``` -CMake & Vcpkg comes with Visual Studio in Developer Command Prompts already. +```note +CMake & Vcpkg comes with Visual Studio in Developer Command Prompts already.
26-40: Fix list indentation to match Markdown style guide (MD007).Lists should use 2-space indentation, not 4. Apply this diff:
- - CMake Tools for Windows - - HLSL Tools -- [Git](https://git-scm.com/downloads) - - Edit the `PATH` environment variable and add the Git.exe install path as a new value + - CMake Tools for Windows + - HLSL Tools + - [Git](https://git-scm.com/downloads) + - Edit the `PATH` environment variable and add the Git.exe install path as a new valueAnd apply the same 2-space indentation to lines 39-40 under Optional Requirements.
75-76: Use hyphenated adjective "right-click".For consistency and grammatical correctness, hyphenate compound adjectives before a noun.
Apply this diff:
-If you change the `Solution Explorer` into `CMake Targets View`, you can find optional targets to create zip packages for each feature. -Right click on the target and select `Build` to create the zip package in `./dist/`. +If you change the `Solution Explorer` into `CMake Targets View`, you can find optional targets to create zip packages for each feature. +Right-click on the target and select `Build` to create the zip package in `./dist/`..claude/CLAUDE.md (1)
53-55: Add language specifier to code block for consistency.Line 53's fenced code block should specify a language identifier.
Apply this diff:
-``` +``` CommunityShadersOutputDir=F:/MySkyrimModpack/mods/CommunityShaders;F:/SteamLibrary/steamapps/common/SkyrimVR/Data;F:/SteamLibrary/steamapps/common/Skyrim Special Edition/Data -``` +```CMakeLists.txt (1)
2-10: CMake 4.0+ version guard is reasonable but consider a softer approach.The hard ERROR for CMake 4.0+ will prevent users with newer CMake from even configuring the project. This is a strong stance. Consider:
- Keep as-is if EASTL truly cannot build with CMake 4.0+
- Or downgrade to WARNING and skip the EASTL find_package only if CMake 4.0+ is detected
The current approach is defensible for now (to prevent mysterious build failures), but plan to revisit once EASTL/vcpkg port is fixed.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.claude/CLAUDE.md(2 hunks)AI-INSTRUCTIONS.md(1 hunks)CMakeLists.txt(4 hunks)CMakePresets.json(1 hunks)README.md(3 hunks)
🧰 Additional context used
🧠 Learnings (8)
📚 Learning: 2025-07-05T05:20:45.823Z
Learnt from: alandtse
Repo: doodlum/skyrim-community-shaders PR: 0
File: :0-0
Timestamp: 2025-07-05T05:20:45.823Z
Learning: In the skyrim-community-shaders repository, file deletion error handling improvements that replace existence checks and try-catch blocks with std::filesystem::remove error-code-based approaches are considered bug fixes rather than refactoring, as they address inadequate error handling and misleading log messages.
Applied to files:
.claude/CLAUDE.mdCMakeLists.txt
📚 Learning: 2025-08-17T18:37:35.839Z
Learnt from: CR
Repo: doodlum/skyrim-community-shaders PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T18:37:35.839Z
Learning: Always consult .claude/CLAUDE.md for complete development guidance and AI-INSTRUCTIONS.md for quick reference before seeking other sources
Applied to files:
.claude/CLAUDE.mdAI-INSTRUCTIONS.md
📚 Learning: 2025-06-17T05:40:22.785Z
Learnt from: alandtse
Repo: doodlum/skyrim-community-shaders PR: 577
File: features/Wetness Effects/Shaders/WetnessEffects/WetnessEffects.hlsli:57-61
Timestamp: 2025-06-17T05:40:22.785Z
Learning: Default parameter values are supported in the HLSL compiler used by the skyrim-community-shaders project, contrary to standard HLSL (FXC/DXC) limitations.
Applied to files:
.claude/CLAUDE.mdAI-INSTRUCTIONS.mdREADME.md
📚 Learning: 2025-08-17T18:37:35.839Z
Learnt from: CR
Repo: doodlum/skyrim-community-shaders PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T18:37:35.839Z
Learning: On Windows, use BuildRelease.bat ALL as the primary build preset; do not cancel builds or shader validation
Applied to files:
.claude/CLAUDE.mdAI-INSTRUCTIONS.mdREADME.md
📚 Learning: 2025-10-02T14:20:33.454Z
Learnt from: ThePagi
Repo: doodlum/skyrim-community-shaders PR: 1369
File: src/Features/SnowCover.cpp:515-515
Timestamp: 2025-10-02T14:20:33.454Z
Learning: In the Community Shaders codebase (skyrim-community-shaders repository), hardcoded shader resource slot numbers are used directly in code rather than being defined as named constants. This is the established convention and should not be flagged as an issue.
Applied to files:
.claude/CLAUDE.mdCMakeLists.txt
📚 Learning: 2025-08-17T18:37:35.839Z
Learnt from: CR
Repo: doodlum/skyrim-community-shaders PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T18:37:35.839Z
Learning: Applies to features/*/Shaders/**/*.{hlsl,hlsli,fx,fxh} : Place all feature shaders under features/YourFeature/Shaders/
Applied to files:
.claude/CLAUDE.mdREADME.md
📚 Learning: 2025-08-17T18:37:35.839Z
Learnt from: CR
Repo: doodlum/skyrim-community-shaders PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T18:37:35.839Z
Learning: Applies to features/*/Shaders/**/*.{hlsl,hlsli,fx,fxh} : Avoid GPU register/buffer conflicts in HLSL; verify register usage (e.g., with hlslkit buffer scanning)
Applied to files:
CMakeLists.txt
📚 Learning: 2025-06-24T07:17:36.604Z
Learnt from: alandtse
Repo: doodlum/skyrim-community-shaders PR: 0
File: :0-0
Timestamp: 2025-06-24T07:17:36.604Z
Learning: When reviewing PRs, always clarify the scope if there are multiple related features or dependencies. WeatherPicker was a separate PR that was already merged, while this PR focuses specifically on WetnessEffects climate preset system enhancements.
Applied to files:
CMakeLists.txt
🪛 LanguageTool
README.md
[grammar] ~76-~76: Use a hyphen to join words.
Context: ...ate zip packages for each feature. Right click on the target and select Build t...
(QB_NEW_EN_HYPHEN)
🪛 markdownlint-cli2 (0.18.1)
.claude/CLAUDE.md
53-53: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
README.md
26-26: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
27-27: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
29-29: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
33-33: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
39-39: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
40-40: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build plugin and addons
🔇 Additional comments (6)
AI-INSTRUCTIONS.md (1)
29-48: Well-structured documentation addition—no concerns.The Build Options section appropriately cross-references
.claude/CLAUDE.mdfor detailed guidance while providing quick reference links to key targets and options.CMakePresets.json (1)
65-101: Build presets restructuring is well-designed.The new preset organization (Dev, Package, Shaders, Debug) clearly separates development workflows from CI/packaging workflows. Descriptions are helpful, and the configurations align with the PR's goal of making packaging explicit rather than automatic.
.claude/CLAUDE.md (1)
122-156: Excellent documentation of manual packaging targets.The detailed section clearly explains the purpose of each target (Core, Feature, AIO-Manual, AIO), their outputs, and the install-based packaging approach. This greatly aids users in understanding the new packaging workflow.
CMakeLists.txt (3)
710-731: AIO packaging logic with stamp files is well-structured.The approach of using a stamp file as OUTPUT (line 721) while keeping the archive filename timestamped is clever and avoids unnecessary rebuilds. The comment clearly explains the pattern. This design aligns well with the PR's goal of making packaging efficient and explicit.
295-514: Incremental AIO preparation system is well-designed.The conditional block (lines 295–514) implementing incremental file copying with
copy_if_differentis a major improvement over wholesale directory copies. This reduces rebuild times significantly. The logic is sound:
- Gathers package and feature files as inputs
- Uses individual
copy_if_differentcommands to preserve timestamps of unchanged files- Separates shader copying into its own target (
prepare_shaders)- Supports CI validation workflows
This architecture directly addresses the PR's ~5-minute build-time issue.
833-843: User-facing messaging is clear and helpful.The final message summarizes the new packaging workflow, directs users to switch to the Dev preset for faster iteration, and explains how to use
cmake --install. This guidance is exactly what new users need after the restructuring.
- Replace "\"${FEATURE_PATH}/\"" with "${FEATURE_PATH}/" so feature paths are normal strings during packaging.
- Update README: prefer Developer PowerShell/x64 Native Tools prompt; use presets where appropriate and fix typos.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
CMakeLists.txt (1)
277-289: Line 277 and 289: Dangerous install operations still present — require scoped deletion and proper directory removal.The dangerous
file(REMOVE_RECURSE ${CMAKE_INSTALL_PREFIX})at line 277 still blindly deletes the entire install prefix. If a user sets--prefixto an existing data directory or system path, the directory is erased entirely. Additionally, line 289 still usesfile(REMOVE ...)instead offile(REMOVE_RECURSE ...), so the Core directory deletion will fail (REMOVE only deletes files, not directories).Commit 973bd01 addressed AIO_DIR initialization timing but did not resolve these install issues. Required fixes:
- Line 277: Scope deletion to a known subdirectory (e.g.,
${CMAKE_INSTALL_PREFIX}/CommunityShaders) or remove entirely.- Line 289: Replace
file(REMOVE ...)withfile(REMOVE_RECURSE ...)to properly delete directories.
🧹 Nitpick comments (4)
README.md (4)
26-29: Fix unordered list indentation to match Markdown lint standards.The static analysis tool flagged list indentation inconsistencies. Markdown expects 2-space indentation for nested list items, but several lines use 4-space indentation:
- Lines 26–27: CMake Tools, HLSL Tools (4 spaces → 2)
- Line 29: Git PATH setup (4 spaces → 2)
- Lines 39–40: CMake manual install notes (4 spaces → 2)
Apply consistent 2-space indentation throughout the Requirements and Optional Requirements sections.
Also applies to: 39-40
100-100: Fix heading level hierarchy.Line 100 uses a level-4 heading (
####) but should use level-3 (###) to maintain proper Markdown hierarchy after the level-3 "Advanced build with CMake in command line" heading at line 78. This was flagged by static analysis (MD001).-#### Build a zip package +### Build a zip package
33-36: Add language identifier to fenced code block.Line 33 opens a fenced code block without specifying a language, which affects syntax highlighting and linting. Add an appropriate language tag:
-``` +```cmake CMake & Vcpkg comes with Visual Studio in Developer Command Prompts already. Install them manually only if you want them in everywhere. -``` +```
75-76: Use hyphenated form for compound modifier "right-click".Line 76 uses "Right click" as a compound modifier; standard grammar requires a hyphen when such compound adjectives precede a noun:
-Right click on the target and select `Build` to create the zip package in `./dist/`. +Right-click on the target and select `Build` to create the zip package in `./dist/`.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
CMakeLists.txt(4 hunks)README.md(3 hunks)
🧰 Additional context used
🧠 Learnings (7)
📚 Learning: 2025-08-17T18:37:35.839Z
Learnt from: CR
Repo: doodlum/skyrim-community-shaders PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T18:37:35.839Z
Learning: Applies to features/*/Shaders/**/*.{hlsl,hlsli,fx,fxh} : Avoid GPU register/buffer conflicts in HLSL; verify register usage (e.g., with hlslkit buffer scanning)
Applied to files:
CMakeLists.txt
📚 Learning: 2025-07-05T05:20:45.823Z
Learnt from: alandtse
Repo: doodlum/skyrim-community-shaders PR: 0
File: :0-0
Timestamp: 2025-07-05T05:20:45.823Z
Learning: In the skyrim-community-shaders repository, file deletion error handling improvements that replace existence checks and try-catch blocks with std::filesystem::remove error-code-based approaches are considered bug fixes rather than refactoring, as they address inadequate error handling and misleading log messages.
Applied to files:
CMakeLists.txt
📚 Learning: 2025-06-24T07:17:36.604Z
Learnt from: alandtse
Repo: doodlum/skyrim-community-shaders PR: 0
File: :0-0
Timestamp: 2025-06-24T07:17:36.604Z
Learning: When reviewing PRs, always clarify the scope if there are multiple related features or dependencies. WeatherPicker was a separate PR that was already merged, while this PR focuses specifically on WetnessEffects climate preset system enhancements.
Applied to files:
CMakeLists.txt
📚 Learning: 2025-10-02T14:20:33.454Z
Learnt from: ThePagi
Repo: doodlum/skyrim-community-shaders PR: 1369
File: src/Features/SnowCover.cpp:515-515
Timestamp: 2025-10-02T14:20:33.454Z
Learning: In the Community Shaders codebase (skyrim-community-shaders repository), hardcoded shader resource slot numbers are used directly in code rather than being defined as named constants. This is the established convention and should not be flagged as an issue.
Applied to files:
CMakeLists.txt
📚 Learning: 2025-08-17T18:37:35.839Z
Learnt from: CR
Repo: doodlum/skyrim-community-shaders PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T18:37:35.839Z
Learning: On Windows, use BuildRelease.bat ALL as the primary build preset; do not cancel builds or shader validation
Applied to files:
README.md
📚 Learning: 2025-06-17T05:40:22.785Z
Learnt from: alandtse
Repo: doodlum/skyrim-community-shaders PR: 577
File: features/Wetness Effects/Shaders/WetnessEffects/WetnessEffects.hlsli:57-61
Timestamp: 2025-06-17T05:40:22.785Z
Learning: Default parameter values are supported in the HLSL compiler used by the skyrim-community-shaders project, contrary to standard HLSL (FXC/DXC) limitations.
Applied to files:
README.md
📚 Learning: 2025-08-17T18:37:35.839Z
Learnt from: CR
Repo: doodlum/skyrim-community-shaders PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T18:37:35.839Z
Learning: Applies to features/*/Shaders/**/*.{hlsl,hlsli,fx,fxh} : Place all feature shaders under features/YourFeature/Shaders/
Applied to files:
README.md
🪛 LanguageTool
README.md
[grammar] ~76-~76: Use a hyphen to join words.
Context: ...ate zip packages for each feature. Right click on the target and select Build t...
(QB_NEW_EN_HYPHEN)
🪛 markdownlint-cli2 (0.18.1)
README.md
26-26: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
27-27: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
29-29: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
33-33: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
39-39: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
40-40: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
100-100: Heading levels should only increment by one level at a time
Expected: h2; Actual: h4
(MD001, heading-increment)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build plugin and addons
🔇 Additional comments (4)
CMakeLists.txt (2)
437-514: Shader preparation and CI validation targets look solid.The incremental shader copy system using
copy_if_differentand stamp-based tracking is well-designed. The standaloneprepare_shaderstarget (Line 509) provides a clean way for CI to validate shaders without waiting for the full build, which aligns well with the PR goals.
833-843: User guidance messages are clear and helpful.The configuration summary provides good direction for users on the new packaging targets and development workflows. Messages align well with the README updates and preset changes.
README.md (2)
137-140: Clarify or remove BuildRelease.bat reference.Lines 137–140 reference a
./BuildRelease.batfile as a convenience wrapper, but past reviews indicated this file does not exist in the repository. Either:
- Add
BuildRelease.batat the repo root with the documented cmake and cmake --build commands, and update line 138 with its correct path, or- Remove the reference to
./BuildRelease.batand keep only the inline command examples above.
171-195: New Debugging section is well-structured and helpful.The addition of MO2-SKSE launching and RenderDoc capture instructions provides valuable guidance for developers. Instructions are clear and follow best practices for SKSE debugging workflows.
- Replace literal project name in CORE_SOURCES with "$<TARGET_FILE:${PROJECT_NAME}>" so add_custom_command DEPENDS references the actual DLL path at build time.
* chore(ui): update discord banner (community-shaders#1493) * fix: use proper filename settingsuser.json (community-shaders#1491) * chore(upscaling): increase fsr sharpness * chore: rename d3d12interop to d3d12SwapChainActive (community-shaders#1494) * feat(llf): remove particle lights (community-shaders#1495) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * feat(llf): move llf to core (community-shaders#1496) * fix: remove water clamp (community-shaders#1497) * fix(upscaling): more upscaling fixes (community-shaders#1498) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * fix: fix some internal errors when debugging (community-shaders#1500) * fix(ui): fix save settings conflicts & welcome screen (community-shaders#1501) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * fix(ui): add constraints for discord banner size (community-shaders#1463) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: doodlum <15017472+doodlum@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * fix(VR): fix exiting menu using controllers (community-shaders#1502) * build: fix warnings (community-shaders#1505) * feat(UI): allow tooltips for disabled elements (community-shaders#1503) * feat(upscaling): add downscale percentages (community-shaders#1506) * perf(ssgi): optimize (community-shaders#1499) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * feat(ui): font size and perf overlay improvements (community-shaders#1511) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * chore: remove unused hooks (community-shaders#1510) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * fix: adjust IsInterior to consider kNoSky or kFixedDimensions flags (community-shaders#1512) * fix(hair): correct hair indirect normal, marschner by default (community-shaders#1515) Co-authored-by: doodlum <15017472+doodlum@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * chore: mostly revert ISHDR to 1.3.6 (community-shaders#1516) * chore(upscaling): simplify interop and upscale methods (community-shaders#1514) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * fix(hair): typo in code (community-shaders#1517) * feat(ibl): lerp sky ibl using skylighting (community-shaders#1519) * fix(sss): burley artifacts with effect blend (community-shaders#1518) * fix(upscaling): fix screenshots when upscaling enabled (community-shaders#1520) * fix(upscaling): fix mipbias sometimes being wrong (community-shaders#1521) * fix: fix compile error if snow shader on (community-shaders#1522) * chore(upscaling): revert fsr to typical settings (community-shaders#1523) * fix: fix minor ui issues (community-shaders#1524) * chore(grass collision): simpler grass collision (community-shaders#1525) * fix: update skylighting and version * fix(pbr): fix inconsistencies (community-shaders#1526) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: jiayev <l936249247@hotmail.com> * feat(upscaling): sharpening slider (community-shaders#1527) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * chore: bump versions * fix(ibl): add ibl to reflection normalization (community-shaders#1528) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * fix(hair): remove pbr lighting mult for hair (community-shaders#1531) * chore(upscaling): add back upscale multiplier (community-shaders#1532) * fix(upscaling): fix minor upscaling issues (community-shaders#1536) * chore: gamma space normalisation (community-shaders#1535) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * feat(grass collision): implement with texture and history (community-shaders#1539) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * chore(grass collision): less aggressive (community-shaders#1546) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * fix(skylighting): fix cell id casting (community-shaders#1544) * chore(emat): auto detect terrain parallax (community-shaders#1545) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * chore: update versions * feat(VR): enable upscaling (community-shaders#1507) * fix(terrain shadows): fix brightened lods (community-shaders#1547) * chore(upscaling): reduce ghosting near camera (community-shaders#1548) * fix: fix grass not animating (community-shaders#1549) Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * fix(grass collision): fix non-standard timescales (community-shaders#1550) * build: deploy only updated files (community-shaders#1556) * feat: add Clear Shader Cache to Advanced (community-shaders#1555) * chore(featureissues): default collapse testing menu (community-shaders#1554) * fix(VR): use only supported shaders from cache (community-shaders#1553) Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * build: use gersemi cmake formatter (community-shaders#1557) * fix(terrain): vanilla diffuse in pbr terrain cell too bright due to wrong color space (community-shaders#1558) * docs: add new feature development template guide (community-shaders#1529) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * docs(UI): remove duplicate GPL license statement (community-shaders#1561) * feat: add renderdoc for debugging (community-shaders#1560) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: alandtse <7086117+alandtse@users.noreply.github.com> Co-authored-by: Alan Tse <alandtse@gmail.com> * fix(ui): welcome popup size issues (community-shaders#1573) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * chore(grass collision): minor tweaks (community-shaders#1568) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * fix(terrain helper): fix conflicting bit (community-shaders#1566) Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * feat(UI): separate theme settings, UI refactor, font support (community-shaders#1571) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * chore: bump versions * build: fix zipping aio (community-shaders#1579) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * fix(grass collision): clamp maximum depth of grass (community-shaders#1578) * feat(UI): enhance shader blocking (community-shaders#1564) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: alandtse <7086117+alandtse@users.noreply.github.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Alan Tse <alandtse@gmail.com> * fix: remove duplicate buffer setup (community-shaders#1586) * feat: update shader compile elapsed time every second (community-shaders#1587) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * build: add cmake install commands (community-shaders#1372) * feat(perf-overlay): add size controls (community-shaders#1591) * fix(perf-overlay): fix infinite draw calls table height (community-shaders#1590) * refactor(perf-overlay): remove collapsible headers (community-shaders#1572) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * fix(perf-overlay): removed ImGuiTableFlags_ScrollX/Y for scroll bar issues (community-shaders#1594) * build: fix shader copying to relative paths (community-shaders#1603) * fix(ibl): apply ibl to cubemap normalisation for non deferred (community-shaders#1604) * fix(grass): use correct light direction (community-shaders#1602) * fix(welcome-popup): adjust font size & window spacing (community-shaders#1592) * feat(lod): add gamma sliders (community-shaders#1588) * build: correct CodeRabbit schema syntax (community-shaders#1608) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: alandtse <7086117+alandtse@users.noreply.github.com> * build: add compile-time validation of GPU buffers (community-shaders#1427) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: alandtse <7086117+alandtse@users.noreply.github.com> * ci: run shader validation on CMake and CI config changes (community-shaders#1606) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: alandtse <7086117+alandtse@users.noreply.github.com> * feat: procedural sun * limb darkening * another darkening * build(deps): remove orphaned Intel XeSS dependency (community-shaders#1611) Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> * fix: accumulate sunlight color in pixel shader output * fix(ui): enter key now behaves properly when first time popup is open (community-shaders#1615) * feat(ui): add tabs to advanced settings & PBR search (community-shaders#1599) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * build: add HLSL intellisense (community-shaders#1614) * refactor(UI): move light limit visualization into debug (community-shaders#1619) * refactor(ui): add settings for shader block hotkeys (community-shaders#1624) Co-authored-by: Bruce <44987693+brucenguyen@users.noreply.github.com> * fix(ui): anchor reset settings button position (community-shaders#1621) Co-authored-by: Giovanni Correia <Gistix@users.noreply.github.com> * fix(hair): use indirect normal for deferred marschner hair (community-shaders#1626) * build: fix Package-AIO-Manual for fresh pulls (community-shaders#1625) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * fix(snow): use world space vectors (community-shaders#1618) * feat(UI): add gaussian blur shader core files (community-shaders#1595) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * feat(ui): add test conditions button (community-shaders#1637) * fix(ui): blocked shader info overflow in Shader Debug tab (community-shaders#1632) * fix(upscaling): replace NIS with RCAS for DLSS (community-shaders#1620) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * fix(dynamic cubemaps): add a check for timeskip (community-shaders#1639) * refactor: restructure lighting (community-shaders#1633) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * feat(ui): add themes & fonts (community-shaders#1596) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * feat(water): add flowmap parallax (community-shaders#1636) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * fix cloud shadow setting saving --------- Co-authored-by: zxcvbn <66063766+zndxcvbn@users.noreply.github.com> Co-authored-by: davo0411 <davidkehoe0411@outlook.com> Co-authored-by: doodlum <15017472+doodlum@users.noreply.github.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Alan Tse <alandtse@users.noreply.github.com> Co-authored-by: soda <130315225+soda3000@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: ThePagi <32794457+ThePagi@users.noreply.github.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: alandtse <7086117+alandtse@users.noreply.github.com> Co-authored-by: Alan Tse <alandtse@gmail.com> Co-authored-by: Yupeng Zhang <ArcEarth@outlook.com> Co-authored-by: kuplion <kuplion@hotmail.com> Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> Co-authored-by: Giovanni Correia <Gistix@users.noreply.github.com> Co-authored-by: Bruce <44987693+brucenguyen@users.noreply.github.com> Co-authored-by: Midona <106106405+midona-rhel@users.noreply.github.com>
3 participants
The project is currently spending 5 minutes every time I press the build button, after linking CommunityShaders.dll, without any prompt.
Turns out lot of this was the cost from all the
POST_BUILD`` commands attached to CommunityShaders, withZIP_TO_DISTdefault toON`.This change splitted the expensive zip packaging command into seperate Package-XXX targets, where developers can manually build before release.
Beside, this change also added proper
cmake --installsupport. You can use it to install the AIO package into any folder withcmake --install ./build/ALL --prefix <TARGET>command.Finally,
COMMENTShave been added to all commands, prompting what is running, building, copying or zipping.With all these change, the default preset can now finish an incremental build in about 10 seconds instead of 5 minutes.
The Package targets in Visual Studio CMake Targets View:
Incremental build now takes ~10 seconds to complete (with AIO folder update), and show prompts:
Summary by CodeRabbit
New Features
Chores
Documentation