Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions src/core/include/scene/unit.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -220,9 +220,10 @@ namespace gkit::scene {
using reference = Unit&;

public:
// Constructor and operators
iterator(Unit* owner, size_t pos);
auto operator*() -> reference const;
auto operator->() -> pointer const;
auto operator*() const -> reference;
auto operator->() const -> pointer;
auto operator++() -> iterator&;
auto operator++(int) -> iterator;
auto operator--() -> iterator&;
Expand All @@ -236,10 +237,7 @@ namespace gkit::scene {
friend class Unit;
};

// Why this part didn't use auto, because it need to have a const_iterator use
// but auto could not allow two same function but with different return
// but begin() and end() need those two return
// So I will not change it

auto begin() -> iterator;
auto end() -> iterator;

Expand All @@ -253,9 +251,11 @@ namespace gkit::scene {
using pointer = const Unit*;
using reference = const Unit&;

public:
// Constructor and operators
const_iterator(const Unit* owner, size_t pos);
auto operator*() -> reference const;
auto operator->() -> pointer const;
auto operator*() const -> reference;
auto operator->() const -> pointer;
auto operator++() -> const_iterator&;
auto operator++(int) -> const_iterator;
auto operator--() -> const_iterator&;
Expand Down
16 changes: 10 additions & 6 deletions src/core/scene/unit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -188,11 +188,11 @@ auto gkit::scene::Unit::get_parent<gkit::scene::Unit>() noexcept -> std::optiona

// iterator part use
gkit::scene::Unit::iterator::iterator(Unit* owner, size_t pos) : m_owner(owner), m_pos(pos) {}
auto gkit::scene::Unit::iterator::operator*() -> reference const {
auto gkit::scene::Unit::iterator::operator*() const -> reference {
auto child_opt = m_owner->get_available_child(static_cast<uint32_t>(m_pos));
return **child_opt;
}
auto gkit::scene::Unit::iterator::operator->() -> pointer const {
auto gkit::scene::Unit::iterator::operator->() const -> pointer {
auto child_opt = m_owner->get_available_child(static_cast<uint32_t>(m_pos));
return *child_opt;
}
Expand Down Expand Up @@ -228,12 +228,12 @@ auto gkit::scene::Unit::end() -> iterator{
// now is const_iterator use
gkit::scene::Unit::const_iterator::const_iterator(const Unit* owner, size_t pos) : m_owner(owner), m_pos(pos) {}

auto gkit::scene::Unit::const_iterator::operator*() -> reference const {
auto gkit::scene::Unit::const_iterator::operator*() const -> reference {
auto child_opt = const_cast<Unit*>(m_owner)->get_available_child(static_cast<uint32_t>(m_pos));
return **child_opt;
}

auto gkit::scene::Unit::const_iterator::operator->() -> pointer const {
auto gkit::scene::Unit::const_iterator::operator->() const -> pointer {
auto child_opt = const_cast<Unit*>(m_owner)->get_available_child(static_cast<uint32_t>(m_pos));
return *child_opt;
}
Expand Down Expand Up @@ -274,8 +274,12 @@ auto gkit::scene::Unit::cbegin() const -> const_iterator { return begin(); }
auto gkit::scene::Unit::cend() const -> const_iterator { return end(); }

// This is a reverse iterator, implemented using std::reverse_iterator.
using reverse_iterator = std::reverse_iterator<gkit::scene::Unit::iterator>;
using const_reverse_iterator = std::reverse_iterator<gkit::scene::Unit::const_iterator>;
// using at here maybe have some problem, just I guess,
// So I deleted it

// using reverse_iterator = std::reverse_iterator<gkit::scene::Unit::iterator>;
// using const_reverse_iterator = std::reverse_iterator<gkit::scene::Unit::const_iterator>;

auto gkit::scene::Unit::rbegin() -> reverse_iterator { return reverse_iterator(end()); }
auto gkit::scene::Unit::rend() -> reverse_iterator { return reverse_iterator(begin()); }
auto gkit::scene::Unit::rbegin() const -> const_reverse_iterator { return const_reverse_iterator(end()); }
Expand Down
120 changes: 120 additions & 0 deletions test/core/scene/test_unit_iterator.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
#include "scene/unit.hpp"
#include <gkit.hpp>
#include <format>
#include <iostream>
#include <memory>
#include <string>
#include <vector>
#include <iterator>

using gkit::scene::Unit;

#define TEST(cond, msg) do { \
if (!(cond)) { \
std::cerr << "FAIL: " << msg << " (" << __FILE__ << ":" << __LINE__ << ")" << std::endl; \
return false; \
} else { \
std::cout << "PASS: " << msg << std::endl; \
} \
} while(0)


class TestUnit : public Unit {
public:
using Unit::ready_handler;
using Unit::process_handler;
using Unit::exit_handler;
using Unit::parent;

static auto create(std::string name = "") {
return Unit::create<TestUnit>(std::move(name));
}

int ready_calls = 0;
int process_calls = 0;
int physics_calls = 0;
int exit_calls = 0;
std::string name_storage;

// Custom constructor
TestUnit(const std::string& name) : Unit(name), name_storage(name) {}

void _ready() override {
ready_calls++;
std::cout << "TestUnit[" << name_storage << "] _ready()\n";
}
void _process() override {
process_calls++;
std::cout << "TestUnit[" << name_storage << "] _process()\n";
}
void _physics_process() override {
physics_calls++;
}
void _exit() override {
exit_calls++;
std::cout << "TestUnit[" << name_storage << "] _exit()\n";
}
};

// ==================== test ====================

bool test_iterator() {
std::cout << "\n=== test_iterator ===\n";
auto parent = TestUnit::create("parent");
std::vector<TestUnit*> children;
for (int i = 0; i < 5; ++i) {
auto child = TestUnit::create("child" + std::to_string(i));
children.push_back(child.get());
parent->add_child(std::move(child));
}
parent->process_handler();

int idx = 0;
for (auto& u : *parent) {
TEST(&u == children[idx], "iterator order matches addition");
idx++;
}
TEST(idx == 5, "iterated all 5 children");

idx = 4;
for (auto it = parent->rbegin(); it != parent->rend(); ++it) {
TEST(&(*it) == children[idx], "reverse iterator order");
idx--;
}
TEST(idx == -1, "reverse iterated all");

auto it = parent->begin();
++it;
TEST(&(*it) == children[1], "pre-increment");
auto old_it = it++;
TEST(&(*old_it) == children[1], "post-increment returns old value");
TEST(&(*it) == children[2], "post-increment advances iterator");
auto old_it2 = it--;
TEST(&(*old_it2) == children[2], "post-decrement returns old value");
TEST(&(*it) == children[1], "post-decrement moves iterator back");
--it;
TEST(&(*it) == children[0], "pre-decrement");

const auto& const_parent = *parent;
idx = 0;
for (auto& u : const_parent) {
TEST(&u == children[idx], "const iterator works");
idx++;
}
return true;
}
Comment on lines +61 to +105
Copy link

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test doesn't verify iterator behavior with an empty parent (zero children). Consider adding a test case that verifies begin() == end() when a Unit has no children, to ensure the iterator handles this edge case correctly.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So if you can help me, plz tell me what should I do?

Copy link
Copy Markdown
Contributor

@CoraBlack CoraBlack Feb 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So if you can help me, plz tell me what should I do?

It won't response to you, please read the requirement from copilot carefully


// ==================== main function ====================
int main() {
bool all_passed = true;
// Only run iterator test
all_passed &= test_iterator();

if (all_passed) {
std::cout << "\nAll tests passed!\n";
return 0;
} else {
std::cerr << "\nSome tests failed.\n";
return 1;
}
}