From 6df45a2e306de7b18502419305533a42960ba07a Mon Sep 17 00:00:00 2001 From: Canberk Pitirli Date: Sun, 4 Oct 2026 20:03:05 +0300 Subject: [PATCH 1/2] fix: let parents own their children and search the whole scene hierarchy --- include/public/Core/GameObject.hpp | 12 +- include/public/Core/Object.hpp | 13 +- include/public/Core/SceneBase.hpp | 17 +-- src/Assets/ModelLoader.cpp | 10 +- src/Scene/GameObject.cpp | 27 ++-- src/Scene/SceneBase.cpp | 190 +++++++++++++++++------------ 6 files changed, 158 insertions(+), 111 deletions(-) diff --git a/include/public/Core/GameObject.hpp b/include/public/Core/GameObject.hpp index 897adf3b..bc39fbf3 100644 --- a/include/public/Core/GameObject.hpp +++ b/include/public/Core/GameObject.hpp @@ -11,6 +11,7 @@ namespace Sleak { namespace Math {class Vector3D;}; + class SceneBase; /// Scene entity holding components and an optional parent/child transform /// hierarchy. Scene owns the instance; destroy via Scene::DestroyObject(). /// @@ -31,6 +32,10 @@ namespace Sleak { /// for deferred destruction, which is what you want while iterating, or /// SceneBase::RemoveObject() for immediate destruction. /// + /// A parent owns every child that was not itself added to a scene, so a + /// hierarchy built with SetParent() (for example by ModelLoader) only + /// needs its root added. Deleting a parent deletes those children too. + /// /// Camera, Light, and its subclasses all derive from GameObject, so /// they are added and found the same way as anything else. /// @@ -116,7 +121,7 @@ namespace Sleak { return GetComponent() != nullptr; } - /// Initializes the object and its components; repeat calls no-op. + /// Initializes the object, components and children; repeats no-op. virtual void Initialize(); bool IsInitialized() const { return bIsInitialized; } virtual void Update(float deltaTime); @@ -138,6 +143,8 @@ namespace Sleak { void RemoveChild(GameObject* child); bool HasParent() const { return m_parent != nullptr; } bool HasChildren() const { return m_children.GetSize() > 0; } + /// True once a scene took ownership through SceneBase::AddObject(). + bool IsOwnedByScene() const { return m_ownedByScene; } virtual bool IsLight() const { return false; } @@ -159,6 +166,7 @@ namespace Sleak { private: bool m_isActive; bool m_pendingDestroy; + bool m_ownedByScene = false; std::string m_tag = "Untagged"; List> Components; @@ -171,6 +179,8 @@ namespace Sleak { void DestroyComponents(); /// Initializes a component once, then enables it if active. void InitializeComponent(Component* component); + + friend class SceneBase; }; } diff --git a/include/public/Core/Object.hpp b/include/public/Core/Object.hpp index 80b860a7..122cfe8e 100644 --- a/include/public/Core/Object.hpp +++ b/include/public/Core/Object.hpp @@ -12,11 +12,9 @@ namespace Sleak { class Object { public: Object(const std::string& name = "Object") - : m_uniqueID(s_nextUniqueID.fetch_add(1, std::memory_order_relaxed)), - m_name(name) - { - SetName(m_name + "-" + std::to_string(m_uniqueID)); - } + : m_name(name), + m_uniqueID(s_nextUniqueID.fetch_add(1, std::memory_order_relaxed)) + {} virtual ~Object() = default; @@ -24,6 +22,11 @@ namespace Sleak { const std::string& GetName() const { return m_name; } + /// Name plus unique ID, for logs where names can repeat. + std::string GetDebugName() const { + return m_name + "-" + std::to_string(m_uniqueID); + } + inline void SetName(const std::string& value) { m_name = value; } private: diff --git a/include/public/Core/SceneBase.hpp b/include/public/Core/SceneBase.hpp index e139cbd9..1ca2606f 100644 --- a/include/public/Core/SceneBase.hpp +++ b/include/public/Core/SceneBase.hpp @@ -42,9 +42,10 @@ namespace Sleak { /// object (and its children) for deletion at the end of the frame, /// which is the safe choice while iterating. /// - /// Objects added to the scene are registered with the LightManager if - /// they report IsLight(), and their colliders are registered with the - /// PhysicsWorld, hierarchy included. + /// Children of an added object that were never added themselves are + /// owned by their parent and belong to the scene through it. Lights and + /// colliders anywhere in that hierarchy are registered with the + /// LightManager and PhysicsWorld, and the queries below search it too. /// /// @code{.cpp} /// // Destroy safely from inside an update @@ -124,19 +125,19 @@ namespace Sleak { // Object management — scene takes ownership of added objects /// Takes ownership of object, registering it with lighting and physics as needed. virtual void AddObject(GameObject* object); - /// Unregisters and deletes object immediately. - /// Cancels a queued DestroyObject() on the same object. + /// Unregisters and deletes object and its whole hierarchy immediately. + /// Cancels a queued DestroyObject() on anything it deletes. virtual void RemoveObject(GameObject* object); /// Queues an object for destruction; actually freed on the next ProcessPendingDestroy(). void DestroyObject(GameObject* object); const List& GetObjects() const { return Objects; } // Object queries - /// Linear search for the first object with a matching name. + /// First object with a matching name, children included. GameObject* FindObjectByName(const std::string& name); - /// Linear search for the object with a matching unique ID. + /// Object with a matching unique ID, children included. GameObject* FindObjectByID(uint64_t id); - /// Collects every object whose tag matches. + /// Collects every object whose tag matches, children included. List FindObjectsByTag(const std::string& tag); size_t GetObjectCount() const { return Objects.GetSize(); } diff --git a/src/Assets/ModelLoader.cpp b/src/Assets/ModelLoader.cpp index b3d1d4b6..3842a902 100644 --- a/src/Assets/ModelLoader.cpp +++ b/src/Assets/ModelLoader.cpp @@ -27,14 +27,6 @@ namespace Sleak { -/// Recursively initialize a GameObject and all its children. -static void InitializeRecursive(GameObject* obj) { - obj->Initialize(); - for (size_t i = 0; i < obj->GetChildren().GetSize(); ++i) { - InitializeRecursive(obj->GetChildren()[i]); - } -} - Math::Matrix4 ModelLoader::ConvertMatrix(const void* aiMatPtr) { const auto& m = *static_cast( static_cast(aiMatPtr)); @@ -107,7 +99,7 @@ GameObject* ModelLoader::Load(const std::string& filePath, options, textureCache, skeleton, clips); } - InitializeRecursive(root); + root->Initialize(); return root; } diff --git a/src/Scene/GameObject.cpp b/src/Scene/GameObject.cpp index 7f1e5752..227b4822 100644 --- a/src/Scene/GameObject.cpp +++ b/src/Scene/GameObject.cpp @@ -12,19 +12,20 @@ namespace Sleak { GameObject::~GameObject() { + List children = std::move(m_children); + for (size_t i = 0; i < children.GetSize(); ++i) { + GameObject* child = children[i]; + if (!child) continue; + child->m_parent = nullptr; + if (!child->m_ownedByScene) delete child; + } + DestroyComponents(); if (m_parent) { m_parent->RemoveChild(this); m_parent = nullptr; } - - for (size_t i = 0; i < m_children.GetSize(); ++i) { - if (m_children[i]) { - m_children[i]->m_parent = nullptr; - } - } - m_children.clear(); } void GameObject::Initialize() { @@ -34,6 +35,10 @@ namespace Sleak { for (size_t i = 0; i < Components.GetSize(); ++i) { if (Components[i]) InitializeComponent(Components[i].get()); } + + for (size_t i = 0; i < m_children.GetSize(); ++i) { + if (m_children[i]) m_children[i]->Initialize(); + } } void GameObject::InitializeComponent(Component* component) { @@ -106,6 +111,14 @@ namespace Sleak { void GameObject::SetParent(GameObject* parent) { if (m_parent == parent) return; + for (GameObject* p = parent; p; p = p->m_parent) { + if (p == this) { + SLEAK_WARN("Cannot parent '{}' to its own descendant", + GetName()); + return; + } + } + if (m_parent) { m_parent->RemoveChild(this); } diff --git a/src/Scene/SceneBase.cpp b/src/Scene/SceneBase.cpp index 2d01f830..1347af3d 100644 --- a/src/Scene/SceneBase.cpp +++ b/src/Scene/SceneBase.cpp @@ -13,28 +13,58 @@ namespace Sleak { -/// Walks obj and its children, registering any ColliderComponent found with world. -static void RegisterCollidersRecursive(GameObject* obj, Physics::PhysicsWorld* world) { - if (!obj || !world) return; - auto* collider = obj->GetComponent(); - if (collider) { - world->RegisterCollider(collider); - } - for (size_t i = 0; i < obj->GetChildren().GetSize(); ++i) { - RegisterCollidersRecursive(obj->GetChildren()[i], world); +/// Calls fn on obj and the descendants it owns, skipping scene owned ones. +template +static void ForEachOwned(GameObject* obj, Fn& fn) { + if (!obj) return; + fn(obj); + const auto& children = obj->GetChildren(); + for (size_t i = 0; i < children.GetSize(); ++i) { + if (children[i] && !children[i]->IsOwnedByScene()) + ForEachOwned(children[i], fn); } } -/// Walks obj and its children, unregistering any ColliderComponent found from world. -static void UnregisterCollidersRecursive(GameObject* obj, Physics::PhysicsWorld* world) { - if (!obj || !world) return; - auto* collider = obj->GetComponent(); - if (collider) { - world->UnregisterCollider(collider); - } - for (size_t i = 0; i < obj->GetChildren().GetSize(); ++i) { - UnregisterCollidersRecursive(obj->GetChildren()[i], world); +/// Depth-first search of obj and the descendants it owns. +template +static GameObject* FindOwned(GameObject* obj, const Pred& pred) { + if (!obj) return nullptr; + if (pred(obj)) return obj; + const auto& children = obj->GetChildren(); + for (size_t i = 0; i < children.GetSize(); ++i) { + if (!children[i] || children[i]->IsOwnedByScene()) continue; + if (GameObject* found = FindOwned(children[i], pred)) return found; } + return nullptr; +} + +/// Registers lights and colliders in obj's owned subtree, skipping null +/// services. +static void RegisterTree(GameObject* obj, LightManager* lights, + Physics::PhysicsWorld* world) { + auto reg = [&](GameObject* o) { + if (lights && o->IsLight()) + lights->RegisterLight(static_cast(o)); + if (world) { + if (auto* collider = o->GetComponent()) + world->RegisterCollider(collider); + } + }; + ForEachOwned(obj, reg); +} + +/// Unregisters lights and colliders in obj's owned subtree. +static void UnregisterTree(GameObject* obj, LightManager* lights, + Physics::PhysicsWorld* world) { + auto unreg = [&](GameObject* o) { + if (lights && o->IsLight()) + lights->UnregisterLight(static_cast(o)); + if (world) { + if (auto* collider = o->GetComponent()) + world->UnregisterCollider(collider); + } + }; + ForEachOwned(obj, unreg); } SceneBase::~SceneBase() { @@ -118,19 +148,20 @@ void SceneBase::Resume() { bool SceneBase::Initialize() { if (bInitialized) return true; + LightManager* newLights = nullptr; if (!m_lightManager) { - m_lightManager = new LightManager(); + m_lightManager = newLights = new LightManager(); m_lightManager->Initialize(); } + Physics::PhysicsWorld* newWorld = nullptr; if (!m_physicsWorld) { - m_physicsWorld = new Physics::PhysicsWorld(); + m_physicsWorld = newWorld = new Physics::PhysicsWorld(); + } - // Register colliders from objects added before PhysicsWorld existed + if (newLights || newWorld) { for (size_t i = 0; i < Objects.GetSize(); ++i) { - if (Objects[i]) { - RegisterCollidersRecursive(Objects[i], m_physicsWorld); - } + RegisterTree(Objects[i], newLights, newWorld); } } @@ -201,22 +232,24 @@ void SceneBase::Update(float deltaTime) { } }; - for (size_t i = 0; i < Objects.GetSize(); ++i) { - if (!Objects[i]) continue; - auto* collider = Objects[i]->GetComponent(); - if (!collider) continue; + auto drawObject = [&](GameObject* obj) { + auto* collider = obj->GetComponent(); + if (!collider) return; Math::Vector3D pos(0, 0, 0), scale(1, 1, 1); - auto* transform = Objects[i]->GetComponent(); + auto* transform = obj->GetComponent(); if (transform) { pos = transform->GetWorldPosition() + collider->GetOffset(); scale = transform->GetWorldScale(); - } else if (auto* cam = dynamic_cast(Objects[i])) { + } else if (auto* cam = dynamic_cast(obj)) { pos = cam->GetPosition() + collider->GetOffset(); } drawColliderShape(collider, pos, scale); - } + }; + for (size_t i = 0; i < Objects.GetSize(); ++i) { + ForEachOwned(Objects[i], drawObject); + } } DebugLineRenderer::Flush(m_activeCamera); @@ -249,15 +282,9 @@ void SceneBase::AddObject(GameObject* object) { if (Objects.indexOf(object) != -1) return; // already in scene Objects.add(object); + object->m_ownedByScene = true; - if (m_lightManager && object->IsLight()) { - m_lightManager->RegisterLight( - static_cast(object)); - } - - if (m_physicsWorld) { - RegisterCollidersRecursive(object, m_physicsWorld); - } + RegisterTree(object, m_lightManager, m_physicsWorld); if (bInitialized && !object->IsInitialized()) { object->Initialize(); @@ -266,22 +293,23 @@ void SceneBase::AddObject(GameObject* object) { void SceneBase::RemoveObject(GameObject* object) { if (!object) return; - int index = Objects.indexOf(object); - if (index == -1) return; + if (Objects.indexOf(object) == -1) return; - if (m_lightManager && object->IsLight()) { - m_lightManager->UnregisterLight(static_cast(object)); - } - if (m_physicsWorld) { - UnregisterCollidersRecursive(object, m_physicsWorld); + List children = object->GetChildren(); + for (size_t i = 0; i < children.GetSize(); ++i) { + if (children[i] && children[i]->IsOwnedByScene()) + RemoveObject(children[i]); } - int pending = m_pendingDestroy.indexOf(object); - if (pending != -1) { - m_pendingDestroy.erase(pending); - } + UnregisterTree(object, m_lightManager, m_physicsWorld); + + auto dropPending = [&](GameObject* o) { + int pending = m_pendingDestroy.indexOf(o); + if (pending != -1) m_pendingDestroy.erase(pending); + }; + ForEachOwned(object, dropPending); - Objects.erase(index); + Objects.erase(Objects.indexOf(object)); delete object; } @@ -301,29 +329,28 @@ void SceneBase::DestroyObject(GameObject* object) { } GameObject* SceneBase::FindObjectByName(const std::string& objectName) { + auto match = [&](GameObject* o) { return o->GetName() == objectName; }; for (size_t i = 0; i < Objects.GetSize(); ++i) { - if (Objects[i] && Objects[i]->GetName() == objectName) { - return Objects[i]; - } + if (GameObject* found = FindOwned(Objects[i], match)) return found; } return nullptr; } GameObject* SceneBase::FindObjectByID(uint64_t id) { + auto match = [&](GameObject* o) { return o->GetUniqueID() == id; }; for (size_t i = 0; i < Objects.GetSize(); ++i) { - if (Objects[i] && Objects[i]->GetUniqueID() == id) { - return Objects[i]; - } + if (GameObject* found = FindOwned(Objects[i], match)) return found; } return nullptr; } List SceneBase::FindObjectsByTag(const std::string& tag) { List result; + auto collect = [&](GameObject* o) { + if (o->GetTag() == tag) result.add(o); + }; for (size_t i = 0; i < Objects.GetSize(); ++i) { - if (Objects[i] && Objects[i]->GetTag() == tag) { - result.add(Objects[i]); - } + ForEachOwned(Objects[i], collect); } return result; } @@ -331,37 +358,38 @@ List SceneBase::FindObjectsByTag(const std::string& tag) { void SceneBase::ProcessPendingDestroy() { if (m_pendingDestroy.empty()) return; - for (size_t i = 0; i < m_pendingDestroy.GetSize(); ++i) { - GameObject* obj = m_pendingDestroy[i]; - if (!obj) continue; - - if (m_lightManager && obj->IsLight()) { - m_lightManager->UnregisterLight( - static_cast(obj)); - } - - if (m_physicsWorld) { - UnregisterCollidersRecursive(obj, m_physicsWorld); - } + List pending = std::move(m_pendingDestroy); + for (size_t i = 0; i < pending.GetSize(); ++i) { + GameObject* obj = pending[i]; + UnregisterTree(obj, m_lightManager, m_physicsWorld); int index = Objects.indexOf(obj); - if (index != -1) { - Objects.erase(index); - } + if (index != -1) Objects.erase(index); + } - delete obj; + // Detach first so no pending object is deleted through its parent. + for (size_t i = 0; i < pending.GetSize(); ++i) { + if (pending[i]) pending[i]->SetParent(nullptr); + } + for (size_t i = 0; i < pending.GetSize(); ++i) { + delete pending[i]; } - m_pendingDestroy.clear(); } void SceneBase::DestroyAllObjects() { - // Clear pending list first (those are also in Objects) m_pendingDestroy.clear(); - for (size_t i = 0; i < Objects.GetSize(); ++i) { - delete Objects[i]; + List objects = std::move(Objects); + + for (size_t i = 0; i < objects.GetSize(); ++i) { + UnregisterTree(objects[i], m_lightManager, m_physicsWorld); + } + for (size_t i = 0; i < objects.GetSize(); ++i) { + if (objects[i]) objects[i]->SetParent(nullptr); + } + for (size_t i = 0; i < objects.GetSize(); ++i) { + delete objects[i]; } - Objects.clear(); } void SceneBase::SetSkybox(Skybox* skybox) { From 3cc5b12f7e2cc92462808c5c48e0d6d2e0160a21 Mon Sep 17 00:00:00 2001 From: Canberk Pitirli Date: Thu, 8 Oct 2026 11:24:32 +0300 Subject: [PATCH 2/2] style: apply clang-format to changed lines --- include/public/Core/Object.hpp | 19 +++++++++---------- include/public/Core/SceneBase.hpp | 3 ++- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/include/public/Core/Object.hpp b/include/public/Core/Object.hpp index 122cfe8e..954c6750 100644 --- a/include/public/Core/Object.hpp +++ b/include/public/Core/Object.hpp @@ -11,21 +11,20 @@ namespace Sleak { /// @ingroup core class Object { public: - Object(const std::string& name = "Object") + Object(const std::string& name = "Object") : m_name(name), - m_uniqueID(s_nextUniqueID.fetch_add(1, std::memory_order_relaxed)) - {} + m_uniqueID(s_nextUniqueID.fetch_add(1, std::memory_order_relaxed)) {} - virtual ~Object() = default; + virtual ~Object() = default; - uint64_t GetUniqueID() const { return m_uniqueID; } + uint64_t GetUniqueID() const { return m_uniqueID; } - const std::string& GetName() const { return m_name; } + const std::string& GetName() const { return m_name; } - /// Name plus unique ID, for logs where names can repeat. - std::string GetDebugName() const { - return m_name + "-" + std::to_string(m_uniqueID); - } + /// Name plus unique ID, for logs where names can repeat. + std::string GetDebugName() const { + return m_name + "-" + std::to_string(m_uniqueID); + } inline void SetName(const std::string& value) { m_name = value; } diff --git a/include/public/Core/SceneBase.hpp b/include/public/Core/SceneBase.hpp index 1ca2606f..ca4e2371 100644 --- a/include/public/Core/SceneBase.hpp +++ b/include/public/Core/SceneBase.hpp @@ -29,7 +29,8 @@ namespace Sleak { namespace Physics { class PhysicsWorld; } /// Non-templated scene interface: object ownership, state machine, and - /// per-frame update hooks. Game scenes derive from Scene, not this directly. + /// per-frame update hooks. Game scenes derive from Scene, not this + /// directly. /// /// SceneBase defines what every scene can do regardless of subclass: /// hold objects, move through the SceneState machine, and answer