Skip to content

feat(ui): add feature summary tooltips and enhanced unloaded UI#1134

Merged
doodlum merged 2 commits into
community-shaders:devfrom
alandtse:feature_summary
Jun 11, 2025
Merged

feat(ui): add feature summary tooltips and enhanced unloaded UI#1134
doodlum merged 2 commits into
community-shaders:devfrom
alandtse:feature_summary

Conversation

@alandtse
Copy link
Copy Markdown
Collaborator

@alandtse alandtse commented Jun 11, 2025

  • Add GetFeatureSummary() virtual method to Feature base class
  • Implement feature summaries for 20+ features with descriptions and key features
  • Add hover tooltips in feature list showing comprehensive feature information
  • Create default DrawUnloadedUI() with theme-consistent error colors
  • Update TerrainVariation and TerrainHelper to use new summary system
  • Enhance user experience with rich feature descriptions and consistent UI

Summary by CodeRabbit

  • New Features

    • Added detailed feature summaries and key feature lists to tooltips in the menu for both loaded and unloaded features, providing clear descriptions and highlights of each feature.
    • Standardized the UI presentation for features that are not installed, including descriptive messages and bullet-pointed key features.
  • Improvements

    • Enhanced clarity and consistency of feature information displayed throughout the UI, making it easier for users to understand each feature’s capabilities at a glance.

- Add GetFeatureSummary() virtual method to Feature base class
- Implement feature summaries for 20+ features with descriptions and key features
- Add hover tooltips in feature list showing comprehensive feature information
- Create default DrawUnloadedUI() with theme-consistent error colors
- Update TerrainVariation and TerrainHelper to use new summary system
- Enhance user experience with rich feature descriptions and consistent UI
Copilot AI review requested due to automatic review settings June 11, 2025 09:10
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jun 11, 2025

Walkthrough

A standardized feature summary system was introduced across all feature structs by adding a virtual GetFeatureSummary() method. This method provides a description and key features for each feature. The UI logic for unloaded features and tooltips was updated to display this summary, ensuring consistent presentation in both the main menu and feature-specific UIs.

Changes

