Conversation
|
last sonar issues are related to the tests |
src/engine/src/registry/Registry.hpp
Outdated
| /** | ||
| * Add multiple systems as a tuple to the registry. | ||
| * | ||
| * @tparam TScheduler The type of scheduler to use. | ||
| * @param systems Arg containing multiple systems to register. | ||
| */ | ||
| template <typename TScheduler = ES::Engine::Scheduler::Update, typename... Systems> | ||
| void RegisterSystem(Systems... systems); | ||
|
|
There was a problem hiding this comment.
Rather than creating a new method which enlarge current API, why didn't you update the current method ?
There was a problem hiding this comment.
It works and doesn't break tests, I will push it
There was a problem hiding this comment.
This test feels like pretty heavy to understand and I don't know why you added a new scheduler and run it two times. I think that you could just have 2 systems that set 2 variables to true and that's all.
There was a problem hiding this comment.
Even I am having trouble explaining the tests so I will add some comments for them
There was a problem hiding this comment.
I rewrote the entire tests according to your recommandations, it should be more explicit now.
I accept the issues related to global variables but passing std::vector must (in majority of cases) be made with reference avoiding unoptimised copy of data. |
|
Related to: