fix: make projet able to have a working xmake repo#598
Conversation
# Pull Request ## Description I put examples relative to their plugins inside their corresponding plugins. I also added some other secondary examples. ## Related Issues (Put "None" if there are no related issues) close #537 ## Type of Change Please delete options that are not relevant. - Documentation update - Build/CI configuration change ## Changes Made List the main changes in this PR: - Moved examples to their plugins - Added primary and secondary examples kind - Added documentation inside BasicCoreUsage ## Testing Describe the tests you ran to verify your changes. Please delete options that are not relevant. - Code follows the project's style guidelines (`clang-format`) - Manual testing performed (describe what you tested): tried every examples ### 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. - I have added/updated comments in the code ## Checklist (Don't delete any options) - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [x] 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 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added Ncurses, Raylib, and SFML interactive example applications. * Reorganized examples with updated naming conventions. * Introduced BasicCoreUsage tutorial demonstrating core lifecycle. * **Documentation** * Updated example build instructions and naming standards. * Added README for BasicCoreUsage example. * **Chores** * Consolidated and streamlined example build configurations. * Removed deprecated example variants. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
# Pull Request ## Description I added a link and an explanation on how to check examples and indicated where is an "hello world" looks like. ## Related Issues (Put "None" if there are no related issues) close #527 ## Type of Change Please delete options that are not relevant. - Documentation update ## Changes Made List the main changes in this PR: - Added a link and an explanation on how to check examples - Indicated where is an "hello world" looks like ## Testing Describe the tests you ran to verify your changes. Please delete options that are not relevant. None ### 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. - I have updated the relevant documentation ## Checklist (Don't delete any options) - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [x] 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 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Updated links to Lights and Material example code to reflect new repository structure. * Added "Hello World" section with guidance on accessing basic examples across example folders. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Description Adds a color utility helper to simplify RGB-to-vec3 conversion as requested in issue #498. ## What's New - `Color::FromRGB(uint8_t r, uint8_t g, uint8_t b)` - Converts 0-255 integer RGB values to normalized glm::vec3 - `Color::FromHex(uint32_t hex)` - Converts hex color codes to normalized glm::vec3 (bonus feature) ## Why This is Needed Eliminates verbose manual division by 255.0f for each channel, reducing boilerplate and potential for errors. ## Changes Made - Added `src/plugin/graphic/src/utils/Color.hpp` with two static helper methods - Included comprehensive Doxygen documentation with usage examples in code comments ## Related Issue Closes #498 ## Type of Change - [x] New feature (non-breaking change which adds functionality) ## Testing - [x] Code follows the project's style guidelines (clang-format) - [x] Manual testing performed (verified file creation and syntax) ### Test Environment - OS: Windows - Compiler: MSVC ## Documentation - [x] I have added/updated comments in the code ## Checklist - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [x] My changes generate no new warnings ## Breaking Changes None ## Additional Notes The Color utility is a simple, inline header-only implementation with no external dependencies beyond GLM, which is already used throughout the graphics module. Co-authored-by: Miouzora <91665686+Miou-zora@users.noreply.github.com>
…#587) # Pull Request ## Description I specified how to write an Architecture Decision Record (ADR) to mark any big architectural change in the project. ## Related Issues (Put "None" if there are no related issues) close #532 ## Type of Change Please delete options that are not relevant. - Documentation update ## Changes Made List the main changes in this PR: - Added ADR template inside `docs/decisions` ## Testing Describe the tests you ran to verify your changes. Please delete options that are not relevant. None ### 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. - I have updated the relevant documentation ## Checklist (Don't delete any options) - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [x] 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 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added a new Architecture Decision Record (ADR) template to standardize ADR entries. Includes front-matter fields for metadata (identifier, title, status, references, dependencies) and a structured body with sections for status, date, context/problem, decision drivers, considered options, chosen outcome and justification, optional consequences, confirmation steps, pros/cons per option, further info, and abandonment notes. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
# Pull Request ## Description I added a `.md` file that describes what is the `tools` folder. ## Related Issues (Put "None" if there are no related issues) close #575 ## Type of Change Please delete options that are not relevant. - Documentation update ## Changes Made List the main changes in this PR: - Added a `.md` file that describes what is the `tools` folder. ## Testing Describe the tests you ran to verify your changes. Please delete options that are not relevant. None ### 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. - I have updated the relevant documentation ## Checklist (Don't delete any options) - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [x] 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 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added guide documenting the tools directory structure and available contributor utilities, including AI-focused helpers and xmake configuration resources. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
# Pull Request ## Description To ensure that the codebase follows good principles like modern usage of cpp, I added clang tidy configuration file with an automatic check inside CI. ## Related Issues (Put "None" if there are no related issues) close #553 ## Type of Change Please delete options that are not relevant. - Build/CI configuration change ## Changes Made List the main changes in this PR: - Added clang tidy config file - Add a gate in CI to check the code conformity to clang tidy ## 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) - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [x] 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 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Enforced stricter static-analysis rules (treat warnings as errors) and scoped checks to source files with repository formatting. * Added CI automation to run these static-analysis checks. * Aligned internal log level definitions with the logging backend for more consistent log behavior. * **Bug Fixes** * Improved cleanup behavior during constraint finalization to reduce errors and improve runtime stability when exceptions occur. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Copilot <copilot@github.com>
…time spent field (#592) # Pull Request ## Description I added a new field (Time spent) for PRs ## Related Issues (Put "None" if there are no related issues) Fixes #560 ## Type of Change Please delete options that are not relevant. - Documentation update ## Changes Made List the main changes in this PR: - Added a Time spent field - Put some text in commentary - Added a new change type ## Testing Describe the tests you ran to verify your changes. Please delete options that are not relevant. None ### 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) - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [x] 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 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Updated pull request template to improve development workflow organization and structure. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
# Pull Request ## Description Add optional system to close window ## Related Issues (Optional, put "None" if there are no related issues) Fixes #515 ## Type of Change <!-- Please delete options that are not relevant. --> - New feature (non-breaking change which adds functionality) ## Changes Made List of the main changes in this PR: - Add optional system to close window ## Testing (Optional, put "None" if you didn't have to test anything) <!-- Describe the tests you ran to verify your changes. Please delete options that are not relevant. --> - Manual testing performed (describe what you tested): added it in example and run it ### Test Environment - OS: macOS - Compiler: Clang ## Documentation <!-- Please delete options that are not relevant. --> - No documentation changes are required ## Checklist (Don't delete any options) - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [x] Any dependent changes have been merged and published ## Time Spent (Optional, put "None" if you don't know) 5 minutes ## Screenshots/Videos (Optional, put "None" if it's not relevant) <!-- Add screenshots or videos to help explain your changes. --> None ## Breaking Changes (Optional, Put "None" if there are no breaking changes) <!-- If this PR introduces breaking changes, describe them here and provide migration instructions. --> None ## Additional Notes (Optional, Put "None" if there are no additional notes) <!-- Any additional information that reviewers should know. --> None <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Refactor** * Reorganized escape key handling logic to improve code structure and maintainability. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
# Pull Request ## Description Added a new camera behavior (OrbitalChaseBehavior) that rotates around an entity. I also added an example to tests camera's behaviors. ## Related Issues (Optional, put "None" if there are no related issues) Fixes #491 ## Type of Change - New feature (non-breaking change which adds functionality) ## Changes Made List of the main changes in this PR: - New OrbitalChaseBehavior - New example ## Testing (Optional, put "None" if you didn't have to test anything) - Code follows the project's style guidelines (`clang-format`) - Manual testing performed (describe what you tested): run `CameraMovementUsage` example ### Test Environment - OS: macOS - Compiler: Clang ## Documentation - 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 ## Time Spent (Optional, put "None" if you don't know) 2 hours ## Screenshots/Videos (Optional, put "None" if it's not relevant) None ## Breaking Changes (Optional, Put "None" if there are no breaking changes) <!-- If this PR introduces breaking changes, describe them here and provide migration instructions. --> None ## Additional Notes (Optional, Put "None" if there are no additional notes) <!-- Any additional information that reviewers should know. --> None <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Orbital chase camera behavior: mouse-driven orbit, scroll zoom, configurable distance limits, and automatic target following. * **Examples** * Added a standalone camera movement example app demonstrating up/down arrow switching between default, static, and orbital-chase behaviors, with on-screen/logged feedback and a clear startup entry for running the demo. The example wires graphics error reporting to surface device faults during initialization. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
# Pull Request ## Description This PR was mainly to add Discord link in readme but ended up to fix some typos and add better explanation of example folders. ## Related Issues (Optional, put "None" if there are no related issues) None ## Type of Change <!-- Please delete options that are not relevant. --> - Project related stuff ## Changes Made List of the main changes in this PR: - Added Discord link - Renamed "basic_core_usage" to "BasicCoreUsage" - Explain where user can find examples ## Testing (Optional, put "None" if you didn't have to test anything) <!-- Describe the tests you ran to verify your changes. Please delete options that are not relevant. --> None ### Test Environment - OS: macOS - Compiler: Clang ## Documentation <!-- Please delete options that are not relevant. --> - I have updated the relevant documentation ## Checklist (Don't delete any options) - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [x] Any dependent changes have been merged and published ## Time Spent (Optional, put "None" if you don't know) <!-- Put here the time you were actively working on it like: 5 minutes, 4 hours, some days, at least 1 month --> 5 minutes ## Screenshots/Videos (Optional, put "None" if it's not relevant) <!-- Add screenshots or videos to help explain your changes. --> None ## Breaking Changes (Optional, Put "None" if there are no breaking changes) <!-- If this PR introduces breaking changes, describe them here and provide migration instructions. --> None ## Additional Notes (Optional, Put "None" if there are no additional notes) <!-- Any additional information that reviewers should know. --> None --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
# Pull Request ## Description I clarified Context by removing it and splitting it into each attribute to corresponding resources. ## Related Issues (Optional, put "None" if there are no related issues) Fixes #452 ## Type of Change - Code refactoring ## Changes Made List of the main changes in this PR: - Splitter Context class ## Testing (Optional, put "None" if you didn't have to test anything) <!-- Describe the tests you ran to verify your changes. Please delete options that are not relevant. --> - Unit tests pass (`xmake test`) - Code follows the project's style guidelines (`clang-format`) ### Test Environment - OS: macOS - Compiler: Clang ## Documentation <!-- Please delete options that are not relevant. --> - No documentation changes are required ## Checklist (Don't delete any options) - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [x] Any dependent changes have been merged and published ## Time Spent (Optional, put "None" if you don't know) 3~5 hours ## Screenshots/Videos (Optional, put "None" if it's not relevant) None ## Breaking Changes (Optional, Put "None" if there are no breaking changes) <!-- If this PR introduces breaking changes, describe them here and provide migration instructions. --> While using context, you should switch to the resource you want rather than using Context class. ## Additional Notes (Optional, Put "None" if there are no additional notes) <!-- Any additional information that reviewers should know. --> None <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Refactor** * Restructured rendering resource management to use granular resource types (DeviceContext, Queue, Adapter, Instance, Surface) instead of a combined Context abstraction, improving separation of concerns across graphics initialization, buffer management, and render pipeline systems. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/EngineSquared/EngineSquared/pull/596?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
… string keys allow per attribute setters (#597) # Pull Request ## Description I added a way to patch components to add an `on_update` mechanism. This allowed to update `Material` class when a texture is changed. I also added a way to add a string in key of `ResourceManager` ## Related Issues (Optional, put "None" if there are no related issues) Fixes #497 ## Type of Change <!-- Please delete options that are not relevant. --> - New feature (non-breaking change which adds functionality) ## Changes Made List of the main changes in this PR: - Add `UpdateComponent` in `Entity` and `EntityId` - Add `on_update` callback when a material is modified though `UpdateComponent` - Add a way to put string, string_view in methods of `ResourceManager` ## Testing (Optional, put "None" if you didn't have to test anything) <!-- Describe the tests you ran to verify your changes. Please delete options that are not relevant. --> - Unit tests pass (`xmake test`) - Code follows the project's style guidelines (`clang-format`) ### Test Environment - OS: macOS - Compiler: Clang ## Documentation <!-- Please delete options that are not relevant. --> - I have added/updated comments in the code ## Checklist (Don't delete any options) - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [x] Any dependent changes have been merged and published ## Time Spent (Optional, put "None" if you don't know) <!-- Put here the time you were actively working on it like: 5 minutes, 4 hours, some days, at least 1 month --> 2 hours ## Screenshots/Videos (Optional, put "None" if it's not relevant) <!-- Add screenshots or videos to help explain your changes. --> None ## Breaking Changes (Optional, Put "None" if there are no breaking changes) <!-- If this PR introduces breaking changes, describe them here and provide migration instructions. --> None ## Additional Notes (Optional, Put "None" if there are no additional notes) <!-- Any additional information that reviewers should know. --> None <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added component update mechanism enabling dynamic modifications to entity properties * Extended resource management system with string-based identifier support * Implemented automatic material synchronization between CPU and GPU components * **Tests** * Added comprehensive test coverage for string-based resource operations <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/EngineSquared/EngineSquared/pull/597?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
Warning Review limit reached
Your plan currently allows 1 review/hour. Refill in 51 minutes and 45 seconds. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more review capacity refills, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughBuild system header inclusion patterns are modernized across multiple plugin targets to dynamically enumerate subdirectories instead of hardcoding header paths. Surface configuration adapter acquisition becomes conditional, allowing initialization without a required adapter resource. ChangesBuild system and runtime resource handling
🎯 2 (Simple) | ⏱️ ~12 minutes Suggested labels
🚥 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/plugin/camera-movement/xmake.lua (1)
44-48: ⚖️ Poor tradeoffConsider extracting the header iteration pattern to reduce duplication.
This dynamic header inclusion loop is duplicated in
src/plugin/physics/xmake.lua(lines 19-23). Consider extracting this pattern into a shared xmake helper function if this pattern will be used across multiple plugin build scripts.🤖 Prompt for 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. In `@src/plugin/camera-movement/xmake.lua` around lines 44 - 48, Extract the duplicated header-iteration loop into a shared xmake helper function (e.g., add_plugin_headers(root_dir) or add_headers_for_dir) placed in a common build helper module and require it from plugin xmake.lua files; move the for _, file in ipairs(os.filedirs("src/*")) ... add_headerfiles("src/(" .. path.filename(file) .. "/**.hpp)") logic into that helper, then replace the loop in src/plugin/camera-movement/xmake.lua and src/plugin/physics/xmake.lua with a single call to the helper (passing "src" or the plugin subdir as needed) so both scripts reuse the same implementation.src/plugin/physics/xmake.lua (2)
19-23: ⚡ Quick winSkip adding
.ippfor physics plugin unless.ippfiles are introduced
src/plugin/physics/xmake.luacurrently only adds*.hppundersrc/*subdirectories; there are no.ippfiles present undersrc/plugin/physics/src/, so the extra.ippinclusion isn’t needed right now. If.ippfiles are added later, extend the glob accordingly.🤖 Prompt for 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. In `@src/plugin/physics/xmake.lua` around lines 19 - 23, Currently the xmake script loops over os.filedirs("src/*") and calls add_headerfiles with a glob that only includes "*.hpp"; to follow the review, do not add "*.ipp" here—leave add_headerfiles("src/(" .. path.filename(file) .. "/*.hpp)") as the header inclusion and avoid adding any "*.ipp" pattern (only add .ipp later if .ipp files are actually introduced); locate the loop using os.filedirs, os.isdir and the add_headerfiles call in src/plugin/physics/xmake.lua and ensure no "*.ipp" glob is added.
19-23: ⚡ Quick winPhysics header glob is sufficient for current layout; recursive glob is optional
src/plugin/physics/xmake.luausessrc/(<subdir>/*.hpp), and the currentsrc/plugin/physics/src/*layout has no nested.hppfiles beyond the immediate subdirectories—so it won’t miss headers today. Switching to/**.hppwould be a consistency/future-proofing improvement, not a correctness fix.🤖 Prompt for 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. In `@src/plugin/physics/xmake.lua` around lines 19 - 23, Update the header glob to be future-proof: in the loop that iterates src/* and calls add_headerfiles("src/(" .. path.filename(file) .. "/*.hpp)"), change the pattern to include recursive matches (e.g. use /**.hpp instead of /* .hpp) so add_headerfiles uses "src/(" .. path.filename(file) .. "/**.hpp)"; keep the same loop and path.filename usage but replace the glob to ensure nested headers are picked up.
🤖 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.
Nitpick comments:
In `@src/plugin/camera-movement/xmake.lua`:
- Around line 44-48: Extract the duplicated header-iteration loop into a shared
xmake helper function (e.g., add_plugin_headers(root_dir) or
add_headers_for_dir) placed in a common build helper module and require it from
plugin xmake.lua files; move the for _, file in ipairs(os.filedirs("src/*")) ...
add_headerfiles("src/(" .. path.filename(file) .. "/**.hpp)") logic into that
helper, then replace the loop in src/plugin/camera-movement/xmake.lua and
src/plugin/physics/xmake.lua with a single call to the helper (passing "src" or
the plugin subdir as needed) so both scripts reuse the same implementation.
In `@src/plugin/physics/xmake.lua`:
- Around line 19-23: Currently the xmake script loops over os.filedirs("src/*")
and calls add_headerfiles with a glob that only includes "*.hpp"; to follow the
review, do not add "*.ipp" here—leave add_headerfiles("src/(" ..
path.filename(file) .. "/*.hpp)") as the header inclusion and avoid adding any
"*.ipp" pattern (only add .ipp later if .ipp files are actually introduced);
locate the loop using os.filedirs, os.isdir and the add_headerfiles call in
src/plugin/physics/xmake.lua and ensure no "*.ipp" glob is added.
- Around line 19-23: Update the header glob to be future-proof: in the loop that
iterates src/* and calls add_headerfiles("src/(" .. path.filename(file) ..
"/*.hpp)"), change the pattern to include recursive matches (e.g. use /**.hpp
instead of /* .hpp) so add_headerfiles uses "src/(" .. path.filename(file) ..
"/**.hpp)"; keep the same loop and path.filename usage but replace the glob to
ensure nested headers are picked up.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 252cc044-076d-46de-927c-6b5e127ef0b2
📒 Files selected for processing (4)
src/engine/xmake.luasrc/plugin/camera-movement/xmake.luasrc/plugin/graphic/src/system/initialization/ConfigureSurface.cppsrc/plugin/physics/xmake.lua
|



Pull Request
Description
Related Issues (Optional, put "None" if there are no related issues)
Fixes #(issue)
Relates to #(issue)
Type of Change
Changes Made
List of the main changes in this PR:
Testing (Optional, put "None" if you didn't have to test anything)
xmake test)clang-format)Test Environment
Documentation
Checklist (Don't delete any options)
Time Spent (Optional, put "None" if you don't know)
Screenshots/Videos (Optional, put "None" if it's not relevant)
Breaking Changes (Optional, Put "None" if there are no breaking changes)
Additional Notes (Optional, Put "None" if there are no additional notes)
Summary by CodeRabbit
Bug Fixes
Chores