File(s) Change Summary
src/Feature.h Added virtual GetFeatureSummary() to Feature struct; implemented DrawUnloadedUI() to display summary and key points.
src/Features/*.h (all feature headers) Added/overrode GetFeatureSummary() in all feature structs to return a summary description and key feature list.
src/Features/TerrainHelper.cpp Updated DrawUnloadedUI() to use GetFeatureSummary() for detailed UI rendering with description and bullet points.
src/Features/TerrainVariation.cpp Removed DrawUnloadedUI() method (now handled by base class and summary system).
src/Features/TerrainVariation.h Added override GetFeatureSummary() providing description and key features for terrain variation.
src/Menu.cpp Updated feature list tooltips to use GetFeatureSummary() for loaded features, displaying description and key points.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Menu
    participant Feature
    participant ImGui

    User->>Menu: Hover over feature in menu
    Menu->>Feature: GetFeatureSummary()
    Feature-->>Menu: (description, key features)
    Menu->>ImGui: Show tooltip with description and key features
Loading
sequenceDiagram
    participant User
    participant FeatureUI
    participant Feature

    User->>FeatureUI: View unloaded feature
    FeatureUI->>Feature: GetFeatureSummary()
    Feature-->>FeatureUI: (description, key features)
    FeatureUI->>ImGui: Render summary and key features in UI
Loading

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between abe20f1 and 22eec61.

📒 Files selected for processing (26)
  • src/Feature.h (1 hunks)
  • src/Features/CloudShadows.h (1 hunks)
  • src/Features/DynamicCubemaps.h (1 hunks)
  • src/Features/ExtendedMaterials.h (1 hunks)
  • src/Features/GrassCollision.h (1 hunks)
  • src/Features/GrassLighting.h (1 hunks)
  • src/Features/HairSpecular.h (1 hunks)
  • src/Features/IBL.h (1 hunks)
  • src/Features/InteriorSunShadows.h (1 hunks)
  • src/Features/InverseSquareLighting.h (1 hunks)
  • src/Features/LODBlending.h (1 hunks)
  • src/Features/LightLimitFix.h (1 hunks)
  • src/Features/ScreenSpaceGI.h (1 hunks)
  • src/Features/ScreenSpaceShadows.h (1 hunks)
  • src/Features/SkySync.h (1 hunks)
  • src/Features/Skylighting.h (1 hunks)
  • src/Features/SubsurfaceScattering.h (1 hunks)
  • src/Features/TerrainBlending.h (1 hunks)
  • src/Features/TerrainHelper.cpp (1 hunks)
  • src/Features/TerrainHelper.h (1 hunks)
  • src/Features/TerrainShadows.h (1 hunks)
  • src/Features/TerrainVariation.h (1 hunks)
  • src/Features/VR.h (1 hunks)
  • src/Features/VolumetricLighting.h (1 hunks)
  • src/Features/WaterEffects.h (1 hunks)
  • src/Features/WetnessEffects.h (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (26)
  • src/Features/LODBlending.h
  • src/Features/IBL.h
  • src/Features/ScreenSpaceShadows.h
  • src/Features/InverseSquareLighting.h
  • src/Features/DynamicCubemaps.h
  • src/Features/VR.h
  • src/Features/GrassLighting.h
  • src/Features/WetnessEffects.h
  • src/Features/ExtendedMaterials.h
  • src/Features/ScreenSpaceGI.h
  • src/Features/TerrainHelper.h
  • src/Features/TerrainHelper.cpp
  • src/Features/TerrainVariation.h
  • src/Features/HairSpecular.h
  • src/Features/TerrainBlending.h
  • src/Features/LightLimitFix.h
  • src/Features/TerrainShadows.h
  • src/Features/SkySync.h
  • src/Features/GrassCollision.h
  • src/Features/WaterEffects.h
  • src/Features/SubsurfaceScattering.h
  • src/Features/VolumetricLighting.h
  • src/Feature.h
  • src/Features/Skylighting.h
  • src/Features/InteriorSunShadows.h
  • src/Features/CloudShadows.h
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: Build plugin and addons
✨ Finishing Touches
  • 📝 Generate Docstrings

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces a new hover tooltip system for feature summaries, enhancing the unloaded UI across multiple features. The changes add a virtual GetFeatureSummary() method to the Feature base class and implement feature-specific summaries (including descriptions and key bullet points) in various feature files.

  • Added GetFeatureSummary() implementations to several feature classes.
  • Updated DrawUnloadedUI() in both TerrainHelper.cpp and Feature.h to display hover tooltips and descriptive messaging.
  • Enhanced UI error messaging for unloaded features to be theme consistent.

Reviewed Changes

Copilot reviewed 29 out of 29 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/Features/TerrainHelper.h Added GetFeatureSummary() method providing terrain helper details.
src/Features/TerrainHelper.cpp Updated DrawUnloadedUI() to use the new summary information.
src/Features/TerrainBlending.h Added GetFeatureSummary() for blending feature description.
src/Features/SubsurfaceScattering.h Added GetFeatureSummary() returning multi-line summary for realistic character lighting.
src/Features/Skylighting.h Introduced summary details for skylighting feature.
src/Features/SkySync.h Added summary for synchronizing celestial lighting.
src/Features/ScreenSpaceShadows.h Added feature summary for improved shadow details.
src/Features/ScreenSpaceGI.h Added summary for global illumination techniques.
src/Features/LightLimitFix.h Implmented summary detailing removal of the 4-light limit.
src/Features/LODBlending.h Added summary for smooth transitions between LODs.
src/Features/InverseSquareLighting.h Added physical-based lighting summary details.
src/Features/InteriorSunShadows.h Added summary for realistic interior sun shadows.
src/Features/IBL.h Added summary for Image Based Lighting enhancements.
src/Features/HairSpecular.h Added summary for hair specular enhancements.
src/Features/GrassLighting.h Added summary detailing enhanced grass lighting features.
src/Features/GrassCollision.h Added summary for dynamic grass collision interactions.
src/Features/ExtendedMaterials.h Added summary for advanced material effects.
src/Features/DynamicCubemaps.h Added summary for real-time environment mapping via cubemaps.
src/Features/CloudShadows.h Added summary for dynamic cloud shadows.
src/Feature.h Updated default DrawUnloadedUI() to utilize feature summaries.

Automated formatting by clang-format, prettier, and other hooks.
See https://pre-commit.ci for details.
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

🧹 Nitpick comments (9)
src/Features/ExtendedMaterials.h (1)

15-28: Feature summary implementation is correct
Description and bullet list accurately reflect the Extended Materials feature. Consider using a raw string literal for the multi-sentence description to improve readability and maintenance.

src/Features/WaterEffects.h (1)

20-33: Feature summary implementation is correct
Summary clearly covers enhanced water rendering aspects. To reduce string-concatenation noise and ease future edits, you might switch to a raw string literal for the two-line description.

src/Features/DynamicCubemaps.h (1)

105-117: Mark GetFeatureSummary() as inline and consider caching its return.
Currently, this method allocates a new std::string and std::vector on every invocation and isn’t marked inline like the other header-defined overrides. For consistency and performance, add the inline specifier and/or store the summary in a static const so repeated calls don’t reallocate.

src/Features/TerrainVariation.h (1)

19-31: Make GetFeatureSummary() inline and avoid per-call allocations.
This override isn’t marked inline and rebuilds the description and key points on every call. To align with the rest of the header-defined methods and reduce runtime allocations, add inline and/or cache the returned pair as a static const.

src/Features/GrassLighting.h (1)

16-29: Add inline specifier to GetFeatureSummary() and optimize memory usage.
This header-defined override should be marked inline and could benefit from caching its summary to prevent repeated heap allocations of the string and vector.

src/Features/SubsurfaceScattering.h (1)

66-79: Consistently inline and cache the feature summary.
As with other header-defined overrides, mark GetFeatureSummary() as inline and consider returning a reference to a static const std::pair to eliminate per-call construction overhead.

src/Features/Skylighting.h (1)

16-28: Inline and memoize GetFeatureSummary() in the header.
This override rebuilds its summary and key-point list on every invocation. For consistency and performance, add the inline keyword and/or cache the summary in a static const to avoid unnecessary allocations.

src/Menu.cpp (1)

413-429: Avoid repeated allocations by caching GetFeatureSummary()

GetFeatureSummary() is invoked every time the item is hovered, constructing a std::pair<std::string, std::vector<std::string>> and potentially allocating memory for the vectors on every frame while the tooltip is visible. For large feature lists this can add measurable overhead to the UI thread.

- if (auto _tt = Util::HoverTooltipWrapper()) {
-     auto [description, keyFeatures] = feat->GetFeatureSummary();
+ if (auto _tt = Util::HoverTooltipWrapper()) {
+     static thread_local std::pair<std::string, std::vector<std::string>> summary;
+     summary = feat->GetFeatureSummary();              // 1-time copy per frame, not per ImGui item
+     const auto& description  = summary.first;
+     const auto& keyFeatures  = summary.second;

(Optional) retain a small LRU cache keyed on Feature* if you expect the summaries to be immutable during the session.

src/Feature.h (1)

41-47: GetFeatureSummary() contract looks good – consider returning std::span<const std::string> for zero-copy

Returning a std::pair<std::string, std::vector<std::string>> works but forces every caller to allocate / copy on every call. Exposing a std::string_view + std::span<const std::string> (or two std::string_views) would avoid extra allocations while keeping the API simple.

No action required if performance is not a concern.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 282ca1c and abe20f1.

⛔ Files ignored due to path filters (1)
  • src/.DS_Store is excluded by !**/.DS_Store
