feat(core): Add path helper utilities for theme/font discovery#1569
feat(core): Add path helper utilities for theme/font discovery#1569davo0411 wants to merge 1 commit into
Conversation
- Add PathHelpers namespace with GetSettingsTheme, GetThemesPath, GetFontsPath - Remove hardcoded path constants from State and SettingsOverrideManager - Add THEME config mode to State for theme file management - Add GetThemesRealPath for accessing theme directory on disk - Add stub LoadTheme/SaveTheme methods (full implementation in PR #4) This provides the foundation for the theme system by centralizing path management and removing hardcoded strings throughout the codebase. Part of UI modernization effort - PR #1 of 8 Related: - Blocks: PR #2 (Core UI Utilities) - See: PR_1530_SPLIT_PLAN.md for full breakdown Testing: - Verify existing configs still load correctly - Check that override paths resolve properly
WalkthroughCentralizes path resolution via Util::PathHelpers across settings/overrides and state management, removes hardcoded path constants, adds theme-related paths and stub LoadTheme/SaveTheme methods, and updates GetConfigPath to return a value. Introduces new PathHelpers for themes and features and adjusts file operations to use derived parent directories. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor App
participant State
participant PathHelpers as Util::PathHelpers
participant FS as File System
App->>State: Save(configMode)
State->>State: configPath = GetConfigPath(mode)
State->>FS: derive parent directory, ensure exists
State->>FS: open/write file at configPath
FS-->>State: success/failure
State-->>App: result
rect rgba(230,245,255,0.6)
note over State,PathHelpers: New/changed interaction
State->>PathHelpers: GetSettings...Path() based on mode (USER/DEFAULT/THEME/TEST)
PathHelpers-->>State: std::filesystem::path
end
sequenceDiagram
autonumber
actor App
participant Overrides as SettingsOverrideManager
participant PathHelpers as Util::PathHelpers
App->>Overrides: ApplyOverrides()
Overrides->>PathHelpers: GetOverridesPath()
Overrides->>PathHelpers: GetAppliedOverridesPath()
PathHelpers-->>Overrides: paths
Overrides-->>App: proceed with paths
sequenceDiagram
autonumber
actor App
participant State
participant PathHelpers as Util::PathHelpers
App->>State: LoadTheme()/SaveTheme()
State->>PathHelpers: GetSettingsThemePath(), GetThemesPath()
PathHelpers-->>State: paths
State-->>App: stub return (logs)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/SettingsOverrideManager.cpp(3 hunks)src/SettingsOverrideManager.h(0 hunks)src/State.cpp(3 hunks)src/State.h(2 hunks)src/Utils/FileSystem.cpp(2 hunks)src/Utils/FileSystem.h(2 hunks)
💤 Files with no reviewable changes (1)
- src/SettingsOverrideManager.h
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{cpp,cxx,cc,c,h,hpp,hxx,hlsl,hlsli,fx,fxh,py}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not include TODO/FIXME placeholders; provide complete, working solutions
Files:
src/SettingsOverrideManager.cppsrc/State.cppsrc/State.hsrc/Utils/FileSystem.cppsrc/Utils/FileSystem.h
src/**/*.{cpp,cxx,cc,h,hpp,hxx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{cpp,cxx,cc,h,hpp,hxx}: Ensure SE/AE/VR runtime compatibility; use runtime detection patterns (e.g., REL::RelocateMember())
Include robust error handling and resource management with graceful degradation in the plugin code
Files:
src/SettingsOverrideManager.cppsrc/State.cppsrc/State.hsrc/Utils/FileSystem.cppsrc/Utils/FileSystem.h
🧬 Code graph analysis (4)
src/SettingsOverrideManager.cpp (1)
src/Utils/FileSystem.cpp (4)
GetOverridesPath(81-84)GetOverridesPath(81-81)GetAppliedOverridesPath(84-87)GetAppliedOverridesPath(84-84)
src/State.cpp (1)
src/Utils/FileSystem.cpp (8)
GetSettingsUserPath(56-59)GetSettingsUserPath(56-56)GetSettingsTestPath(61-64)GetSettingsTestPath(61-61)GetSettingsThemePath(71-74)GetSettingsThemePath(71-71)GetSettingsDefaultPath(66-69)GetSettingsDefaultPath(66-66)
src/State.h (1)
src/State.cpp (4)
LoadTheme(464-469)LoadTheme(464-464)SaveTheme(471-476)SaveTheme(471-471)
src/Utils/FileSystem.h (1)
src/Utils/FileSystem.cpp (14)
GetSettingsDefaultPath(66-69)GetSettingsDefaultPath(66-66)GetSettingsThemePath(71-74)GetSettingsThemePath(71-71)GetThemesPath(76-79)GetThemesPath(76-76)GetOverridesPath(81-84)GetOverridesPath(81-81)GetShadersRealPath(142-145)GetShadersRealPath(142-142)GetThemesRealPath(147-150)GetThemesRealPath(147-147)GetFeaturesRealPath(152-155)GetFeaturesRealPath(152-152)
⏰ 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 (16)
src/SettingsOverrideManager.cpp (3)
4-4: LGTM: Util.h inclusion enables PathHelpers.The inclusion of
Util.hcorrectly provides access to the centralizedPathHelpersnamespace used in this file.
281-284: LGTM: Path resolution centralized via PathHelpers.The method now correctly delegates to
Util::PathHelpers::GetOverridesPath(), eliminating the hardcoded path constant and aligning with the PR's centralization objective.
405-408: LGTM: Applied overrides tracking path centralized.The method now correctly uses
Util::PathHelpers::GetAppliedOverridesPath(), consistent with the centralized path management approach.src/State.h (2)
68-74: LGTM: THEME config mode added for theme system foundation.The addition of the
THEMEconfig mode to the enum appropriately extends configuration management to support theme files, aligning with the PR's objective to establish the foundation for the theme system.
84-85: LGTM: Stub theme APIs defer full implementation to PR #4.The
LoadTheme()andSaveTheme()declarations establish the public interface for theme management. Per the PR description and related code snippets (State.cpp lines 463-476), these are intentional stubs with full implementation planned for PR #4 (Theme System Core).src/Utils/FileSystem.cpp (6)
66-69: LGTM: GetSettingsDefaultPath implementation.The implementation correctly constructs the path to
SettingsDefault.jsonusing the centralizedGetCommunityShaderPath()helper.
71-74: LGTM: GetSettingsThemePath implementation.The implementation correctly constructs the path to
SettingsTheme.json, supporting the new THEME config mode.
76-79: LGTM: GetThemesPath implementation.The implementation correctly constructs the path to the
Themesdirectory for theme discovery.
142-145: LGTM: GetShadersRealPath implementation.The implementation correctly resolves the real filesystem path to the
Shadersdirectory, which is useful for operations outside the VFS (e.g., MO2).
147-150: LGTM: GetThemesRealPath implementation.The implementation correctly constructs the real filesystem path to the
Themesdirectory (SKSE/Plugins/CommunityShaders/Themes), enabling direct filesystem access for theme files.
152-155: LGTM: GetFeaturesRealPath implementation.The implementation correctly constructs the real filesystem path to the
Featuresdirectory by building uponGetShadersRealPath().src/State.cpp (5)
154-167: LGTM: GetConfigPath refactored to use PathHelpers.The function signature change from returning
const std::string&tostd::stringis appropriate since the function now constructs paths on-the-fly usingUtil::PathHelpersand.string(). The addition of theTHEMEcase correctly supports the new config mode. All paths are consistently resolved through centralized path helpers.
175-175: LGTM: configFolderPath derived from GetConfigPath.The computation of
configFolderPathusingstd::filesystem::path(GetConfigPath(a_configMode)).parent_path().string()correctly extracts the parent directory for creating directories during Load operations.
400-406: LGTM: configFolderPath computation in Save.The refactored approach computes
configFolderPathfromconfigPathusingparent_path(), which is consistent with the Load method and eliminates reliance on previously stored paths.
409-412: LGTM: Early file open check improves robustness.The early check that the
ofstreamopened successfully prevents writing to a closed stream. This is a good defensive programming practice, especially when combined with the directory creation logic above.
464-476: LGTM: Stub implementations clearly documented.The
LoadTheme()andSaveTheme()stub implementations include clear comments indicating that full implementation is deferred to PR #4 (Theme System Core). The debug logging provides visibility when these methods are called. This is appropriate for a foundational PR (1 of 8).
| std::filesystem::path GetOverridesPath() | ||
| { | ||
| return GetCommunityShaderPath() / "Overrides"; | ||
| } std::filesystem::path GetAppliedOverridesPath() |
There was a problem hiding this comment.
Fix formatting: separate function definitions.
Line 84 has GetOverridesPath()'s closing brace immediately followed by GetAppliedOverridesPath() on the same line, reducing readability.
Apply this diff:
std::filesystem::path GetOverridesPath()
{
return GetCommunityShaderPath() / "Overrides";
- } std::filesystem::path GetAppliedOverridesPath()
+ }
+
+ std::filesystem::path GetAppliedOverridesPath()
{
return GetCommunityShaderPath() / "AppliedOverrides.json";
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| std::filesystem::path GetOverridesPath() | |
| { | |
| return GetCommunityShaderPath() / "Overrides"; | |
| } std::filesystem::path GetAppliedOverridesPath() | |
| std::filesystem::path GetOverridesPath() | |
| { | |
| return GetCommunityShaderPath() / "Overrides"; | |
| } | |
| std::filesystem::path GetAppliedOverridesPath() | |
| { | |
| return GetCommunityShaderPath() / "AppliedOverrides.json"; | |
| } |
🤖 Prompt for AI Agents
In src/Utils/FileSystem.cpp around lines 81 to 84, the closing brace of
GetOverridesPath() is immediately followed by the next function signature on the
same line; separate the function definitions by moving the
GetAppliedOverridesPath() declaration to its own line (add a newline between the
closing brace of GetOverridesPath() and the start of GetAppliedOverridesPath()),
preserving existing indentation and line endings to restore readability and
consistent formatting.
| /** | ||
| * Gets the Overrides directory path | ||
| * @return CommunityShaderPath / "Overrides" | ||
| */ | ||
| std::filesystem::path GetOverridesPath(); /** |
There was a problem hiding this comment.
Fix formatting: separate function declaration from comment block.
The closing brace of GetOverridesPath() and the opening of the comment block for GetAppliedOverridesPath() are on the same line, making the code harder to read.
Apply this diff:
- std::filesystem::path GetOverridesPath(); /**
+ std::filesystem::path GetOverridesPath();
+
+ /**
* Gets the AppliedOverrides.json file path
* @return CommunityShaderPath / "AppliedOverrides.json"
*/📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * Gets the Overrides directory path | |
| * @return CommunityShaderPath / "Overrides" | |
| */ | |
| std::filesystem::path GetOverridesPath(); /** | |
| /** | |
| * Gets the Overrides directory path | |
| * @return CommunityShaderPath / "Overrides" | |
| */ | |
| std::filesystem::path GetOverridesPath(); | |
| /** | |
| * Gets the AppliedOverrides.json file path | |
| * @return CommunityShaderPath / "AppliedOverrides.json" | |
| */ |
🤖 Prompt for AI Agents
In src/Utils/FileSystem.h around lines 95 to 99, the declaration of
GetOverridesPath() and the next comment block are run together on one line;
separate them by placing the function declaration (closing semicolon) on its own
line and start the following comment block on the next line so the comment and
declaration are not concatenated. Update formatting to ensure
GetOverridesPath(); ends the line, then add a newline before the /** comment for
GetAppliedOverridesPath().
|
✅ A pre-release build is available for this PR: |
1 participant
This provides the foundation for the theme system by centralising path management and removing hardcoded strings throughout the codebase.
Part of UI modernisation - PR #1 of 8
Summary by CodeRabbit