feat(logging): enhance system error logging with system names by Miou-zora · Pull Request #454 · EngineSquared/EngineSquared · GitHub

This pull request introduces improvements to function and system identification and logging throughout the engine and utility code. The main focus is on adding support for retrieving and logging the human-readable names of systems and functions, in addition to their IDs. This enhancement improves debugging and log clarity, especially when handling errors or duplicate registrations. The PR also updates related tests to ensure correct error policy handling.

Function and System Naming Enhancements:

Added a pure virtual GetName() method to the BaseFunction interface, requiring all derived function wrappers to provide a human-readable name.
Implemented the GetName() method and a static GetCallableName() utility in both CallableFunction and WrappedSystem classes, which use RTTI to provide class names or fallback to IDs for non-class callables. [1] [2] [3] [4] [5] [6]
Updated AScheduler::RunSystem to log system names instead of IDs on errors, improving log readability.

Logging and Duplicate Handling Improvements:

Enhanced duplicate function registration warnings in FunctionContainer to include the function's name, both for direct and pointer-based additions. [1] [2]

Test Enhancements:

Updated system tests to explicitly set the error policy to Nothing for all schedulers, ensuring consistent error handling behavior during testing. [1] [2] [3] [4] [5] [6]
Improved error handling tests to use EXPECT_THROW for better assertion of expected exceptions. [1] [2]

Summary by CodeRabbit

Improvements
- Logs and warnings now display human-readable system and function names (demangled when available) instead of numeric IDs.
New Features
- Systems and callables expose runtime names for clearer reporting and diagnostics.
Tests
- Tests updated to configure scheduler error policy and now expect exceptions for the revised error-handling behavior.

📝 Walkthrough

Walkthrough

Expose human-readable callable/system names (demangled when available) via new GetName() APIs, propagate those names into logs and duplicate warnings, add a demangling utility, and update tests to set scheduler error policy and expect exceptions for error scenarios.

Changes

Cohort / File(s)	Summary
Scheduler / System src/engine/src/scheduler/AScheduler.cpp, src/engine/src/system/WrappedSystem.hpp	AScheduler log messages now use system->GetName(). WrappedSystem computes/stores a _name via GetCallableName(), exposes GetName(), and changes GetCallableID to take a const reference. Uses demangling for class-type callables.
Function container interface src/utils/function-container/src/BaseFunction.hpp	Adds pure virtual virtual std::string GetName() const to the BaseFunction interface (requires implementors to provide names).
Callable function & container src/utils/function-container/src/CallableFunction.hpp, src/utils/function-container/src/FunctionContainer.inl	CallableFunction now computes/stores _name, adds GetName() and GetCallableName(). FunctionContainer::AddFunction and duplicate warnings now include function names (uses GetName() or GetCallableName()).
Demangle utility src/utils/function-container/src/Demangle.hpp	New inline FunctionUtils::DemangleTypeName(const std::type_info&) returns demangled type names when supported, falling back to type_info::name() otherwise.
Tests src/engine/tests/engine/SystemTest.cpp	Tests now call core.SetErrorPolicyForAllSchedulers(Scheduler::SchedulerErrorPolicy::Nothing) in several cases and two tests changed to EXPECT_THROW(core.RunSystems(), std::runtime_error) to expect errors.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐇 I hop through headers, demangle names with cheer,
New GetName()s whisper so the logs appear.
Warnings wear labels, systems show their face,
Tests now expect tumults in the right place.
🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1 ❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'feat(logging): enhance system error logging with system names' accurately describes the main change of updating AScheduler to log system names instead of IDs and enhancing overall logging visibility.

✏️ 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 make-system-logging-more-explicit

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 review

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

…leFunction

Actionable comments posted: 1

Caution

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

⚠️ Outside diff range comments (1)

src/engine/src/system/WrappedSystem.hpp (1)
69-97: ⚠️ Potential issue | 🟠 Major

Change GetCallableID and GetCallableName to accept const reference to avoid requiring copies of move-only callables.

Passing _system (an lvalue) to by-value GetCallableID and GetCallableName parameters requires a copy. For move-only callables (e.g., lambdas with move-only captures), this fails to compile. Use const TSystem& instead, which is idiomatic for std::hash operators and avoids unnecessary copies.
Suggested change
-    static FunctionUtils::FunctionID GetCallableID(TSystem callable)
+    static FunctionUtils::FunctionID GetCallableID(const TSystem &callable)
     {
         if constexpr (std::is_class_v<TSystem>)
         {
             return typeid(callable).hash_code();
         }
         else
         {
             return std::hash<TSystem>{}(callable);
         }
     }

