Actionable comments posted: 1

src/plugin/relationship/tests/Relationship.cpp (1)

204-221: ⚡ Quick win

Add nullopt-path assertions for TryGetParentComponent.

This test only validates the happy path. Please add cases for “child has no parent” and “parent exists but lacks TestComponent” to lock in the API contract.

✅ Suggested additions

 TEST(Relationship, try_get_parent_component)
 {
     Engine::Core core;
@@
     auto parentComponent = Relationship::Utils::TryGetParentComponent<TestComponent>(child);
 
     ASSERT_TRUE(parentComponent.has_value());
     ASSERT_EQ(parentComponent->value, 42);
+
+    auto orphan = core.CreateEntity();
+    orphan.AddComponent<Relationship::Component::Relationship>();
+    ASSERT_EQ(Relationship::Utils::TryGetParentComponent<TestComponent>(orphan), std::nullopt);
+
+    auto parentWithoutTest = core.CreateEntity();
+    auto childWithoutParentTest = core.CreateEntity();
+    parentWithoutTest.AddComponent<Relationship::Component::Relationship>();
+    childWithoutParentTest.AddComponent<Relationship::Component::Relationship>();
+    Relationship::Utils::SetChildOf(childWithoutParentTest, parentWithoutTest);
+    ASSERT_EQ(Relationship::Utils::TryGetParentComponent<TestComponent>(childWithoutParentTest), std::nullopt);
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/plugin/relationship/tests/Relationship.cpp` around lines 204 - 221, Add
two negative-path assertions to the Relationship.try_get_parent_component test:
(1) create a child entity without calling Relationship::Utils::SetChildOf and
assert
Relationship::Utils::TryGetParentComponent<TestComponent>(child).has_value() is
false; (2) create a child and parent, attach
Relationship::Component::Relationship to both and call
Relationship::Utils::SetChildOf(child, parent) but do not add TestComponent to
parent, then assert TryGetParentComponent<TestComponent>(child).has_value() is
false—use the same entities and helpers (SetChildOf, TryGetParentComponent,
TestComponent) used in the existing test to validate the nullopt paths.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/plugin/relationship/src/utils/Utils.hpp`:
- Around line 67-70: Forward-declare the ForEachChild function before the
TryGetChildComponents template so the template can find it during instantiation;
add a declaration matching the actual signature used (e.g., a function taking
Engine::Entity and a callable/lambda) above the TryGetChildComponents
definition, then keep the full ForEachChild definition later in the header so
TryGetChildComponents can call it without lookup/instantiation issues.

---

Nitpick comments:
In `@src/plugin/relationship/tests/Relationship.cpp`:
- Around line 204-221: Add two negative-path assertions to the
Relationship.try_get_parent_component test: (1) create a child entity without
calling Relationship::Utils::SetChildOf and assert
Relationship::Utils::TryGetParentComponent<TestComponent>(child).has_value() is
false; (2) create a child and parent, attach
Relationship::Component::Relationship to both and call
Relationship::Utils::SetChildOf(child, parent) but do not add TestComponent to
parent, then assert TryGetParentComponent<TestComponent>(child).has_value() is
false—use the same entities and helpers (SetChildOf, TryGetParentComponent,
TestComponent) used in the existing test to validate the nullopt paths.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6b41502f-ef21-4b37-9d36-ac60f070e72e

Reviewing files that changed from the base of the PR and between c7a6de6 and 0766e3b.

• src/plugin/relationship/examples/RelationshipUsage/src/main.cpp
• src/plugin/relationship/src/utils/Utils.hpp
• src/plugin/relationship/tests/Relationship.cpp