FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix(UIContext): prevent rendering when document is not loaded by Miou-zora · Pull Request #557 · EngineSquared/EngineSquared · GitHub

fix(UIContext): prevent rendering when document is not loaded - #557

Merged
Miou-zora merged 5 commits into
mainfrom
490-bug-spurious-rmlui-not-ready-to-render-warning-at-startup
Apr 4, 2026
Merged

fix(UIContext): prevent rendering when document is not loaded#557
Miou-zora merged 5 commits into
mainfrom
490-bug-spurious-rmlui-not-ready-to-render-warning-at-startup

Conversation

Miou-zora commented Apr 4, 2026
edited
Loading

Copy link
Copy Markdown
Contributor

Pull Request

Description

I added a check to avoid printing a warning when no document is used. I also fix xmake cache and ai prompt for commit message generation.

Related Issues (Put "None" if there are no related issues)

close #490

Type of Change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)

Changes Made

List the main changes in this PR:

  • Added a check just before running Render method

Testing

Describe the tests you ran to verify your changes. Please delete options that are not relevant.

  • Unit tests pass (xmake test)

Test Environment

  • OS: macOS
  • Compiler: Clang

Screenshots/Videos (Put "None" if there are no related issues)

None

Documentation

Please delete options that are not relevant.

  • No documentation changes are required

Checklist (Don't delete any options)

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published

Breaking Changes (Put "None" if there are no related issues)

None

Additional Notes (Put "None" if there are no related issues)

None

Summary by CodeRabbit

  • Bug Fixes

    • Fixed a rendering stability issue that could occur when no active document was loaded.
  • Chores

    • Improved CI cache configuration to make builds and checks more reliable across platforms.

Miou-zora self-assigned this Apr 4, 2026
Miou-zora added the quality of life Improvements that enhance user or developer experience without adding new core features label Apr 4, 2026
Miou-zora linked an issue Apr 4, 2026 that may be closed by this pull request

coderabbitai Bot commented Apr 4, 2026
edited
Loading

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

Added a null-document guard in RmlUI rendering to avoid rendering when no main document is loaded; updated GitHub Actions xmake cache keys to use the job matrix OS variable instead of the runner OS.

Changes

Cohort / File(s) Summary
RmlUI Rendering Guard
src/plugin/rmlui/src/resource/UIContext.cpp
Inserted early-return in UIContext::Render() when _document is nullptr, preventing render calls with no active document.
CI xmake Cache Keys
.github/workflows/ci.yml
Replaced ${{ runner.os }} with ${{ matrix.os }} in xmake cache key definitions across CI jobs to scope caches per matrix OS.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A small guard hopped into place,
If no document, skip the race,
CI caches tuned per OS,
Logs stay tidy, seeds of gloss,
I nibble bugs and leave a trace 🥕

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Out of Scope Changes check ❓ Inconclusive The PR includes unrelated CI workflow changes to GitHub Actions cache configuration that appear disconnected from the UIContext bug fix objective. Clarify whether the GitHub Actions cache changes are intentional or should be reverted to keep the PR focused on the UIContext rendering fix.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main code change: adding a guard to prevent rendering when the document is not loaded, directly addressing the linked issue.
Linked Issues check ✅ Passed The PR adds an early-return guard in UIContext::Render when _document is nullptr, which prevents the spurious warning by avoiding rendering attempts before initialization completes.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches 📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 490-bug-spurious-rmlui-not-ready-to-render-warning-at-startup

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

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai Bot left a comment

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
.github/workflows/ci.yml (1)

56-67: ⚠️ Potential issue | 🔴 Critical

matrix.os is undefined in the lint_code job — cache keys will be empty.

The lint_code job does not define a matrix strategy (it runs directly on ubuntu-latest at line 42), so ${{ matrix.os }} evaluates to an empty string. This will result in empty or invalid cache keys, causing cache misses or unexpected behavior.

Either revert to ${{ runner.os }} for this job, or add a matrix strategy if multi-OS linting is intended.

🔧 Proposed fix to revert to runner.os for this job
       - name: Install xmake
         uses: xmake-io/github-action-setup-xmake@fadadea1162ec75ce1541d5bb68226fb147c221e
         with:
           xmake-version: latest
           actions-cache-folder: ".xmake-cache"
-          actions-cache-key: ${{ matrix.os }}
+          actions-cache-key: ${{ runner.os }}
           build-cache: true
-          build-cache-key: ${{ matrix.os }}
+          build-cache-key: ${{ runner.os }}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/ci.yml around lines 56 - 67, The cache keys in the
lint_code job use the undefined variable matrix.os (seen in actions-cache-key
and build-cache-key), causing empty keys; update those keys to use runner.os
instead (or alternatively add a matrix strategy for the job if you intended
multi-OS linting). Locate the lint_code job where Install xmake uses
actions-cache-key: ${{ matrix.os }} and build-cache-key: ${{ matrix.os }}, and
replace those occurrences with ${{ runner.os }} (or add a matrix with an os axis
and use matrix.os consistently).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In @.github/workflows/ci.yml:
- Around line 56-67: The cache keys in the lint_code job use the undefined
variable matrix.os (seen in actions-cache-key and build-cache-key), causing
empty keys; update those keys to use runner.os instead (or alternatively add a
matrix strategy for the job if you intended multi-OS linting). Locate the
lint_code job where Install xmake uses actions-cache-key: ${{ matrix.os }} and
build-cache-key: ${{ matrix.os }}, and replace those occurrences with ${{
runner.os }} (or add a matrix with an os axis and use matrix.os consistently).

ℹ️ Review info ⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7b046c48-6a6a-415e-8068-34973a800eaf

📥 Commits

Reviewing files that changed from the base of the PR and between 393a31e and f4d25b9.

📒 Files selected for processing (1)
  • .github/workflows/ci.yml

sonarqubecloud Bot commented Apr 4, 2026

Copy link
Copy Markdown

Miou-zora merged commit 3aaa52f into main Apr 4, 2026
29 checks passed
Miou-zora deleted the 490-bug-spurious-rmlui-not-ready-to-render-warning-at-startup branch April 4, 2026 20:39
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

quality of life Improvements that enhance user or developer experience without adding new core features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Spurious RmlUI "not ready to render" warning at startup

1 participant


Back | FazBrowse Home | New Git URL