-    static std::string GetCallableName(TSystem callable)
+    static std::string GetCallableName(const TSystem &callable)
     {
         if constexpr (std::is_class_v<TSystem>)
         {
             return typeid(callable).name();
         }
         else
         {
             return std::to_string(GetCallableID(callable));
         }
     }

🤖 Fix all issues with AI agents

In `@src/engine/src/system/WrappedSystem.hpp`:
- Around line 89-92: The code uses typeid(callable).name() inside WrappedSystem
(see the return in the constexpr branch) which yields mangled names on
GCC/Clang; change this to produce human-readable names by demangling on
platforms that need it (e.g., call abi::__cxa_demangle or boost::core::demangle
when compiling with GCC/Clang) and fall back to typeid(...).name() on other
compilers, and also add an optional explicit name override mechanism (e.g.,
accept a provided name string in the WrappedSystem constructor or a static
trait) so callers can supply portable names when demangling is unavailable.
Ensure the change targets the code path that currently returns
typeid(callable).name() and preserves noexcept/ownership semantics for the
returned string.

…System and CallableFunction

Actionable comments posted: 3

🤖 Fix all issues with AI agents

In `@src/engine/src/system/WrappedSystem.hpp`:
- Around line 4-7: Remove the non-portable include <cxxabi.h> from
WrappedSystem.hpp since this header does not use any ABI APIs directly; keep the
existing include of "Demangle.hpp" and ensure all demangling calls rely on
FunctionUtils::DemangleTypeName() from Demangle.hpp (so no references to cxxabi
symbols remain in WrappedSystem.hpp). Update the includes in WrappedSystem.hpp
to only what is required (e.g., "Demangle.hpp", <string>, <typeinfo>) so the
file builds on MSVC and other platforms.

In `@src/utils/function-container/src/Demangle.hpp`:
- Around line 18-27: The current demangling code may leak memory because
abi::__cxa_demangle can allocate even when status != 0; ensure that before
returning typeInfo.name() you check if demangledName is non-null and call
std::free(demangledName). Update the block around demangledName (the variable
returned by __cxa_demangle) in Demangle.hpp so that in the else/failure branch
you free demangledName if it was allocated, then return typeInfo.name().
- Around line 3-6: The Demangle.hpp include of <cxxabi.h> and any use of
abi::__cxa_demangle must be guarded for non-MSVC platforms; wrap the `#include`
<cxxabi.h> with a platform check (e.g. `#if` !defined(_MSC_VER) / `#endif`) and
update the demangle function (the symbol-demangling function in Demangle.hpp,
e.g., demangle or demangleName) to call abi::__cxa_demangle only when that
platform macro is not set, otherwise simply return the input typeInfo.name()
string; ensure the fallback preserves signature and behavior (no exception/alloc
changes) so callers work on MSVC.

🧹 Nitpick comments (1)

src/engine/src/system/WrappedSystem.hpp (1)
72-98: Change GetCallableID and GetCallableName to accept const TSystem&.

The functions only read from the parameter and never modify it. Using const TSystem& improves const-correctness and allows these functions to accept temporaries without unnecessary restriction, while maintaining efficiency for lvalue references.
♻️ Proposed adjustment
-static FunctionUtils::FunctionID GetCallableID(TSystem &callable)
+static FunctionUtils::FunctionID GetCallableID(const TSystem &callable)
 {
     if constexpr (std::is_class_v<TSystem>)
     {
         return typeid(callable).hash_code();
     }
     else
     {
         return std::hash<TSystem>{}(callable);
     }
 }
 ...
-static std::string GetCallableName(TSystem &callable)
+static std::string GetCallableName(const TSystem &callable)
 {
     if constexpr (std::is_class_v<TSystem>)
     {
         return FunctionUtils::DemangleTypeName(typeid(callable));
     }
     else
     {
         return std::to_string(GetCallableID(callable));
     }
 }

…nces for better performance

…ngleTypeName for clarity

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@src/utils/function-container/src/Demangle.hpp`:
- Around line 3-8: Demangle.hpp conditionally includes <string> and <typeinfo>
(used by the function signature that accepts/returns std::string and
std::type_info) and misses <memory> (used for std::unique_ptr), which breaks
MSVC; fix by moving or adding unconditional includes for <string>, <typeinfo>,
and <memory> outside the `#if` defined(__GNUG__) || defined(__clang__) block so
that the function declaration/definition referencing std::string,
std::type_info, and std::unique_ptr compiles on all compilers (locate the
includes near the top of Demangle.hpp and update them accordingly).

small comment in code