📒 Files selected for processing (28)
  • src/Feature.h (1 hunks)
  • src/Features/CloudShadows.h (1 hunks)
  • src/Features/DynamicCubemaps.h (1 hunks)
  • src/Features/ExtendedMaterials.h (1 hunks)
  • src/Features/GrassCollision.h (1 hunks)
  • src/Features/GrassLighting.h (1 hunks)
  • src/Features/HairSpecular.h (1 hunks)
  • src/Features/IBL.h (1 hunks)
  • src/Features/InteriorSunShadows.h (1 hunks)
  • src/Features/InverseSquareLighting.h (1 hunks)
  • src/Features/LODBlending.h (1 hunks)
  • src/Features/LightLimitFix.h (1 hunks)
  • src/Features/ScreenSpaceGI.h (1 hunks)
  • src/Features/ScreenSpaceShadows.h (1 hunks)
  • src/Features/SkySync.h (1 hunks)
  • src/Features/Skylighting.h (1 hunks)
  • src/Features/SubsurfaceScattering.h (1 hunks)
  • src/Features/TerrainBlending.h (1 hunks)
  • src/Features/TerrainHelper.cpp (1 hunks)
  • src/Features/TerrainHelper.h (1 hunks)
  • src/Features/TerrainShadows.h (1 hunks)
  • src/Features/TerrainVariation.cpp (0 hunks)
  • src/Features/TerrainVariation.h (1 hunks)
  • src/Features/VR.h (1 hunks)
  • src/Features/VolumetricLighting.h (1 hunks)
  • src/Features/WaterEffects.h (1 hunks)
  • src/Features/WetnessEffects.h (1 hunks)
  • src/Menu.cpp (1 hunks)
