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

fix(NativeScripting): compilation errors from latest refactor by Divengerss · Pull Request #468 · EngineSquared/EngineSquared · GitHub

fix(NativeScripting): compilation errors from latest refactor - #468

Merged
ripel2 merged 3 commits into
mainfrom
fix/native-scripting-example
Feb 2, 2026
Merged

fix(NativeScripting): compilation errors from latest refactor#468
ripel2 merged 3 commits into
mainfrom
fix/native-scripting-example

Conversation

Divengerss commented Feb 2, 2026
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Contributor

Not related to any issue.

This PR fixes the Native Scripting example compilation.

Summary by CodeRabbit

  • New Features

    • Example script now emits a "destroyed" log message when the script instance is torn down.
  • Chores

    • Minor maintenance to example code and initialization behavior.

Note: This release includes a visible log on script destruction in the example.

Divengerss self-assigned this Feb 2, 2026

coderabbitai Bot commented Feb 2, 2026
edited
Loading

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

Added an explicit public constructor and a final destructor to the TestScript class in the native scripting example; the constructor forwards the Entity to the ScriptableEntity base via an initializer list, and the destructor prints a destruction message.

Changes

Cohort / File(s) Summary
TestScript Lifecycle
examples/nativescripting_usage/src/main.cpp
Added explicit TestScript(Engine::Entity entity) : ScriptableEntity(entity) {} and ~TestScript() final { std::cout << "TestScript Destroyed" << std::endl; }. No changes to OnCreate, OnUpdate, or OnDestroy logic.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐇 I hopped into code with a curious grin,
Built a tiny ctor, let the entity in.
When the last line's run and the log is flushed,
I whisper "Destroyed" as the memory's hushed.
— a rabbit, nibbling on bytes 🥕

🚥 Pre-merge checks | ✅ 1 | ❌ 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.
Title check ❓ Inconclusive The title references fixing compilation errors from a refactor, but the actual changes add explicit constructor and destructor to TestScript for SonarQube compliance, which are preventive improvements rather than fixes to compilation errors. Clarify whether this addresses actual compilation failures or implements SonarQube compliance improvements. Consider updating the title to better reflect the actual changes (e.g., 'Add explicit constructor and destructor to TestScript').
✅ Passed checks (1 passed)
Check name Status Explanation
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
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/native-scripting-example

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.

ripel2 requested a review from a team February 2, 2026 20:14
ripel2 merged commit fd398a2 into main Feb 2, 2026
1 of 4 checks passed
ripel2 deleted the fix/native-scripting-example branch February 2, 2026 20:20

sonarqubecloud Bot commented Feb 2, 2026

Copy link
Copy Markdown

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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants


Back | FazBrowse Home | New Git URL