Quality Gate passed

Issues
0 New issues
2 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

feat(logging): enhance system error logging with system names

f6d4f96

Miou-zora requested a review from a team January 30, 2026 11:00

Miou-zora self-assigned this Jan 30, 2026

Miou-zora added the enhancement New feature or request label Jan 30, 2026

Merge branch 'main' into make-system-logging-more-explicit

347e454

Miou-zora marked this pull request as ready for review January 30, 2026 11:00

feat(logging): add missing string include in WrappedSystem and Callab…

7fe4496

…leFunction

coderabbitai Bot reviewed Jan 30, 2026

View reviewed changes

Comment thread src/engine/src/system/WrappedSystem.hpp

feat(logging): add demangling functionality for type names in Wrapped…

0880421

…System and CallableFunction

coderabbitai Bot reviewed Jan 30, 2026

View reviewed changes

Comment thread src/engine/src/system/WrappedSystem.hpp Outdated

Comment thread src/utils/function-container/src/Demangle.hpp

Comment thread src/utils/function-container/src/Demangle.hpp Outdated

Miou-zora added 6 commits January 30, 2026 16:52

refactor(logging): make type parameters in WrappedSystem const refere…

304bb61

…nces for better performance

refactor(logging): encapsulate includes and memory management in Dema…

8932a51

…ngleTypeName for clarity

Merge branch 'main' into make-system-logging-more-explicit

3dc86e9

Merge branch 'main' into make-system-logging-more-explicit

445e4ea

Merge branch 'main' into make-system-logging-more-explicit

cd5edb8

refactor: simplify demangling logic in DemangleTypeName function

1d891a0

coderabbitai Bot reviewed Feb 1, 2026

View reviewed changes

Comment thread src/utils/function-container/src/Demangle.hpp

Miou-zora added 2 commits February 1, 2026 14:43

refactor: reorder include in Demangle.hpp file

c4e4743

refactor: improve memory management in DemangleTypeName function

2132551

ripel2 reviewed Feb 2, 2026

View reviewed changes

Comment thread src/utils/function-container/src/Demangle.hpp Outdated

ripel2 reviewed Feb 2, 2026

View reviewed changes

Miou-zora added 3 commits February 2, 2026 13:38

doc: remove french comment

f1e3770

Merge branch 'main' into make-system-logging-more-explicit

2c55405

Merge branch 'main' into make-system-logging-more-explicit

38e5f9a

Miou-zora merged commit 45f1c47 into main Feb 10, 2026
16 checks passed

Miou-zora deleted the make-system-logging-more-explicit branch February 10, 2026 16:58

coderabbitai Bot mentioned this pull request Apr 10, 2026

docs(engine): specify which inline file extension to use inl, tpp or ipp and when #580

Merged

7 tasks

feat(logging): enhance system error logging with system names by Miou-zora · Pull Request #454 · EngineSquared/EngineSquared · GitHub

Uh oh!

feat(logging): enhance system error logging with system names - #454

feat(logging): enhance system error logging with system names#454
Miou-zora merged 15 commits into
mainfrom
make-system-logging-more-explicit

Miou-zora commented Jan 30, 2026 •
edited by coderabbitai Bot
Loading

Uh oh!

Uh oh!

coderabbitai Bot commented Jan 30, 2026 •
edited
Loading

Uh oh!

Uh oh!

Miou-zora commented Jan 30, 2026

Uh oh!

coderabbitai Bot commented Jan 30, 2026

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

ripel2 left a comment

Uh oh!

sonarqubecloud Bot commented Feb 10, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

feat(logging): enhance system error logging with system names by Miou-zora · Pull Request #454 · EngineSquared/EngineSquared · GitHub

Uh oh!

feat(logging): enhance system error logging with system names - #454

feat(logging): enhance system error logging with system names#454Miou-zora merged 15 commits intomainEngineSquared/EngineSquared:mainfrom make-system-logging-more-explicitEngineSquared/EngineSquared:make-system-logging-more-explicitCopy head branch name to clipboard

Conversation

Miou-zora commented Jan 30, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jan 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

Miou-zora commented Jan 30, 2026

Uh oh!

coderabbitai Bot commented Jan 30, 2026

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ripel2 left a comment

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud Bot commented Feb 10, 2026

Quality Gate passed

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

feat(logging): enhance system error logging with system names#454
Miou-zora merged 15 commits into
mainfrom
make-system-logging-more-explicit

Miou-zora commented Jan 30, 2026 •
edited by coderabbitai Bot
Loading

coderabbitai Bot commented Jan 30, 2026 •
edited
Loading