💤 Files with no reviewable changes (1)
  • src/Features/TerrainVariation.cpp
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/Features/TerrainHelper.cpp (1)
src/Feature.h (1)
  • description (52-73)
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: Build plugin and addons
🔇 Additional comments (12)
src/Features/TerrainShadows.h (1)

16-28: Correctly implemented feature summary
GetFeatureSummary() delivers a precise description and a comprehensive list of key capabilities. Content is clear, consistent with other features, and free of typos.

src/Features/VR.h (1)

14-26: Well-defined VR feature summary
The summary text concisely conveys purpose and optimization points for VR. Implementation matches the established pattern with no issues.

src/Features/IBL.h (1)

17-30: Correct IBL feature summary
GetFeatureSummary() provides an accurate overview and relevant bullet points for image-based lighting. Aligns with the standardized pattern; no changes needed.

src/Features/ScreenSpaceShadows.h (1)

14-28: Approve addition of GetFeatureSummary
The summary and key points accurately describe Screen Space Shadows and align with the standardized format.

src/Features/WetnessEffects.h (1)

16-28: Approve addition of GetFeatureSummary
The descriptive text and bullet list clearly convey the Wetness Effects feature and match the style of other feature summaries.

src/Features/TerrainBlending.h (1)

15-27: Approve addition of GetFeatureSummary
The summary string and highlights effectively outline the Terrain Blending feature’s capabilities and follow the established pattern.

src/Features/GrassCollision.h (1)

15-27: Approve addition of GetFeatureSummary
The summary and bullet points accurately reflect Grass Collision behavior and maintain consistency with other feature descriptions.

src/Features/LightLimitFix.h (1)

18-31: Approve addition of GetFeatureSummary
The feature summary clearly explains the Light Limit Fix improvements and aligns with the format used across features.

src/Features/HairSpecular.h (1)

14-26: 🛠️ Refactor suggestion

⚠️ Potential issue

ODR violation risk: missing inline on header-defined method
Defining a non-inline, non-template virtual method in a header can lead to multiple-definition errors at link time.

