Refactor automatic splash background color. #1149
Conversation
WalkthroughAdds EditorSettings helpers to read a setting directly from the on-disk editor settings file and provide a default base color; centralizes boot splash background color selection into Main::get_boot_splash_bg_color(); updates rendering to use this central color and registers a new project default/application boot color. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Renderer as RendererViewport
participant Main as Main::get_boot_splash_bg_color()
participant Editor as EditorSettings
participant Disk as VariantParser/rp_new
Note over Main,Editor: Resolve boot/clear background color
Renderer->>Main: request boot splash bg color
alt Editor/tools context
Main->>Editor: query "interface/theme/base_color" (in-memory)
alt in-memory present
Editor-->>Main: return Color
else
Editor->>Disk: parse editor settings file (VariantParser / rp_new)
Disk-->>Editor: parsed Variant or error
Editor-->>Main: extracted Color or get_default_base_color on failure
end
else not editor/tools
Main-->>Main: read GLOBAL_GET("application/boot_splash/bg_color")
end
Main-->>Renderer: resolved Color
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used🧬 Code graph analysis (1)main/main.cpp (1)
⏰ 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). (15)
🔇 Additional comments (4)
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: 1
Fix all issues with AI Agents 🤖
In @servers/rendering/renderer_viewport.cpp:
- Line 40: The current change calls Main::_get_boot_splash_bg_color() every
frame from RendererViewport::draw_viewports(), but that function may call
EditorSettings::get_setting_directly() and reparse the editor settings file per
call; modify Main::_get_boot_splash_bg_color() to first check
EditorSettings::get_singleton() and, if present, read
"interface/theme/base_color" from the singleton (in-memory) and only use
EditorSettings::get_setting_directly() as a fallback when the singleton is null
(early boot); keep the RendererViewport::draw_viewports() call to
set_default_clear_color(Main::_get_boot_splash_bg_color()) so runtime uses the
fast in-memory lookup.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
main/main.cpp (1)
3171-3175: Improve performance of boot background color lookup in hot rendering pathThe refactor to
_get_boot_splash_bg_color()centralizes boot/background color decisions well, but has a performance issue in editor mode:
_get_boot_splash_bg_color()is called fromRendererViewport::draw_viewports()every frame in editor mode. Currently, it always callsEditorSettings::get_setting_directly("interface/theme/base_color", ...), which opens and parses the editor settings file from disk on each invocation. This is inefficient onceEditorSettingsis initialized.Recommended fix: Use the in-memory cached settings when the singleton is available:
Color Main::_get_boot_splash_bg_color() { #if defined(TOOLS_ENABLED) if (editor || project_manager) { if (EditorSettings::get_singleton()) { // Use cached in-memory settings return EditorSettings::get_singleton()->get_setting("interface/theme/base_color").operator Color(); } // Early boot: singleton not yet initialized, fall back to direct file read return EditorSettings::get_setting_directly("interface/theme/base_color", EditorSettings::get_default_base_color()); } #endif return GLOBAL_DEF_BASIC("application/boot_splash/bg_color", boot_splash_bg_color); }This preserves early-boot behavior while eliminating per-frame disk parsing during editing and project execution from the editor.
Note: The behavior divergence between editor builds (using theme base color) and runtime builds (using project rendering default) is intentional per the code structure, so that aspect is fine.
editor/settings/editor_settings.cpp (1)
1-1: Fix clang-format violations.The CI pipeline indicates that formatting changes were applied by the clang-format hook. Please run clang-format on this file and commit the formatting changes to resolve the pipeline failure.
#!/bin/bash # Run clang-format to fix formatting issues clang-format -i editor/settings/editor_settings.cpp
🧹 Nitpick comments (2)
main/main.h (1)
40-41: Public helper naming and dependency on incompleteColortypeThe new public static
_get_boot_splash_bg_color()is fine mechanically, but two small nits:
- A public method with a leading underscore reads like a private/internal helper; consider exposing it as
get_boot_splash_bg_color()(and optionally keeping a private_get_...if you want) for clarity.- With only
struct Color;here, any translation unit using this function must already include the realColordefinition via some other header; that’s currently true, but a direct#include "core/math/color.h"here would make the dependency explicit and reduce the chance of call‑site compile surprises later.Also applies to: 78-78
editor/settings/editor_settings.h (1)
141-142: Clarify intended usage and cost ofget_setting_directly()in the APIThe new helpers are useful, but
get_setting_directly()is a fairly “heavy” operation (direct file parse) and bypasses the normal in-memoryEditorSettingsstate:
- Consider documenting (in a brief comment in the header) that this is meant for early-boot or exceptional cases where the singleton isn’t available yet, and shouldn’t be used in per-frame or other hot paths.
- Where the singleton exists, callers should generally prefer
EditorSettings::get_singleton()->get_setting(...)so changes made in the current session are respected and lookups stay cheap.This helps future callers (including plugin authors) avoid accidentally using the disk-based path in tight loops.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
editor/settings/editor_settings.cppeditor/settings/editor_settings.hmain/main.cppmain/main.hservers/rendering/renderer_viewport.cpp
🧰 Additional context used
🧬 Code graph analysis (3)
servers/rendering/renderer_viewport.cpp (1)
main/main.cpp (2)
_get_boot_splash_bg_color(3767-3775)_get_boot_splash_bg_color(3767-3767)
editor/settings/editor_settings.h (1)
editor/settings/editor_settings.cpp (4)
get_setting_directly(1247-1287)get_setting_directly(1247-1247)get_default_base_color(1289-1291)get_default_base_color(1289-1289)
main/main.h (1)
main/main.cpp (2)
_get_boot_splash_bg_color(3767-3775)_get_boot_splash_bg_color(3767-3767)
🪛 GitHub Actions: 🔗 GHA
servers/rendering/renderer_viewport.cpp
[error] 1-1: clang-format: formatting changes were applied by the hook.
editor/settings/editor_settings.h
[error] 1-1: clang-format formatting check failed. Changes were made by the hook.
main/main.h
[error] 1-1: clang-format formatting check failed. Changes were made by the hook.
editor/settings/editor_settings.cpp
[error] 1-1: clang-format: formatting changes were applied by the hook.
main/main.cpp
[error] 1-1: clang-format: formatting changes were applied by the hook.
🔇 Additional comments (5)
main/main.cpp (1)
1933-1942: Fallback to project manager when project load fails looks correctOn
globals->setup()failure in tools builds, forcingeditor = falseandproject_manager = truemirrors the later “no project found ⇒ open project manager” logic and avoids hard erroring when no valid project is present. This is a sensible refinement of the startup behavior.editor/settings/editor_settings.cpp (4)
46-46: LGTM!The include is necessary for the new
get_setting_directlyfunction that usesVariantParser.
586-586: Good refactoring to centralize the default base color.Centralizing the default base color allows it to be reused in other parts of the codebase (such as boot splash initialization).
1289-1291: Verify the significant change in default base color.The default base color changed from
Color(0.2, 0.23, 0.31)toColor(0.06, 0.09, 0.14), which is considerably darker:
- Old: RGB(51, 59, 79)
- New: RGB(15, 23, 36)
This is a major visual change that will affect the default editor theme. Please confirm this is intentional and aligns with the design goals for the splash screen and editor theme.
1243-1287: No changes needed. Verification confirmsget_setting_directlyis called only once during boot splash initialization in_get_boot_splash_bg_color(). The current implementation is appropriate for this single early-startup use case. Performance concerns about multiple calls and caching suggestions are not applicable here.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
main/main.cpp (1)
3483-3485: Consider clarifying or simplifying the clear color setup logic.The default clear color is set at line 3485, then conditionally reset at lines 3851-3852 for non-editor/project-manager builds from the same project setting. This appears redundant for game builds.
If there's a specific reason for this two-phase setup (e.g., the setting might change between these points, or there are ordering dependencies), please add a comment explaining the intent. Otherwise, consider simplifying by removing the redundant assignment.
Also applies to: 3850-3853
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
main/main.cpp
🧰 Additional context used
🧬 Code graph analysis (1)
main/main.cpp (1)
editor/settings/editor_settings.cpp (6)
get_singleton(1221-1223)get_singleton(1221-1221)get_setting_directly(1247-1287)get_setting_directly(1247-1247)get_default_base_color(1289-1291)get_default_base_color(1289-1289)
🔇 Additional comments (3)
main/main.cpp (3)
3169-3170: LGTM - Early boot color setup looks correct.The early window clear color is properly set using the new helper function before display server initialization.
3823-3823: LGTM - Boot logo color usage is correct.The boot splash background color is now consistently retrieved using the centralized helper function.
1936-1947: The fallback to project manager mode when project loading fails is intentional and well-handled.This behavior is by design in TOOLS_ENABLED builds—when
globals->setup()fails, the engine gracefully falls back to project manager mode instead of aborting. The code appropriately handles this state:
- The
found_projectflag tracks whether project initialization succeeded- Subsequent initialization code treats
editor || project_managerconsistently (WorkerThreadPool, InputMap, rendering setup)- There's a secondary fallback at line ~2018:
project_manager = !found_project && !cmdline_tool;that reinforces this pattern- In non-TOOLS builds, the error path is correctly preserved
No changes needed.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
editor/settings/editor_settings.cpp (1)
1243-1291: Direct settings reader is useful; consider making the resource stub consume tokens
EditorSettings::get_setting_directly()cleanly encapsulates the early-on-disk read logic currently needed frommain.cpp, with conservative fallbacks on any failure paths and no impact on the regular EditorSettings lifecycle. Two small robustness notes:
_parse_direct_resource_dummy()currently just returnsOKwithout consuming any tokens. For settings whose values areResources, a future reuse ofget_setting_directly()could run into parsing issues. Mirroring the behavior of_parse_resource_dummyinmain.cpp(i.e., actually advancing past the resource expression) would make this safer.- Inside the loop you ignore non-
OK, non-ERR_FILE_EOFparse errors and continue. That’s acceptable for this narrow use-case (base color as aColor), but if the helper is reused more broadly, you might want to bail out early on the first parse error and returnp_defaultto avoid spinning on a malformed file.Functionally, for the current caller (
interface/theme/base_coloras aColor), this should work as intended and correctly falls back toget_default_base_color()when anything goes wrong.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
editor/settings/editor_settings.cppmain/main.cppmain/main.hservers/rendering/renderer_viewport.cpp
🚧 Files skipped from review as they are similar to previous changes (1)
- servers/rendering/renderer_viewport.cpp
🧰 Additional context used
🧬 Code graph analysis (2)
main/main.h (1)
main/main.cpp (2)
get_boot_splash_bg_color(3765-3776)get_boot_splash_bg_color(3765-3765)
main/main.cpp (1)
editor/settings/editor_settings.cpp (6)
get_singleton(1221-1223)get_singleton(1221-1221)get_setting_directly(1247-1287)get_setting_directly(1247-1247)get_default_base_color(1289-1291)get_default_base_color(1289-1289)
⏰ 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). (15)
- GitHub Check: 🐧 Linux / Template w/ Mono, debug (target=template_debug)
- GitHub Check: 🐧 Linux / Editor w/ Mono (target=editor)
- GitHub Check: 🐧 Linux / Template w/ Mono, release (target=template_release)
- GitHub Check: 🏁 Windows / Editor (target=editor)
- GitHub Check: 🐧 Linux / Editor with ThreadSanitizer (target=editor, dev_build=yes, use_tsan=yes, use_llvm=yes, linker=lld)
- GitHub Check: 🍎 macOS / Template (target=template_release)
- GitHub Check: 🐧 Linux / Minimal template (target=template_release, everything disabled)
- GitHub Check: 🌐 Web / Template w/o threads (target=template_release, threads=no)
- GitHub Check: 🏁 Windows / Template w/ GCC (target=template_release, use_mingw=yes)
- GitHub Check: 🍎 macOS / Editor (target=editor)
- GitHub Check: 🏁 Windows / Template (target=template_release)
- GitHub Check: 🌐 Web / Template w/ threads (target=template_release, threads=yes)
- GitHub Check: 🤖 Android / Template arm32 (target=template_release, arch=arm32)
- GitHub Check: 🤖 Android / Template arm64 (target=template_release, arch=arm64)
- GitHub Check: 🍏 iOS / Template (target=template_release)
🔇 Additional comments (8)
main/main.h (1)
38-41: New Color forward declaration and boot splash accessor look correctUsing a forward declaration for
Colorhere is sufficient for the static accessor declaration, and exposingMain::get_boot_splash_bg_color()as a central API for callers is a clean design choice with no obvious ABI or correctness issues.Also applies to: 73-79
main/main.cpp (5)
1938-1940: Treating failed project load as project manager context is reasonableSwitching to
project_manager = trueonglobals->setup()failure (TOOLS builds) makes subsequent initialization (thread pool sizing, display, hints) consistently follow the project manager path instead of a half-editor/half-game state. No functional regressions spotted.
3169-3171: Centralized boot background for early window clearUsing
Color boot_bg_color = get_boot_splash_bg_color();and passing it toDisplayServer::set_early_window_clear_color_override()ensures the OS window clear color matches the eventual splash background in both editor and game contexts. This is a solid cohesion improvement.
3484-3485: Default clear color fallback now aligned with boot splash colorUsing:
Color clear = GLOBAL_DEF_BASIC("rendering/environment/defaults/default_clear_color", get_boot_splash_bg_color());means a fresh project without an explicit default clear color will inherit the boot splash color, which is a sensible and backwards-compatible default.
3765-3776: get_boot_splash_bg_color() logic is sound and respects editor vs project settingsThe new accessor cleanly separates behavior:
In TOOLS builds with
editor || project_manager:
- Prefer
EditorSettings::get_singleton()->get_setting("interface/theme/base_color")when the singleton is initialized.- Otherwise, fall back to
EditorSettings::get_setting_directly(..., EditorSettings::get_default_base_color()), which safely reads the on-disk config or returns the internal default.In non-editor contexts, defer to
GLOBAL_DEF_BASIC("application/boot_splash/bg_color", boot_splash_bg_color)as before.This centralizes all splash background decisions and avoids duplicating the early-EditorSettings parsing logic in
main.cpp.
3823-3853: Boot logo now consistently uses centralized boot color, with correct post-boot clear reset
setup_boot_logo()now reusesget_boot_splash_bg_color()for both custom and default splash rendering, which keeps editor/project-manager and game boot visuals in sync with the chosen base color.- The new block:
if (!editor && !project_manager) { RenderingServer::get_singleton()->set_default_clear_color( GLOBAL_GET("rendering/environment/defaults/default_clear_color")); }ensures that for games the runtime clear color reverts to the project’s
default_clear_colorafter the boot logo, while editor/project-manager sessions keep using the editor base color as their default clear. This matches expectations for decoupling splash vs in-project background without breaking existing editor behavior.editor/settings/editor_settings.cpp (2)
49-49: VariantParser include is appropriate for new direct parsing helperAdding
#include "core/variant/variant_parser.h"is required for the new direct settings reader and is scoped correctly to this implementation file.
586-587: Base color default now shared via helper – good centralizationUsing
get_default_base_color()in:EDITOR_SETTING_BASIC(Variant::COLOR, PROPERTY_HINT_NONE, "interface/theme/base_color", get_default_base_color(), "")eliminates the hard-coded literal and ensures the editor default, boot splash background, and any other callers of
get_default_base_color()stay consistent.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
doc/classes/ProjectSettings.xml (1)
290-292: Fix dangling docs references to removedapplication/boot_splash/bg_color(breaks make-rst)
application/boot_splash/bg_coloris no longer a documented member, but two descriptions still link to it, causing the reported XML doc build failures. Reword those notes to drop the[member ...]reference.Proposed XML diff to remove broken member links
@@ - <member name="application/boot_splash/show_image" type="bool" setter="" getter="" default="true"> - If [code]true[/code], displays the image specified in [member application/boot_splash/image] when the engine starts. If [code]false[/code], only displays the plain color specified in [member application/boot_splash/bg_color]. - </member> + <member name="application/boot_splash/show_image" type="bool" setter="" getter="" default="true"> + If [code]true[/code], displays the image specified in [member application/boot_splash/image] when the engine starts. If [code]false[/code], only displays a plain background color. + </member> @@ - <member name="display/window/size/transparent" type="bool" setter="" getter="" default="false"> - If [code]true[/code], enables a window manager hint that the main window background [i]can[/i] be transparent. This does not make the background actually transparent. For the background to be transparent, the root viewport must also be made transparent by enabling [member rendering/viewport/transparent_background]. - [b]Note:[/b] To use a transparent splash screen, set [member application/boot_splash/bg_color] to [code]Color(0, 0, 0, 0)[/code]. - [b]Note:[/b] This setting has no effect if [member display/window/per_pixel_transparency/allowed] is set to [code]false[/code]. - [b]Note:[/b] This setting has no effect on Android as transparency is controlled only via [member display/window/per_pixel_transparency/allowed]. - </member> + <member name="display/window/size/transparent" type="bool" setter="" getter="" default="false"> + If [code]true[/code], enables a window manager hint that the main window background [i]can[/i] be transparent. This does not make the background actually transparent. For the background to be transparent, the root viewport must also be made transparent by enabling [member rendering/viewport/transparent_background]. + [b]Note:[/b] To use a transparent splash screen, ensure the boot splash background color is fully transparent, for example [code]Color(0, 0, 0, 0)[/code]. + [b]Note:[/b] This setting has no effect if [member display/window/per_pixel_transparency/allowed] is set to [code]false[/code]. + [b]Note:[/b] This setting has no effect on Android as transparency is controlled only via [member display/window/per_pixel_transparency/allowed]. + </member>Also applies to: 1026-1031
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
doc/classes/ProjectSettings.xml
🧰 Additional context used
🪛 GitHub Actions: 🔗 GHA
doc/classes/ProjectSettings.xml
[error] 1-1: make-rst: Unresolved member reference "application/boot_splash/bg_color" in property "application/boot_splash/show_image" description. 2 errors were found in the class reference XML.
[error] 1-1: make-rst: Unresolved member reference "application/boot_splash/bg_color" in property "display/window/size/transparent" description.
🔇 Additional comments (2)
doc/classes/ProjectSettings.xml (2)
2800-2802: Doc default forrendering/environment/defaults/default_clear_colorupdated correctlyThe new default color value is consistent with the PR’s refactor of splash/background color; the member docs stay accurate and self‑contained.
3163-3165: Mobile renderer override default switched togl_compatibilityThe updated default for
rendering/renderer/rendering_method.mobilematches the engine change and keeps the description valid.
|
@Arctis-Fireblight After reviewing this, i can see what is happening, i just don't know why. --doctool generates the new documents from the code with the member definition removed because i guess its not needed? But the CI fails because it wants the member to be defined. unclear. But if i add the member definition manually it failed to build the rst files because its a undefined member?? i'm confused atm.. Will work on it tomorrow. |
Remove Unneeded files
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI Agents
In @main/main.cpp:
- Around line 4030-4031: The global property "application/boot_splash/bg_color"
is only registered in the doctool-only path, so Main::get_boot_splash_bg_color()
(which calls GLOBAL_GET) can see an undefined/default Color at runtime; fix by
registering the global in the normal startup path (move or add the
GLOBAL_DEF(PropertyInfo(..., "application/boot_splash/bg_color"), Color(...))
outside the doctool-only block), e.g. in start() before the doctool check or
alongside other boot splash GLOBAL_DEF calls in setup(), or change
get_boot_splash_bg_color() to call GLOBAL_DEF instead of GLOBAL_GET so the
setting is defined on first access.
- Line 1939: The assignment project_manager = true in main.cpp's project-loading
error path appears unintended for non-tools builds; either remove this
assignment from the error path or wrap it with #ifdef TOOLS_ENABLED so it only
takes effect in tools builds, and ensure the project-loading failure handling
code (the branch that currently sets project_manager) remains unchanged for
non-tools builds so the variable is not erroneously toggled where project
manager code is not compiled.
🧹 Nitpick comments (2)
doc/classes/ProjectSettings.xml (1)
3165-3169: Mobile renderer override change is significant; confirm engine default matchesSwitching the documented mobile override from
"mobile"to"gl_compatibility"meaningfully changes expectations for new mobile projects. Please double‑check that:
- The core registration actually uses
"gl_compatibility"as the default forrendering/renderer/rendering_method.mobile, and- This aligns with current mobile support policy (Vulkan/Mobile vs GL Compatibility) so docs and runtime behavior stay in sync.
If this value comes from regenerated doctool output, that likely already reflects the engine default, but it's worth explicitly confirming given the behavioral impact.
main/main.cpp (1)
3851-3854: Consider adding a comment to explain the conditional logic.The conditional logic correctly ensures that in editor/project_manager mode, the clear color remains as the boot splash background color (editor theme color), while in game mode it uses the project's configured clear color. However, this behavior might not be immediately obvious to future maintainers.
🔎 Suggested documentation
+ // In editor/project_manager mode, keep the boot splash background color. + // In game mode, switch to the project's configured clear color. if (!editor && !project_manager) { RenderingServer::get_singleton()->set_default_clear_color( GLOBAL_GET("rendering/environment/defaults/default_clear_color")); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
doc/classes/ProjectSettings.xmlmain/main.cpp
🧰 Additional context used
🧬 Code graph analysis (1)
main/main.cpp (1)
editor/settings/editor_settings.cpp (6)
get_singleton(1221-1223)get_singleton(1221-1221)get_setting_directly(1247-1287)get_setting_directly(1247-1247)get_default_base_color(1289-1291)get_default_base_color(1289-1289)
⏰ 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). (15)
- GitHub Check: 🐧 Linux / Editor with ThreadSanitizer (target=editor, dev_build=yes, use_tsan=yes, use_llvm=yes, linker=lld)
- GitHub Check: 🐧 Linux / Minimal template (target=template_release, everything disabled)
- GitHub Check: 🐧 Linux / Template w/ Mono, debug (target=template_debug)
- GitHub Check: 🏁 Windows / Template (target=template_release)
- GitHub Check: 🤖 Android / Template arm32 (target=template_release, arch=arm32)
- GitHub Check: 🐧 Linux / Template w/ Mono, release (target=template_release)
- GitHub Check: 🐧 Linux / Editor w/ Mono (target=editor)
- GitHub Check: 🌐 Web / Template w/o threads (target=template_release, threads=no)
- GitHub Check: 🍎 macOS / Template (target=template_release)
- GitHub Check: 🌐 Web / Template w/ threads (target=template_release, threads=yes)
- GitHub Check: 🤖 Android / Template arm64 (target=template_release, arch=arm64)
- GitHub Check: 🏁 Windows / Template w/ GCC (target=template_release, use_mingw=yes)
- GitHub Check: 🍏 iOS / Template (target=template_release)
- GitHub Check: 🏁 Windows / Editor (target=editor)
- GitHub Check: 🍎 macOS / Editor (target=editor)
🔇 Additional comments (4)
doc/classes/ProjectSettings.xml (1)
2802-2804: Updated default clear color doc looks consistentThe new
default_clear_colorvalue is documented correctly and the description still matches how projects override it. No further changes needed here.main/main.cpp (3)
3169-3170: LGTM!Correctly uses the centralized
get_boot_splash_bg_color()function to set the early window clear color override during display server initialization.
3484-3485: LGTM!Correctly uses
get_boot_splash_bg_color()as the default value for therendering/environment/defaults/default_clear_colorsetting. This ensures the default clear color matches the boot splash background color.
3824-3825: LGTM!Both usages correctly centralize boot splash background color retrieval:
- Line 3824: For custom boot logo rendering
- Line 3837: For fallback default boot splash
The centralization improves maintainability and ensures consistent color handling.
Also applies to: 3837-3838
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI Agents
In @main/main.cpp:
- Line 3791: The GLOBAL_DEF for "application/boot_splash/bg_color" is defined
inside setup_boot_logo() but get_boot_splash_bg_color() (which uses GLOBAL_GET)
is called earlier in setup2(), causing an undefined/transparent color; move the
GLOBAL_DEF(PropertyInfo(...), Color(0.14,0.14,0.14)) into setup2() before the
first call to get_boot_splash_bg_color() (i.e., place it just before the
existing use in setup2()), and remove the duplicate GLOBAL_DEF from
setup_boot_logo() so the global is always defined before any GLOBAL_GET or
get_boot_splash_bg_color() invocation.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
core/config/project_settings.cppdoc/classes/ProjectSettings.xmlmain/main.cpp
✅ Files skipped from review due to trivial changes (1)
- core/config/project_settings.cpp
🚧 Files skipped from review as they are similar to previous changes (1)
- doc/classes/ProjectSettings.xml
⏰ 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). (15)
- GitHub Check: 🐧 Linux / Editor w/ Mono (target=editor)
- GitHub Check: 🐧 Linux / Template w/ Mono, debug (target=template_debug)
- GitHub Check: 🐧 Linux / Template w/ Mono, release (target=template_release)
- GitHub Check: 🐧 Linux / Minimal template (target=template_release, everything disabled)
- GitHub Check: 🍏 iOS / Template (target=template_release)
- GitHub Check: 🏁 Windows / Template (target=template_release)
- GitHub Check: 🏁 Windows / Editor (target=editor)
- GitHub Check: 🍎 macOS / Editor (target=editor)
- GitHub Check: 🐧 Linux / Editor with ThreadSanitizer (target=editor, dev_build=yes, use_tsan=yes, use_llvm=yes, linker=lld)
- GitHub Check: 🏁 Windows / Template w/ GCC (target=template_release, use_mingw=yes)
- GitHub Check: 🍎 macOS / Template (target=template_release)
- GitHub Check: 🌐 Web / Template w/o threads (target=template_release, threads=no)
- GitHub Check: 🤖 Android / Template arm32 (target=template_release, arch=arm32)
- GitHub Check: 🌐 Web / Template w/ threads (target=template_release, threads=yes)
- GitHub Check: 🤖 Android / Template arm64 (target=template_release, arch=arm64)
🔇 Additional comments (1)
main/main.cpp (1)
3764-3775: Well-structured helper function.The centralization of boot splash background color retrieval is clean and properly handles both editor and game contexts with appropriate fallbacks.
decryptedchaos
force-pushed
the
splash-screen-fix
branch
from
f00bae1 to
deb187d
Compare
2 participants
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.