Add support for adjusting color intensity with Color::apply_intensity method#1264
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
✅ Files skipped from review due to trivial changes (1)
WalkthroughThis PR implements the apply_intensity method on the Color type across C++, the variant system, C# bindings, and documentation. The method scales RGB channels by 2^intensity with clamping and early-exit optimization, making it available programmatically in GDScript and C#. ChangesColor Intensity Application Method
Sequence Diagram(s)sequenceDiagram
participant GDScript
participant VariantSystem
participant ColorCpp
GDScript->>VariantSystem: Color.apply_intensity(intensity)
VariantSystem->>ColorCpp: invoke Color::apply_intensity(float)
ColorCpp-->>VariantSystem: Color (RGB scaled, alpha preserved)
VariantSystem-->>GDScript: return Color
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
|
On second thought, after rereviewing the docs for Color, this implementation doesnt really match the existing APIs very well. Methods for I am going to refactor accordingly. |
a500f7f to
259fa3a
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.gitignore:
- Line 394: The .gitignore currently contains an unrelated entry "/.junie/"
which should be removed or justified; either delete the "/.junie/" line from
.gitignore (preferred) or, if you intend it to be a shared ignore, add a short
justification to the commit message and keep it, or move the ignore to a
developer-local exclude by adding it to .git/info/exclude instead; update the PR
accordingly and ensure the change is separate from the Color::apply_intensity
work if retained.
In `@doc/classes/Color.xml`:
- Around line 101-109: The documentation for Color::apply_intensity is
incorrect: update the <method name="apply_intensity"> entry to reflect the real
API by changing the return type from void to Color (replace <return type="void"
/> with <return type="Color" />) and mark the method as const (add the
appropriate const="true" attribute on the <method> tag or equivalent per the doc
schema); keep the existing <param name="intensity" type="float" /> and
description as-is.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1d41a0c1-9e0d-484a-9de6-6159eb36a2b9
📒 Files selected for processing (6)
.gitignorecore/math/color.cppcore/math/color.hcore/variant/variant_call.cppdoc/classes/Color.xmlmodules/mono/glue/GodotSharp/GodotSharp/Core/Color.cs
259fa3a to
f9ba7ba
Compare
OldDev78
left a comment
There was a problem hiding this comment.
"Apply" implies that an absolute value is set, but that's not what apply_intensity does. I would personally use the verb "adjust" instead.
a09e6a0
into
Redot-Engine:master
Added
Color::apply_intensity()method to allow applying an intensity value directly to aColorlike can be done with the Color Picker.Closes Redot-Engine/redot-proposals#117
I added bindings for both GDScript and C#, note that the C# variant is SIMD optimized.
Someone else is welcome to add SSE and NEON intrinsics for the C++ version later on.
Summary by CodeRabbit
New Features
Documentation
Chores