Apply this diff to mark it inline:

-   virtual std::pair<std::string, std::vector<std::string>> GetFeatureSummary() override
+   virtual inline std::pair<std::string, std::vector<std::string>> GetFeatureSummary() override

Likely an incorrect or invalid review comment.

src/Features/ScreenSpaceGI.h (1)

23-36: 🛠️ Refactor suggestion

⚠️ Potential issue

ODR violation risk: missing inline on header-defined method
As above, GetFeatureSummary() is defined in the header without inline, risking multiple-definition linker errors.

-  virtual std::pair<std::string, std::vector<std::string>> GetFeatureSummary() override
+  virtual inline std::pair<std::string, std::vector<std::string>> GetFeatureSummary() override

Likely an incorrect or invalid review comment.

src/Features/CloudShadows.h (1)

22-34: 🛠️ Refactor suggestion

⚠️ Potential issue

ODR violation risk: missing inline on header-defined method
GetFeatureSummary() appears in the header; mark it inline to avoid duplicate definitions.

-  virtual std::pair<std::string, std::vector<std::string>> GetFeatureSummary() override
+  virtual inline std::pair<std::string, std::vector<std::string>> GetFeatureSummary() override

Likely an incorrect or invalid review comment.

src/Features/VolumetricLighting.h (1)

36-49: 🛠️ Refactor suggestion

⚠️ Potential issue

ODR violation risk: missing inline on header-defined method
Same pattern: header-defined virtual method needs inline.

-  virtual std::pair<std::string, std::vector<std::string>> GetFeatureSummary() override
+  virtual inline std::pair<std::string, std::vector<std::string>> GetFeatureSummary() override

Likely an incorrect or invalid review comment.

Comment on lines +14 to +26
virtual std::pair<std::string, std::vector<std::string>> GetFeatureSummary() override
{
return {
"Provides seamless visual transitions between Level of Detail (LOD) objects and full-detail objects, eliminating harsh transitions and creating smooth visual continuity.",
{
"Smooth LOD object brightness blending",
"Enhanced terrain LOD appearance matching",
"Snow-specific LOD brightness adjustment",
"Optional terrain vertex color modification",
"Seamless transition between detail levels"
}
};
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Mark GetFeatureSummary() as inline (and add const) to prevent ODR issues and match base signature.
Defining a non-inline virtual in a header can cause multiple-definition errors; also confirm the base method is const and mirror its qualifiers for a valid override.

Apply this diff:

-       virtual std::pair<std::string, std::vector<std::string>> GetFeatureSummary() override
+       virtual inline std::pair<std::string, std::vector<std::string>> GetFeatureSummary() const override
📝 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.

Suggested change
virtual std::pair<std::string, std::vector<std::string>> GetFeatureSummary() override
{
return {
"Provides seamless visual transitions between Level of Detail (LOD) objects and full-detail objects, eliminating harsh transitions and creating smooth visual continuity.",
{
"Smooth LOD object brightness blending",
"Enhanced terrain LOD appearance matching",
"Snow-specific LOD brightness adjustment",
"Optional terrain vertex color modification",
"Seamless transition between detail levels"
}
};
}
virtual inline std::pair<std::string, std::vector<std::string>> GetFeatureSummary() const override
{
return {
"Provides seamless visual transitions between Level of Detail (LOD) objects and full-detail objects, eliminating harsh transitions and creating smooth visual continuity.",
{
"Smooth LOD object brightness blending",
"Enhanced terrain LOD appearance matching",
"Snow-specific LOD brightness adjustment",
"Optional terrain vertex color modification",
"Seamless transition between detail levels"
}
};
}
🤖 Prompt for AI Agents
In src/Features/LODBlending.h lines 14 to 26, mark the GetFeatureSummary()
method as inline and add the const qualifier to match the base class signature
and prevent multiple-definition (ODR) errors. Change the method declaration to
inline and append const after the override keyword to ensure it correctly
overrides the base method.

Comment on lines +19 to +31
virtual std::pair<std::string, std::vector<std::string>> GetFeatureSummary() override
{
return {
"Implements physically accurate inverse square falloff for lighting, making light attenuation behave realistically with distance for more natural illumination.",
{
"Physically accurate light attenuation based on distance",
"Automatic radius calculation for optimal performance",
"Enhanced light editor for fine-tuning light properties",
"Realistic shadow caster handling",
"Support for both point and directional lighting"
}
};
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add inline specifier (and const) to GetFeatureSummary() in-header.
This definition lives in a header; mark it inline and align constness with the base declaration to ensure proper linkage and overriding.

-       virtual std::pair<std::string, std::vector<std::string>> GetFeatureSummary() override
+       virtual inline std::pair<std::string, std::vector<std::string>> GetFeatureSummary() const override
🤖 Prompt for AI Agents
In src/Features/InverseSquareLighting.h around lines 19 to 31, the
GetFeatureSummary() method is defined in the header but lacks the inline
specifier and const qualifier. To fix this, add the inline keyword before the
method definition and mark the method as const to match the base class
declaration, ensuring correct linkage and proper overriding.

Comment on lines +14 to +26
virtual std::pair<std::string, std::vector<std::string>> GetFeatureSummary() override
{
return {
"Enables realistic sun shadows inside interior spaces that have openings to the exterior, such as windows and doors, bringing natural lighting indoors.",
{
"Sun shadow casting through windows and openings",
"Double-sided rendering for accurate interior shadows",
"Automatic detection of interiors with sun exposure",
"Enhanced directional light culling for interiors",
"Seamless integration with existing shadow systems"
}
};
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Make GetFeatureSummary() inline and const-correct.
Header-defined virtuals should be marked inline; add const if the base signature is const to guarantee a true override.

-       virtual std::pair<std::string, std::vector<std::string>> GetFeatureSummary() override
+       virtual inline std::pair<std::string, std::vector<std::string>> GetFeatureSummary() const override

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In src/Features/InteriorSunShadows.h lines 14 to 26, the GetFeatureSummary()
method should be marked inline and const-correct. Add the inline keyword before
the method definition and append const at the end of the method signature to
match the base class and ensure proper overriding.

Comment on lines +15 to +27
virtual std::pair<std::string, std::vector<std::string>> GetFeatureSummary() override
{
return {
"Provides enhanced terrain material support for terrain mods that require additional texture slots and parallax mapping capabilities.",
{
"Extended texture slot support for terrain materials",
"Parallax mapping integration for terrain textures",
"Automatic terrain material detection and setup",
"Support for advanced terrain modifications",
"Compatibility layer for terrain enhancement mods"
}
};
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Inline-qualify and const-qualify GetFeatureSummary().
Since this method is implemented in a header, add inline and mirror the base method’s const qualifier to avoid ODR violations and ensure it actually overrides.

-       virtual std::pair<std::string, std::vector<std::string>> GetFeatureSummary() override
+       virtual inline std::pair<std::string, std::vector<std::string>> GetFeatureSummary() const override
📝 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.

Suggested change
virtual std::pair<std::string, std::vector<std::string>> GetFeatureSummary() override
{
return {
"Provides enhanced terrain material support for terrain mods that require additional texture slots and parallax mapping capabilities.",
{
"Extended texture slot support for terrain materials",
"Parallax mapping integration for terrain textures",
"Automatic terrain material detection and setup",
"Support for advanced terrain modifications",
"Compatibility layer for terrain enhancement mods"
}
};
}
virtual inline std::pair<std::string, std::vector<std::string>> GetFeatureSummary() const override
{
return {
"Provides enhanced terrain material support for terrain mods that require additional texture slots and parallax mapping capabilities.",
{
"Extended texture slot support for terrain materials",
"Parallax mapping integration for terrain textures",
"Automatic terrain material detection and setup",
"Support for advanced terrain modifications",
"Compatibility layer for terrain enhancement mods"
}
};
}
🤖 Prompt for AI Agents
In src/Features/TerrainHelper.h around lines 15 to 27, the GetFeatureSummary()
method lacks the inline specifier and const qualifier. To fix this, add the
inline keyword before the method declaration and append const at the end of the
method signature to match the base class method. This prevents ODR violations
and ensures proper overriding.

Comment thread src/Features/SkySync.h
Comment on lines +15 to +27
virtual std::pair<std::string, std::vector<std::string>> GetFeatureSummary() override
{
return {
"Synchronizes celestial lighting with the actual sky state, ensuring sun and moon lighting direction and intensity accurately match their visual position in the sky.",
{
"Accurate sun and moon lighting direction matching sky position",
"Alternative sun path calculation for improved realism",
"Configurable moon light source selection",
"Real-time celestial body tracking and updates",
"Enhanced sky and climate transition handling"
}
};
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add inline and const to GetFeatureSummary() for header safety and proper override.
Marking this virtual in a header as inline prevents multiple definitions; add const to match the base declaration.

-       virtual std::pair<std::string, std::vector<std::string>> GetFeatureSummary() override
+       virtual inline std::pair<std::string, std::vector<std::string>> GetFeatureSummary() const override
📝 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.

Suggested change
virtual std::pair<std::string, std::vector<std::string>> GetFeatureSummary() override
{
return {
"Synchronizes celestial lighting with the actual sky state, ensuring sun and moon lighting direction and intensity accurately match their visual position in the sky.",
{
"Accurate sun and moon lighting direction matching sky position",
"Alternative sun path calculation for improved realism",
"Configurable moon light source selection",
"Real-time celestial body tracking and updates",
"Enhanced sky and climate transition handling"
}
};
}
virtual inline std::pair<std::string, std::vector<std::string>> GetFeatureSummary() const override
{
return {
"Synchronizes celestial lighting with the actual sky state, ensuring sun and moon lighting direction and intensity accurately match their visual position in the sky.",
{
"Accurate sun and moon lighting direction matching sky position",
"Alternative sun path calculation for improved realism",
"Configurable moon light source selection",
"Real-time celestial body tracking and updates",
"Enhanced sky and climate transition handling"
}
};
}
🤖 Prompt for AI Agents
In src/Features/SkySync.h lines 15 to 27, the GetFeatureSummary() method should
be marked as inline to avoid multiple definition errors when included in
multiple translation units, and it should be marked const to match the base
class declaration and ensure it does not modify the object state. Update the
method signature to include both inline and const keywords accordingly.

Comment on lines +8 to 24
auto [description, keyFeatures] = GetFeatureSummary();

ImGui::PushTextWrapPos(ImGui::GetFontSize() * 35.0f);
ImGui::Text("%s", description.c_str());

if (!keyFeatures.empty()) {
ImGui::Spacing();
ImGui::Text("Key Features:");
for (const auto& feature : keyFeatures) {
ImGui::BulletText("%s", feature.c_str());
}
}

ImGui::Spacing();
ImGui::TextWrapped("Note: This feature is only required if a terrain mod you are using specifically requires it, otherwise it does nothing.");
ImGui::PopTextWrapPos();
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Ensure consistency with base DrawUnloadedUI() behavior
This override omits the “This feature is not installed!” header and uses ImGui::Text instead of TextWrapped for the summary.

Consider:

  1. Re-adding the error-colored header to match other unloaded UIs.
  2. Using ImGui::TextWrapped for the description to respect wrapping.
  3. Guarding PushTextWrapPos/PopTextWrapPos inside a non-empty check.

Example diff:

 void TerrainHelper::DrawUnloadedUI()
 {
-   auto [description, keyFeatures] = GetFeatureSummary();
+   auto [description, keyFeatures] = GetFeatureSummary();
+
+   if (!description.empty() || !keyFeatures.empty()) {
+       ImGui::TextColored(globals::menu->settings.Theme.StatusPalette.Error, "This feature is not installed!");
+       ImGui::Spacing();
+
+       ImGui::PushTextWrapPos(ImGui::GetFontSize() * 35.0f);
+       ImGui::TextWrapped("%s", description.c_str());
+       if (!keyFeatures.empty()) {
+           ImGui::Spacing();
+           ImGui::TextWrapped("Key Features:");
+           for (const auto& feature : keyFeatures) {
+               ImGui::BulletText("%s", feature.c_str());
+           }
+       }
+       ImGui::Spacing();
+       ImGui::TextWrapped("Note: This feature is only required if a terrain mod you are using specifically requires it, otherwise it does nothing.");
+       ImGui::PopTextWrapPos();
+   }

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In src/Features/TerrainHelper.cpp lines 8 to 24, the UI override is missing the
error-colored header "This feature is not installed!" present in the base
DrawUnloadedUI(), and it uses ImGui::Text instead of TextWrapped for the
description. To fix this, add the error-colored header at the top using
ImGui::TextColored with an appropriate error color, replace ImGui::Text with
ImGui::TextWrapped for the description to ensure proper text wrapping, and only
call ImGui::PushTextWrapPos and ImGui::PopTextWrapPos if the description is not
empty to avoid unnecessary calls.

Comment thread src/Feature.h Outdated
Comment on lines +52 to +73
virtual void DrawUnloadedUI()
{
auto [description, keyFeatures] = GetFeatureSummary();

if (!description.empty() || !keyFeatures.empty()) {
ImGui::TextColored(globals::menu->settings.Theme.StatusPalette.Error, "This feature is not installed!");
ImGui::Spacing();

if (!description.empty()) {
ImGui::TextWrapped("%s", description.c_str());
ImGui::Spacing();
}

if (!keyFeatures.empty()) {
ImGui::TextWrapped("Key features:");
for (const auto& feature : keyFeatures) {
ImGui::BulletText("%s", feature.c_str());
}
ImGui::Spacing();
}
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Header now contains ImGui calls – add explicit include to keep compilation order-agnostic

DrawUnloadedUI() uses ImGui::* symbols but imgui.h is not included. This relies on every translation unit that includes Feature.h to have already included ImGui, which is fragile.

#pragma once
+
+#include <imgui.h>          // required for ImGui::TextWrapped etc.
+#include "Menu.h"          // for globals::menu

Placing the include behind an #ifdef IMGUI_VERSION guard is fine if you want to keep it optional.

🤖 Prompt for AI Agents
In src/Feature.h around lines 52 to 73, the DrawUnloadedUI() method uses ImGui
functions but the header does not include imgui.h, causing fragile dependencies
on include order. Fix this by adding an explicit #include "imgui.h" at the top
of Feature.h, optionally guarded by #ifdef IMGUI_VERSION to keep it optional and
ensure compilation order-agnostic usage of ImGui symbols.

@doodlum doodlum merged commit 5793468 into community-shaders:dev Jun 11, 2025
8 of 9 checks passed
@alandtse alandtse deleted the feature_summary branch June 20, 2025 17:30
alandtse added a commit to alandtse/open-shaders that referenced this pull request Jul 20, 2025
…unity-shaders#1134)

* feat(ui): add feature summary tooltips and enhanced unloaded UI

- Add GetFeatureSummary() virtual method to Feature base class
- Implement feature summaries for 20+ features with descriptions and key features
- Add hover tooltips in feature list showing comprehensive feature information
- Create default DrawUnloadedUI() with theme-consistent error colors
- Update TerrainVariation and TerrainHelper to use new summary system
- Enhance user experience with rich feature descriptions and consistent UI

* style: 🎨 apply pre-commit.ci formatting

Automated formatting by clang-format, prettier, and other hooks.
See https://pre-commit.ci for details.

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants