Skip to content

refactor(ibl): move ibl to core feature#1396

Merged
alandtse merged 2 commits into
community-shaders:devfrom
jiayev:IBL
Aug 16, 2025
Merged

refactor(ibl): move ibl to core feature#1396
alandtse merged 2 commits into
community-shaders:devfrom
jiayev:IBL

Conversation

@jiayev
Copy link
Copy Markdown
Collaborator

@jiayev jiayev commented Aug 15, 2025

Summary by CodeRabbit

  • New Features

    • Image-Based Lighting is now designated as a core capability and will be treated as always available by default.
  • Refactor

    • Classification updated so the lighting feature is treated as core while preserving existing behavior and settings (name, short name, shader define, VR support, category).
  • Chores

    • Minor internal declaration adjustments to align with the core classification; no user-facing behavior changes.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Aug 15, 2025

Walkthrough

Replaces the IBL feature's mod link constant/method with a core-feature indicator: removes the private MOD_ID and GetFeatureModLink(), and adds a public override IsCore() const returning true. No other interfaces or logic changed. (≤50 words)

Changes

Cohort / File(s) Summary
Feature interface update
src/Features/IBL.h
Removed private MOD_ID and GetFeatureModLink(); added virtual bool IsCore() const override { return true; } to struct IBL : Feature. No other changes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • alandtse
  • doodlum

Poem

I nibble code beneath the moon,
A single flag hums out its tune,
"Core," I whisper, soft and bright,
One tiny change, snug and right.
Hop, hop — the feature's set to light. 🥕🐇

Tip

🔌 Remote MCP (Model Context Protocol) integration is now available!

Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats.


📜 Recent review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 236906b and 3afb291.

📒 Files selected for processing (1)
  • src/Features/IBL.h (1 hunks)
⏰ 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 (2)
src/Features/IBL.h (2)

7-7: IBL correctly marked as a core feature.

Adding IsCore() const override { return true; } is aligned with the PR’s goal. Signature and constness look right.


5-12: Confirm default GetFeatureModLink and UI handling

I wasn’t able to locate the base Feature declaration or any references to GetFeatureModLink() in the search above—please double-check that removing the override in IBL won’t break “Get Mod” link rendering:

• Inspect the Feature base class (e.g. Feature.h) to verify it declares GetFeatureModLink() and provides a safe default or pure-virtual stub
• Confirm the UI or other code paths that render “Get Mod” links call GetFeatureModLink(), hide/skip it when IsCore() is true, or correctly handle an empty/default return
• Ensure there are no remaining IBL-specific MOD_ID or MakeNexusModURL constants elsewhere

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.
    • 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.
  • 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 the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

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

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

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.

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • 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.

@github-actions
Copy link
Copy Markdown

Using provided base ref: 527793f
Using base ref: 527793f
Base commit date: 2025-08-14T12:59:37+01:00 (Thursday, August 14, 2025 12:59 PM)
No actionable suggestions for changed features.

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: 0

🧹 Nitpick comments (1)
src/Features/IBL.h (1)

10-10: Nit: drop the trailing semicolon after the inline definition for consistency

Most inline definitions in this class (e.g., GetName, GetShortName, GetFeatureModLink, GetShaderDefineName) omit the trailing semicolon. Consider aligning with that style.

-	virtual bool IsCore() const override { return true; };
+	virtual bool IsCore() const override { return true; }
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 527793f and 236906b.

📒 Files selected for processing (1)
  • src/Features/IBL.h (1 hunks)
⏰ 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 (2)
src/Features/IBL.h (2)

10-10: Marking IBL as a core feature looks good

The const-qualified override returning true is clear and aligns with the PR intent of moving IBL to a core feature.


10-10: Verify IsCore const-qualification and call-site implications — src/Features/IBL.h not found

Repository search returned no matches for src/Features/IBL.h or any IsCore declarations/call sites, so I could not verify the override binding.

Observed snippet (from PR):
virtual bool IsCore() const override { return true; };

Action items:

  • Confirm the base declaration (e.g. Feature / IFeature) declares: bool IsCore() const; otherwise this override will not bind.
  • Review call sites and any UI/config gating that treat "core" features to ensure behavior remains correct if the signature changed.
  • If the header path differs or was omitted from the PR, provide the correct file or run a repo-wide search for IsCore declarations before merging.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Aug 15, 2025

✅ A pre-release build is available for this PR:
Download

@alandtse
Copy link
Copy Markdown
Collaborator

What's the intent for why it's core now?

@jiayev
Copy link
Copy Markdown
Collaborator Author

jiayev commented Aug 15, 2025

What's the intent for why it's core now?

doodlum said so

@alandtse
Copy link
Copy Markdown
Collaborator

Doesn't GetFeatureModLink need to be removed then?

@jiayev
Copy link
Copy Markdown
Collaborator Author

jiayev commented Aug 16, 2025

Doesn't GetFeatureModLink need to be removed then?

yeah i forgot. removed now.

@alandtse alandtse merged commit 2a9e819 into community-shaders:dev Aug 16, 2025
15 checks passed
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.

2 participants