From bbc7ea1db43fb6442cf6e9d3480a48b63ae14929 Mon Sep 17 00:00:00 2001 From: GauthierMalfilatre Date: Mon, 21 Sep 2026 11:10:00 +0200 Subject: [PATCH 1/2] [FIX] Fix little issues --- example/snake/src/Snake.cpp | 6 - include/kronkworld/component/Component.hpp | 5 +- include/kronkworld/kronkflow/Scheduler.hpp | 9 + .../kronkworld/ressource/RessourceManager.hpp | 17 +- include/kronkworld/system/ISystem.hpp | 2 +- include/kronkworld/system/System.hpp | 77 +++++-- include/kronkworld/world/World.hpp | 44 +++- src/world/World.cpp | 10 +- tests/unit/schedules.cpp | 199 ++++++++++++++++++ tests/unit/type_ids.cpp | 86 ++++++++ 10 files changed, 421 insertions(+), 34 deletions(-) create mode 100644 tests/unit/schedules.cpp create mode 100644 tests/unit/type_ids.cpp diff --git a/example/snake/src/Snake.cpp b/example/snake/src/Snake.cpp index 59d82e0..cb07c52 100644 --- a/example/snake/src/Snake.cpp +++ b/example/snake/src/Snake.cpp @@ -5,14 +5,8 @@ #include #include #include -#include -#include -#include #include -#include "../../../include/kronkworld/Kronkworld.hpp" #include -#include "SFML/Graphics.hpp" -#include #include "systems/StartupSystems.hpp" #include "systems/UpdateSystems.hpp" #include "systems/RenderSystems.hpp" diff --git a/include/kronkworld/component/Component.hpp b/include/kronkworld/component/Component.hpp index de25a3c..291246d 100644 --- a/include/kronkworld/component/Component.hpp +++ b/include/kronkworld/component/Component.hpp @@ -10,6 +10,7 @@ #include "ComponentError.hpp" #include "kronkworld/entity/Entity.hpp" #include + #include #include #include #include @@ -106,12 +107,12 @@ namespace kw template Component id(void) const { - static Component id = m_id++; + static const Component id = m_id.fetch_add(1); return id; } private: - inline static Component m_id; + inline static std::atomic m_id{0}; std::array, MAX_COMPONENTS> m_componentBoxs; }; diff --git a/include/kronkworld/kronkflow/Scheduler.hpp b/include/kronkworld/kronkflow/Scheduler.hpp index acae6a5..a08e100 100644 --- a/include/kronkworld/kronkflow/Scheduler.hpp +++ b/include/kronkworld/kronkflow/Scheduler.hpp @@ -28,6 +28,9 @@ namespace kw } } + Scheduler(Scheduler& other) = delete; + Scheduler(Scheduler&& other) = delete; + ~Scheduler() { kfScheduler_destroy(sch); @@ -52,6 +55,12 @@ namespace kw return kfScheduler_addTask(sch, opt, delay, interval); } + // true if the task was found (its clearer has run or will run) + bool remove(kfTaskID id) + { + return kfScheduler_removeTask(sch, id) != 0; + } + private: kfScheduler* sch; }; diff --git a/include/kronkworld/ressource/RessourceManager.hpp b/include/kronkworld/ressource/RessourceManager.hpp index 36ddd27..edf4380 100644 --- a/include/kronkworld/ressource/RessourceManager.hpp +++ b/include/kronkworld/ressource/RessourceManager.hpp @@ -9,6 +9,7 @@ #include "../entity/Entity.hpp" #include "../component/Component.hpp" #include + #include #include #include #include @@ -20,13 +21,24 @@ namespace kw { + namespace detail + { + // One counter for the whole process: a resource type has the same id + // in every World, and two threads can register types at the same time. + inline std::atomic& resourceIdCounter(void) noexcept + { + static std::atomic counter{0}; + return counter; + } + } + class ResourceManager { public: template - size_t id(void) noexcept + static size_t id(void) noexcept { - static size_t id = m_id++; + static const size_t id = detail::resourceIdCounter().fetch_add(1); return id; } @@ -71,7 +83,6 @@ namespace kw private: std::array, MAX_RESOURCES> m_resources; - size_t m_id = 0; }; } diff --git a/include/kronkworld/system/ISystem.hpp b/include/kronkworld/system/ISystem.hpp index 02d676d..b3cf75c 100644 --- a/include/kronkworld/system/ISystem.hpp +++ b/include/kronkworld/system/ISystem.hpp @@ -24,7 +24,7 @@ namespace kw bool isDone(void) const { return m_isDone; } private: - bool m_isDone; + bool m_isDone = false; }; } diff --git a/include/kronkworld/system/System.hpp b/include/kronkworld/system/System.hpp index 565e660..3346842 100644 --- a/include/kronkworld/system/System.hpp +++ b/include/kronkworld/system/System.hpp @@ -9,6 +9,7 @@ #include #include #include + #include #include #include #include "ISystem.hpp" @@ -22,6 +23,23 @@ namespace kw typedef uint32_t StageId; + // A World owns two schedulers, each one with its own tick counter (so + // "delay" and "interval" count ticks of the schedule the system is in): + // - Fixed: run as many times as needed to catch up with a fixed timestep + // - Frame: run once per rendered frame + // kw doesn't decide when they run: the caller does, with runOnce(schedule). + enum class Schedule : uint8_t { Fixed, Frame }; + + // Identifies a system that was added, to remove it later + struct SystemHandle { + + Schedule schedule = Schedule::Fixed; + kfTaskID id = 0; // 0: no system + + explicit operator bool() const { return id != 0; } + + }; + // static constexpr StageId Startup = 0; // typedef size_t RunPolicy; @@ -40,7 +58,7 @@ namespace kw class SystemManager { public: - SystemManager() : m_scheduler(128) {} + SystemManager() : m_fixed(128), m_frame(128) {} // void addUpdate(std::unique_ptr system) // { @@ -52,17 +70,20 @@ namespace kw // m_renderSystems.push_back(std::move(system)); // } - void addSystem( + // Systems of a same stage, due on the same tick, run in the order + // they were added. A system that returns false is not run again. + SystemHandle addSystem( + Schedule schedule, StageId stage, std::unique_ptr system, size_t delay = 1, - size_t interval = 0, + size_t interval = 1, const RWMask& mask = RWMask(0, 0) ) { ISystem* rawSystem = system.release(); - m_scheduler.pushTask((kfTaskOpt){ + auto id = scheduler(schedule).pushTask((kfTaskOpt){ [](void *ctx, void *arg) -> int { auto task = static_cast(arg); auto ret = task->handle(*static_cast(ctx)); @@ -74,29 +95,47 @@ namespace kw stage, (kfRWMasks){mask.read_mask, mask.write_mask}}, delay, interval); + if (id == 0) { + delete rawSystem; + throw std::bad_alloc(); + } + return SystemHandle{schedule, id}; } - void runOnce(World& world) + // Same as above, in the Fixed schedule + SystemHandle addSystem( + StageId stage, + std::unique_ptr system, + size_t delay = 1, + size_t interval = 1, + const RWMask& mask = RWMask(0, 0) + ) { - // for (auto& ls : m_logicSystems) { - // ls->handle(world); - // } - // for (auto& rs : m_renderSystems) { - // rs->handle(world); - // } - m_scheduler.tick(static_cast(&world)); - // std::erase_if(m_systems, [](const auto& system) { - // return system->isDone(); - // }); + return addSystem(Schedule::Fixed, stage, std::move(system), delay, interval, mask); + } + + // Can be called from a system, including on itself (it then finishes + // its current run and is not run again). + // Returns false if it already ended, or if the handle is empty. + bool removeSystem(const SystemHandle& handle) + { + return handle && scheduler(handle.schedule).remove(handle.id); + } + + void runOnce(World& world, Schedule schedule) + { + scheduler(schedule).tick(static_cast(&world)); } private: - // std::vector> m_logicSystems; - // std::vector> m_renderSystems; + Scheduler& scheduler(Schedule schedule) + { + return schedule == Schedule::Fixed ? m_fixed : m_frame; + } - // std::vector> m_systems; - Scheduler m_scheduler; + Scheduler m_fixed; + Scheduler m_frame; }; } diff --git a/include/kronkworld/world/World.hpp b/include/kronkworld/world/World.hpp index fa0957d..2921be9 100644 --- a/include/kronkworld/world/World.hpp +++ b/include/kronkworld/world/World.hpp @@ -23,7 +23,10 @@ namespace kw { public: void show(Entity entity) const; + // Runs one tick of both schedules: Fixed, then Frame void runOnce(void); + // Runs one tick of a single schedule + void runOnce(Schedule schedule); void run(void); void stop(void); @@ -85,6 +88,7 @@ namespace kw // return *this; // } + // In the Fixed schedule World& addSystem( size_t priority, std::unique_ptr system, @@ -92,9 +96,22 @@ namespace kw size_t interval = 1, const RWMask& mask = RWMask(0, 0) ) + { + return addSystem(Schedule::Fixed, priority, std::move(system), delay, interval, mask); + } + + World& addSystem( + Schedule schedule, + size_t priority, + std::unique_ptr system, + size_t delay = 1, + size_t interval = 1, + const RWMask& mask = RWMask(0, 0) + ) { m_systemManager.addSystem( - priority, + schedule, + static_cast(priority), std::move(system), delay, interval, @@ -103,6 +120,31 @@ namespace kw return *this; } + // Same, but gives back a handle to remove the system later + SystemHandle scheduleSystem( + Schedule schedule, + size_t priority, + std::unique_ptr system, + size_t delay = 1, + size_t interval = 1, + const RWMask& mask = RWMask(0, 0) + ) + { + return m_systemManager.addSystem( + schedule, + static_cast(priority), + std::move(system), + delay, + interval, + mask + ); + } + + bool removeSystem(const SystemHandle& handle) + { + return m_systemManager.removeSystem(handle); + } + /////////////////////////////////////////////////////////////////////// template R& addResource(Args&&... args) diff --git a/src/world/World.cpp b/src/world/World.cpp index f963196..3d9dfea 100644 --- a/src/world/World.cpp +++ b/src/world/World.cpp @@ -7,13 +7,19 @@ void kw::World::show(Entity entity) const void kw::World::runOnce(void) { - m_systemManager.runOnce(*this); + runOnce(Schedule::Fixed); + runOnce(Schedule::Frame); +} + +void kw::World::runOnce(Schedule schedule) +{ + m_systemManager.runOnce(*this, schedule); } void kw::World::run(void) { while (m_running) { - m_systemManager.runOnce(*this); + runOnce(); } } diff --git a/tests/unit/schedules.cpp b/tests/unit/schedules.cpp new file mode 100644 index 0000000..2682882 --- /dev/null +++ b/tests/unit/schedules.cpp @@ -0,0 +1,199 @@ +extern "C" { + #include "kronklab/kronklab.h" +} +#include "../../include/kronkworld/Kronkworld.hpp" +#include +#include +#include +#include + +namespace { + + // Counts its runs, and how many instances were destroyed (= cleared) + class Counter : public kw::ISystem + { + public: + explicit Counter(size_t* runs, size_t* destroyed = nullptr) + : m_runs(runs), m_destroyed(destroyed) {} + + ~Counter() override + { + if (m_destroyed) { + ++*m_destroyed; + } + } + + bool handle(kw::World&) override + { + ++*m_runs; + return true; + } + + private: + size_t* m_runs; + size_t* m_destroyed; + }; + + class Recorder : public kw::ISystem + { + public: + Recorder(std::vector* log, int marker) : m_log(log), m_marker(marker) {} + + bool handle(kw::World&) override + { + m_log->push_back(m_marker); + return true; + } + + private: + std::vector* m_log; + int m_marker; + }; + + // Removes itself the first time it runs + class SelfRemover : public kw::ISystem + { + public: + SelfRemover(size_t* runs, kw::SystemHandle* handle) : m_runs(runs), m_handle(handle) {} + + bool handle(kw::World& world) override + { + ++*m_runs; + world.removeSystem(*m_handle); + return true; + } + + private: + size_t* m_runs; + kw::SystemHandle* m_handle; + }; + +} + +Test(schedules, fixed_and_frame_are_independent) +{ + kw::World world; + size_t fixed = 0, frame = 0; + + world.addSystem(kw::Schedule::Fixed, 0, std::make_unique(&fixed)); + world.addSystem(kw::Schedule::Frame, 0, std::make_unique(&frame)); + for (int i = 0; i < 3; ++i) { + world.runOnce(kw::Schedule::Fixed); + } + AssertEq(fixed, 3, "3 fixed ticks, got %zu", fixed); + AssertEq(frame, 0, "no frame tick yet, got %zu", frame); + for (int i = 0; i < 2; ++i) { + world.runOnce(kw::Schedule::Frame); + } + AssertEq(fixed, 3, "frame ticks don't run fixed systems, got %zu", fixed); + AssertEq(frame, 2, "2 frame ticks, got %zu", frame); + world.runOnce(); + AssertEq(fixed, 4, "runOnce() ticks both, fixed is %zu", fixed); + AssertEq(frame, 3, "runOnce() ticks both, frame is %zu", frame); +} + +Test(schedules, legacy_add_system_goes_in_fixed) +{ + kw::World world; + size_t runs = 0; + + world.addSystem(0, std::make_unique(&runs)); + for (int i = 0; i < 5; ++i) { + world.runOnce(kw::Schedule::Frame); + } + AssertEq(runs, 0, "not in the frame schedule, got %zu", runs); + world.runOnce(kw::Schedule::Fixed); + AssertEq(runs, 1, "in the fixed schedule, got %zu", runs); +} + +Test(schedules, manager_default_periodic) +{ + kw::World world; + kw::SystemManager manager; + size_t runs = 0; + + manager.addSystem(0, std::make_unique(&runs)); + for (int i = 0; i < 4; ++i) { + manager.runOnce(world, kw::Schedule::Fixed); + } + AssertEq(runs, 4, "same default as World (periodic), got %zu", runs); +} + +Test(schedules, one_shot_system_runs_once) +{ + kw::World world; + size_t runs = 0, destroyed = 0; + + world.addSystem(0, std::make_unique(&runs, &destroyed), 0, 0); + for (int i = 0; i < 4; ++i) { + world.runOnce(); + } + AssertEq(runs, 1, "interval 0 runs once, got %zu", runs); + AssertEq(destroyed, 1, "and is destroyed, got %zu", destroyed); +} + +Test(schedules, remove_system) +{ + kw::World world; + size_t runs = 0, destroyed = 0; + auto handle = world.scheduleSystem(kw::Schedule::Fixed, 0, std::make_unique(&runs, &destroyed)); + + AssertEq(static_cast(handle), true, "a valid handle"); + world.runOnce(); + world.runOnce(); + AssertEq(runs, 2, "ran twice, got %zu", runs); + AssertEq(world.removeSystem(handle), true, "removed"); + AssertEq(destroyed, 1, "system destroyed on removal, got %zu", destroyed); + world.runOnce(); + world.runOnce(); + AssertEq(runs, 2, "no more runs, got %zu", runs); + AssertEq(world.removeSystem(handle), false, "already removed"); + AssertEq(world.removeSystem(kw::SystemHandle{}), false, "empty handle"); +} + +Test(schedules, remove_in_right_schedule) +{ + kw::World world; + size_t fixed = 0, frame = 0; + auto h1 = world.scheduleSystem(kw::Schedule::Fixed, 0, std::make_unique(&fixed)); + auto h2 = world.scheduleSystem(kw::Schedule::Frame, 0, std::make_unique(&frame)); + + // The two schedulers hand out ids on their own: a handle needs its schedule + AssertEq(h1.id, h2.id, "same first id in each scheduler"); + world.removeSystem(h2); + world.runOnce(); + AssertEq(fixed, 1, "fixed system untouched, got %zu", fixed); + AssertEq(frame, 0, "frame system removed, got %zu", frame); +} + +Test(schedules, system_removes_itself) +{ + kw::World world; + size_t runs = 0; + kw::SystemHandle handle; + + handle = world.scheduleSystem(kw::Schedule::Fixed, 0, std::make_unique(&runs, &handle)); + for (int i = 0; i < 5; ++i) { + world.runOnce(); + } + AssertEq(runs, 1, "ran once then gone, got %zu", runs); +} + +Test(schedules, registration_order) +{ + kw::World world; + std::vector log; + + for (int m = 1; m <= 6; ++m) { + world.addSystem(kw::Schedule::Fixed, 2, std::make_unique(&log, m)); + } + world.addSystem(kw::Schedule::Fixed, 1, std::make_unique(&log, 0)); + for (int t = 0; t < 3; ++t) { + world.runOnce(kw::Schedule::Fixed); + } + AssertEq(log.size(), 21, "7 systems x 3 ticks, got %zu", log.size()); + for (size_t i = 0; i < log.size(); ++i) { + int expected = (i % 7 == 0) ? 0 : static_cast(i % 7); + AssertEq(log[i], expected, "at %zu expected %d got %d", i, expected, log[i]); + } +} diff --git a/tests/unit/type_ids.cpp b/tests/unit/type_ids.cpp new file mode 100644 index 0000000..df26827 --- /dev/null +++ b/tests/unit/type_ids.cpp @@ -0,0 +1,86 @@ +extern "C" { + #include "kronklab/kronklab.h" +} +#include "../../include/kronkworld/Kronkworld.hpp" +#include +#include +#include +#include +#include +#include + +namespace { + + struct WorldA { int v; }; + struct WorldB { int v; }; + + template struct Tag { int v; }; + + // Calls f.template operator()() for N = 0..Count-1 (or the reverse) + template + void forEachTag(F&& f, std::integer_sequence, bool reverse) + { + constexpr int last = static_cast(sizeof...(Ns)) - 1; + + if (!reverse) { + (f.template operator()(), ...); + } else { + (f.template operator()(), ...); + } + } + + constexpr int TAGS = 64; + + // Every thread registers the same TAGS types, half of them in reverse order, + // then all of them must agree on the id of each type, and ids are distinct. + template + void checkIdsAcrossThreads(IdOf idOf) + { + constexpr int THREADS = 8; + std::vector> ids(THREADS); + std::vector threads; + + for (int t = 0; t < THREADS; ++t) { + threads.emplace_back([&, t] { + forEachTag([&]() { ids[t][N] = idOf.template operator()(); }, + std::make_integer_sequence{}, t % 2 == 1); + }); + } + for (auto& th : threads) { + th.join(); + } + std::set distinct(ids[0].begin(), ids[0].end()); + AssertEq(distinct.size(), TAGS, "ids must be distinct, got %zu distinct", distinct.size()); + for (int t = 1; t < THREADS; ++t) { + AssertEq(ids[t] == ids[0], true, "thread %d disagrees on the ids", t); + } + } + +} + +// Regression: the id used to come from a counter of the first manager that +// saw the type, so a second World could give two types the same slot. +Test(type_ids, resources_independent_of_world) +{ + kw::World w1; + kw::World w2; + + w1.addResource(1); + w2.addResource(20); + w2.addResource(10); + AssertEq(w2.getResource().v, 20, "B untouched by A in w2, got %d", w2.getResource().v); + AssertEq(w2.getResource().v, 10, "A in w2, got %d", w2.getResource().v); + AssertEq(w1.getResource().v, 1, "A in w1, got %d", w1.getResource().v); +} + +Test(type_ids, resource_ids_thread_safe) +{ + checkIdsAcrossThreads([]() { return kw::ResourceManager::id>(); }); +} + +Test(type_ids, component_ids_thread_safe) +{ + kw::ComponentManager components; + + checkIdsAcrossThreads([&]() { return static_cast(components.id>()); }); +} From 20fbb47ffe5dd52cc957763feb4f4e73eb24ec92 Mon Sep 17 00:00:00 2001 From: GauthierMalfilatre Date: Mon, 21 Sep 2026 11:11:47 +0200 Subject: [PATCH 2/2] [ADD] Add wotkflows --- .github/workflows/build.yml | 32 +++++++++++++++++++++ .github/workflows/tests.yml | 56 +++++++++++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+) create mode 100644 .github/workflows/build.yml create mode 100644 .github/workflows/tests.yml diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml new file mode 100644 index 0000000..2c08e2a --- /dev/null +++ b/.github/workflows/build.yml @@ -0,0 +1,32 @@ +name: Build + +on: + push: + branches: [main, dev] + pull_request: + branches: [main, dev] + +jobs: + build: + name: Build (${{ matrix.cxx }}) + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + include: + - cc: gcc + cxx: g++ + - cc: clang + cxx: clang++ + steps: + - uses: actions/checkout@v4 + + - name: Configure + run: > + cmake -S . -B build + -DCMAKE_BUILD_TYPE=Release + -DCMAKE_C_COMPILER=${{ matrix.cc }} + -DCMAKE_CXX_COMPILER=${{ matrix.cxx }} + + - name: Build + run: cmake --build build --parallel diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml new file mode 100644 index 0000000..f1aec7b --- /dev/null +++ b/.github/workflows/tests.yml @@ -0,0 +1,56 @@ +name: Unit tests + +on: + push: + branches: [main, dev] + pull_request: + branches: [main, dev] + +jobs: + unit-tests: + name: Unit tests (${{ matrix.cxx }}) + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + include: + - cc: gcc + cxx: g++ + - cc: clang + cxx: clang++ + steps: + - uses: actions/checkout@v4 + + - name: Configure + run: > + cmake -S . -B build + -DBUILD_TESTS=ON + -DCMAKE_BUILD_TYPE=Release + -DCMAKE_C_COMPILER=${{ matrix.cc }} + -DCMAKE_CXX_COMPILER=${{ matrix.cxx }} + + - name: Build + run: cmake --build build --parallel --target kronkworld_tests + + - name: Run unit tests + # kronklab's runner always exits with 0, even when tests fail or + # crash, so the exit code can't be trusted: parse the final report. + run: | + set -o pipefail + ./build/kronkworld_tests | tee test-output.txt + + report=$(sed 's/\x1b\[[0-9;]*m//g' test-output.txt | grep '^\[REPORT\] total') + echo "$report" + + total=$(sed -E 's/.*total \[([0-9]+)\].*/\1/' <<< "$report") + failed=$(sed -E 's/.*failed \[([0-9]+)\].*/\1/' <<< "$report") + crashed=$(sed -E 's/.*crashed \[([0-9]+)\].*/\1/' <<< "$report") + + if [ -z "$report" ] || [ "${total:-0}" -eq 0 ]; then + echo "::error::No test report found or no test was run" + exit 1 + fi + if [ "$failed" -ne 0 ] || [ "$crashed" -ne 0 ]; then + echo "::error::$failed test(s) failed, $crashed test(s) crashed" + exit 1 + fi