From 8ec65267ea2871f7abe5885e7c847b7ce70573e5 Mon Sep 17 00:00:00 2001 From: AzaezelX Date: Fri, 30 Jan 2026 00:52:33 -0600 Subject: [PATCH] cleanups, utility methods extended template to use a compiletime nullClears (defaults to true) to clear nodes if they would be set to NULL entries cleanups and refactors yet more utility methods --- Engine/source/core/util/treeObject.cpp | 82 +-- Engine/source/core/util/treeObject.h | 731 +++++++++++++++++++------ 2 files changed, 622 insertions(+), 191 deletions(-) diff --git a/Engine/source/core/util/treeObject.cpp b/Engine/source/core/util/treeObject.cpp index 18f89a212..259a9338e 100644 --- a/Engine/source/core/util/treeObject.cpp +++ b/Engine/source/core/util/treeObject.cpp @@ -26,9 +26,20 @@ IMPLEMENT_CONOBJECT(TreeObject); TreeObject::~TreeObject() { - if (mRoot != NULL) - _deleteNode(mRoot, false); - + if (mRoot) + { + delete mRoot; + mRoot = NULL; + } + + // Clean up any orphaned nodes (safety measure) + for (Map::Iterator i = mKeyMap.begin(); i != mKeyMap.end(); ++i) + { + if (i->value && i->value->parent == NULL && i->value != mRoot) + { + delete i->value; + } + } mKeyMap.clear(); } @@ -132,42 +143,46 @@ S32 TreeObject::addNode(S32 parentKey, const char* scriptData, S32 forcedKey) void TreeObject::deleteNode(S32 key) { Node* node = findNode(key); - if (node == NULL) + if (!node) return; - if (node->parent != NULL) - { - Node* parentNode = static_cast(node->parent); - for (S32 i = 0; i < parentNode->size(); i++) - { - if ((*parentNode)[i] == static_cast*>(node)) - { - parentNode->erase(i); - break; - } - } - } if (node == mRoot) + { mRoot = NULL; + } + _deleteNode(node, true); } void TreeObject::_deleteNode(Node* node, bool unlinkFromMap) { - if (!node) return; + if (!node) + return; - for (U32 i = 0; i < node->size(); i++) + // Collect all keys in subtree for map cleanup + Vector keysToRemove; + if (unlinkFromMap) { - Node* child = static_cast((*node)[i]); - _deleteNode(child, unlinkFromMap); + node->forEachInSubtree([&keysToRemove](TreeNode* n) { + Node* scriptNode = static_cast(n); + keysToRemove.push_back(scriptNode->key); + }); } - - if (unlinkFromMap) { - mKeyMap.erase(node->key); - } - - if (node == mRoot) mRoot = NULL; + + // Remove from parent's children array + node->nullParent(); + + // Delete the node (destructor handles children recursively) delete node; + + // Clean up map entries + if (unlinkFromMap) + { + for (U32 i = 0; i < keysToRemove.size(); i++) + { + mKeyMap.erase(keysToRemove[i]); + } + } } TreeObject::Node* TreeObject::findNode(S32 key) @@ -192,6 +207,7 @@ bool TreeObject::toParent(S32 key, S32 newParentKey) if (!targetNode || !newParentNode || key == newParentKey) return false; + // Check for circular reference (prevent making ancestor a child) Node* checkNode = newParentNode; while (checkNode != NULL) { @@ -199,17 +215,9 @@ bool TreeObject::toParent(S32 key, S32 newParentKey) return false; checkNode = static_cast(checkNode->parent); } - if (targetNode->parent) - { - TreeNode* oldParent = targetNode->parent; - for (S32 i = 0; i < oldParent->size(); i++) { - if ((*oldParent)[i] == targetNode) { - oldParent->erase(i); - break; - } - } - } - + + // Remove from old parent and add to new parent + targetNode->nullParent(); targetNode->parent = newParentNode; newParentNode->push_back(targetNode); diff --git a/Engine/source/core/util/treeObject.h b/Engine/source/core/util/treeObject.h index 0ddd14a99..abd634481 100644 --- a/Engine/source/core/util/treeObject.h +++ b/Engine/source/core/util/treeObject.h @@ -27,239 +27,533 @@ #include "core/util/tDictionary.h" #include "console/consoleTypes.h" -template -class TreeNode : public Vector*> { +// Add nullClears as a template argument (default: true. set false for BVH/quad/octrees) +template +class TreeNode : public Vector*> { public: T data; - TreeNode* parent; - TreeNode(const T& val = T(), TreeNode* p = NULL) : data(val), parent(p) {} + TreeNode* parent; + TreeNode(const T& val = T(), TreeNode* p = NULL) : data(val), parent(p) {} + TreeNode(const Vector*>& children, TreeNode* p = NULL) + : data(T()), parent(p) + { + this->increment(children.size()); + for (U32 i = 0; i < children.size(); ++i) { + (*this)[i] = children[i]; + if (children[i]) + children[i]->parent = this; + } + } virtual ~TreeNode() { deleteChildren(); - if (parent) - { - for (U32 i = 0; i < parent->size(); i++) - { - if ((*parent)[i] == this) - { - parent->erase(i); - break; - } - } - parent = NULL; - } + parent = NULL; } //description logic inline bool isRoot() const { return parent == NULL; } - inline bool isLeaf() const { return size() == 0; } + inline bool isLeaf(bool checkNULLs = true) const { + if (!checkNULLs) + return this->size() == 0; + for (U32 i = 0; i < this->size(); ++i) { + if ((*this)[i] != NULL) + return false; + } + return true; + } // Parent logic - inline TreeNode* getParent() const { return parent; } + inline TreeNode* getParent() const { return parent; } - inline void setParent(TreeNode* p) + inline void setParent(TreeNode* p) { if (parent == p) return; if (parent) parent->removeChild(this); parent = p; - if (parent) parent->push_back(this); + if (parent) parent->push_back_unique(this); } //children logic - inline TreeNode* addChild(const T& val) + inline TreeNode* addChild(const T& val) { - TreeNode* child = new TreeNode(val, this); - push_back(child); + TreeNode* child = new TreeNode(val, this); + this->push_back_unique(child); return child; } - inline void addChild(TreeNode* child) - { - if (!child) return; - // Check if already a child to prevent duplicates - for (U32 i = 0; i < size(); i++) - { - if ((*this)[i] == child) - return; // Already a child + inline void addChild(TreeNode* child) + { + // Prevent duplicate child + for (U32 i = 0; i < this->size(); i++) + AssertFatal((*this)[i] != child, "TreeNode::addChild - Attempted to add duplicate child"); + +#ifdef TORQUE_DEBUG + // Prevent cycles + TreeNode* ancestor = this; + while (ancestor) { + AssertFatal(ancestor != child, "TreeNode::addChild - Would create circular parent relationship"); + ancestor = ancestor->parent; + } +#endif + + // If child has a parent and it's not this, remove from old parent + if (child->parent && child->parent != this) { + child->parent->removeChild(child); // Remove from old parent } - // Remove from old parent if needed - if (child->parent && child->parent != this) - child->parent->removeChild(child); - child->parent = this; - push_back(child); + this->push_back_unique(child); } - inline void setChild(U32 i, TreeNode* child) + + inline void setChild(U32 i, TreeNode* child) { - if (i >= size()) + if (i >= this->size()) { - U32 oldSize = size(); - increment((i + 1) - oldSize); - for (U32 j = oldSize; j < size(); j++) + U32 oldSize = this->size(); + this->increment((i + 1) - oldSize); + for (U32 j = oldSize; j < this->size(); j++) (*this)[j] = NULL; } - TreeNode* oldChild = (*this)[i]; + TreeNode* oldChild = (*this)[i]; if (oldChild == child) return; // Unlink old child if (oldChild) - { oldChild->parent = NULL; - } + // If forcing to NULL, just clear the slot and return if (child == NULL) { - (*this)[i] = NULL; + if constexpr (nullClears) + this->erase(i); + else + (*this)[i] = NULL; + return; } - else - { - // Handle moving from different parent vs same parent - if (child->parent && child->parent != this) - { - child->parent->removeChild(child); - } - else if (child->parent == this) - { - // Moving within same parent - find and remove from old slot - for (U32 j = 0; j < size(); j++) - { - if (j != i && (*this)[j] == child) - { - erase(j); - if (j < i) - i--; - break; - } - } - } - (*this)[i] = child; - child->parent = this; +#ifdef TORQUE_DEBUG + // Prevent cycles + TreeNode* ancestor = this; + while (ancestor) { + AssertFatal(ancestor != child, "TreeNode::setChild - Would create circular parent relationship"); + ancestor = ancestor->parent; } +#endif + + // Remove from old parent if different + if (child->parent && child->parent != this) + { + child->parent->removeChild(child); + } + else if (child->parent == this) + { + // Moving within same parent, nullify other references + for (U32 j = 0; j < this->size(); j++) + { + if (j != i && (*this)[j] == child) + if constexpr (nullClears) + this->erase(j); + else + (*this)[j] = NULL; + } + } + + // Prevent duplicate child + for (U32 j = 0; j < this->size(); j++) + AssertFatal(j == i || (*this)[j] != child, "TreeNode::setChild - Attempted to set duplicate child"); + + (*this)[i] = child; + child->parent = this; } - inline TreeNode* getChild(U32 i) const + inline TreeNode* getChild(U32 i) const { - if (i < size()) + if (i < this->size()) + { + // Defensive: validate pointer before returning + if ((*this)[i] && (*this)[i]->parent != this) + { + AssertFatal(false, "TreeNode::getChild - Child parent pointer mismatch"); + } return (*this)[i]; + } return NULL; } - inline void addChildren(const Vector* children) + + inline void addChildren(const Vector*>* children) { if (!children) return; for (U32 i = 0; i < children->size(); i++) { - TreeNode* child = new TreeNode((*children)[i], this); - push_back(child); + TreeNode* child = (*children)[i]; + + // Prevent duplicate child + for (U32 j = 0; j < this->size(); j++) + AssertFatal((*this)[j] != child, "TreeNode::addChildren - Attempted to add duplicate child"); + +#ifdef TORQUE_DEBUG + // Prevent cycles + TreeNode* ancestor = this; + while (ancestor) { + AssertFatal(ancestor != child, "TreeNode::addChildren - Would create circular parent relationship"); + ancestor = ancestor->parent; + } +#endif + + // If child has a parent and it's not this, remove from old parent + if (child->parent && child->parent != this) { + child->parent->removeChild(child); + } + + child->parent = this; + this->push_back_unique(child); } } - inline void removeChild(TreeNode* child) + inline void nullParent(bool orphan = true) { - if (!child) return; + // Null the parent pointer with optional cleanup + // @param orphan If true (default), properly removes from parent's children array first. + // If false, only nulls parent pointer (use during manual tree destruction). + if (orphan && parent) + { + parent->removeChild(this); + } + else + { + parent = NULL; + } + } - for (U32 i = 0; i < size(); i++) + inline bool isValidTree() const + { + // Validate tree structure - check parent-child relationships + for (U32 i = 0; i < this->size(); i++) + { + TreeNode* child = (*this)[i]; + if (child && child->parent != this) + return false; // Child doesn't point back to parent + } + return true; + } + + inline bool hasChild(TreeNode* child) const + { + if (!child) return false; + for (U32 i = 0; i < this->size(); i++) + { + if ((*this)[i] == child) + return true; + } + return false; + } + + inline S32 getChildIndex(TreeNode* child) const + { + if (!child) return -1; + for (U32 i = 0; i < this->size(); i++) + { + if ((*this)[i] == child) + return (S32)i; + } + return -1; + } + + inline void clearChildren() + { + // Remove all children without deleting them + for (U32 i = 0; i < this->size(); i++) + { + TreeNode* child = (*this)[i]; + if (child) + { + // Harden: ensure child is actually parented to this node + if (child->parent != this) + { +#ifdef TORQUE_DEBUG + Con::warnf("TreeNode::clearChildren - Child at index %u has incorrect parent pointer (corruption detected)", i); +#endif + // Detach anyway to avoid dangling pointer + child->parent = NULL; + } + else + { + child->parent = NULL; + } + } + } + clear(); + } + + inline void swapChildren(U32 i, U32 j) + { + if (i >= this->size() || j >= this->size() || i == j) + return; + + TreeNode* childI = (*this)[i]; + TreeNode* childJ = (*this)[j]; + + // Update parent pointers if needed + if (childI && childI->parent != this) + childI->parent = this; + if (childJ && childJ->parent != this) + childJ->parent = this; + + // Swap the pointers + (*this)[i] = childJ; + (*this)[j] = childI; + } + + inline TreeNode* findChild(const T& val) + { + for (U32 i = 0; i < this->size(); i++) + { + if ((*this)[i] && (*this)[i]->data == val) + return (*this)[i]; + } + return NULL; + } + + inline const TreeNode* findChild(const T& val) const + { + for (U32 i = 0; i < this->size(); i++) + { + if ((*this)[i] && (*this)[i]->data == val) + return (*this)[i]; + } + return NULL; + } + + template + inline void forEachChild(Func callback) + { + for (U32 i = 0; i < this->size(); i++) + { + if ((*this)[i]) + callback((*this)[i]); + } + } + + template + inline void forEachInSubtree(Func callback) + { + // Pre-order traversal: process node, then children + callback(this); + for (U32 i = 0; i < this->size(); i++) + { + if ((*this)[i]) + (*this)[i]->forEachInSubtree(callback); + } + } + template + inline void forEachLeaf(Func callback, bool checkNULLs = true) + { + if (isLeaf(checkNULLs)) + { + callback(this); + } + else + { + for (U32 i = 0; i < this->size(); i++) + { + if ((*this)[i]) + (*this)[i]->forEachLeaf(callback, checkNULLs); + } + } + } + + inline void findLeaves(Vector*>& outLeaves, bool checkNULLs = true) + { + forEachLeaf([&outLeaves](TreeNode* leaf) { + outLeaves.push_back_unique(leaf); + }, checkNULLs); + } + inline U32 getTreeSize() const + { + U32 count = 1; // Count this node + for (U32 i = 0; i < this->size(); i++) + { + if ((*this)[i]) + count += (*this)[i]->getTreeSize(); + } + return count; + } + + inline U32 getMaxDepth() const + { + if (isLeaf()) return 0; + + U32 maxDepth = 0; + for (U32 i = 0; i < this->size(); i++) + { + if ((*this)[i]) + { + U32 childDepth = (*this)[i]->getMaxDepth(); + if (childDepth > maxDepth) + maxDepth = childDepth; + } + } + return maxDepth + 1; + } + + inline void removeChild(TreeNode* child) + { + AssertFatal(child != NULL, "TreeNode::removeChild - Attempted to remove NULL child"); + bool found = false; + for (U32 i = 0; i < this->size(); i++) { if ((*this)[i] == child) { child->parent = NULL; - erase(i); - return; + if constexpr (nullClears) + this->erase(i); + else + (*this)[i] = NULL; + found = true; + break; } } + if (!found) + { +#ifdef TORQUE_DEBUG + // Defensive: attempt to repair if possible + for (U32 i = 0; i < this->size(); i++) + { + if ((*this)[i] && (*this)[i]->parent == this) + { + // Child pointer mismatch, but parent pointer matches: repair + (*this)[i]->parent = NULL; + if constexpr (nullClears) + this->erase(i); + else + (*this)[i] = NULL; + Con::warnf("TreeNode::removeChild - Repaired mismatched child pointer at index %u", i); + found = true; + break; + } + } + if (!found) + Con::errorf("TreeNode::removeChild - Child not found in parent's children array (corruption detected)"); +#endif + } + AssertFatal(found, "TreeNode::removeChild - Child not found in parent's children array"); } inline void removeChild(U32 i) { - if (i < size()) - { - TreeNode* child = (*this)[i]; - if (child) - child->parent = NULL; - erase(i); - } + AssertFatal(i < this->size(), "TreeNode::removeChild - Index out of bounds"); + TreeNode* child = (*this)[i]; + if (child) + child->parent = NULL; + if constexpr (nullClears) + this->erase(i); + else + (*this)[i] = NULL; } - inline void replaceChild(TreeNode* oldChild, TreeNode* newChild) + + inline void replaceChild(TreeNode* oldChild, TreeNode* newChild) { - if (!oldChild) return; + AssertFatal(oldChild != NULL, "TreeNode::replaceChild - Attempted to replace NULL child"); - for (U32 i = 0; i < size(); i++) - { - if ((*this)[i] == oldChild) - { - oldChild->parent = NULL; + S32 idx = getChildIndex(oldChild); + AssertFatal(idx != -1, "TreeNode::replaceChild - oldChild not found in children"); - if (newChild) - { - // Handle moving from different parent vs same parent - if (newChild->parent && newChild->parent != this) - { - newChild->parent->removeChild(newChild); - } - else if (newChild->parent == this) - { - // Moving within same parent - find and remove from old slot - for (U32 j = 0; j < size(); j++) - { - if (j != i && (*this)[j] == newChild) - { - erase(j); - if (j < i) - i--; - break; - } - } - } - - (*this)[i] = newChild; - newChild->parent = this; - } - else - { - (*this)[i] = NULL; - } - return; - } - } + // Use hardened setChild for all safety checks and pointer management + setChild((U32)idx, newChild); } inline void compact() { - // Remove NULL entries from children array - for (U32 i = 0; i < size(); ) + for (U32 i = 0; i < this->size(); ) { - if ((*this)[i] == NULL) - erase(i); - else + TreeNode* child = (*this)[i]; + if constexpr (nullClears) + { + // Compact style: erase NULL slots, keep only valid children. + if (child == NULL) + { + this->erase(i); + continue; + } + // Repair parent pointer if needed. + if (child->parent != this) + child->parent = this; i++; + } + else + { + // BVH/slot-stable style: leave NULLs, just repair parent pointers. + if (child && child->parent != this) + child->parent = this; + i++; + } } } - void operator =(Vector* other) { clear(); addChildren(other); } - - inline U32 getNumChildren() const { return size(); } - inline bool hasChildren() const { return size() > 0; } - inline Vector*> getChildren() const + inline bool hasChildren() const { - Vector*> children; - children.reserve(size()); - for (U32 i = 0; i < size(); i++) - { - if ((*this)[i]) // Skip NULLs - children.push_back((*this)[i]); + if constexpr (nullClears) + return this->size() > 0; + else { + for (U32 i = 0; i < this->size(); ++i) + if ((*this)[i]) return true; + return false; + } + } + + inline U32 getNumChildren() const { + if constexpr (nullClears) + return this->size(); + else { + U32 count = 0; + for (U32 i = 0; i < this->size(); ++i) + if ((*this)[i]) ++count; + return count; + } + } + + inline Vector*> getChildren() const + { + Vector*> children; + if constexpr (nullClears) { + // All slots are valid children + children = *this; + } + else { + // Only non-NULL slots are valid children + children.reserve(this->size()); + for (U32 i = 0; i < this->size(); ++i) + if ((*this)[i]) + children.push_back_unique((*this)[i]); } return children; } inline void deleteChildren() { - Vector*> children; - for (U32 i = 0; i < size(); i++) + Vector*> children; + for (U32 i = 0; i < this->size(); i++) { - children.push_back((*this)[i]); + TreeNode* child = (*this)[i]; + if (child) + { + // Harden: ensure child is actually parented to this node + if (child->parent != this) + { +#ifdef TORQUE_DEBUG + Con::warnf("TreeNode::deleteChildren - Child at index %u has incorrect parent pointer (corruption detected)", i); +#endif + // Detach anyway to avoid dangling pointer + child->parent = NULL; + } + else + { + child->parent = NULL; + } + } + children.push_back_unique(child); } clear(); @@ -267,24 +561,50 @@ public: { if (children[i]) { - children[i]->parent = NULL; + // Child should already be detached delete children[i]; } } } - //sibling logic - inline Vector*> getSiblings() const + inline void collectSubtree(Vector*>& nodes) { - Vector*> siblings; + // Pre-order collection of all nodes in subtree + forEachInSubtree([&nodes](TreeNode* n) { + nodes.push_back_unique(n); + }); + } + + inline void collectSubtreeData(Vector& dataList) + { + // Collect just the data from subtree + forEachInSubtree([&dataList](TreeNode* n) { + dataList.push_back_unique(n->data); + }); + } + + void collectAncestors(Vector*>& outAncestors) const + { + TreeNode* node = parent; + while (node) + { + outAncestors.push_back_unique(node); + node = node->parent; + } + } + + //sibling logic + inline Vector*> getSiblings() const + { + Vector*> siblings; if (!parent) return siblings; siblings.reserve(parent->size() - 1); for (U32 i = 0; i < parent->size(); i++) { - TreeNode* sibling = (*parent)[i]; + TreeNode* sibling = (*parent)[i]; if (sibling && sibling != this) // Skip NULLs - siblings.push_back(sibling); + siblings.push_back_unique(sibling); } return siblings; } @@ -299,7 +619,7 @@ public: inline U32 getDepth() const { U32 depth = 0; - TreeNode* current = parent; + TreeNode* current = parent; while (current) { depth++; @@ -308,21 +628,124 @@ public: return depth; } - inline TreeNode* getRoot() + inline TreeNode* getRoot() { - TreeNode* current = this; + TreeNode* current = this; while (current->parent) current = current->parent; return current; } - inline const TreeNode* getRoot() const + inline const TreeNode* getRoot() const { - const TreeNode* current = this; + const TreeNode* current = this; while (current->parent) current = current->parent; return current; } + + /// Returns true if this node is present in the given vector (used for cycle detection). + bool isVisited(const Vector*>& visited) const + { + for (U32 i = 0; i < visited.size(); ++i) + { + if (visited[i] == this) + return true; + } + return false; + } + + // Gather up to maxCandidates leaves or grandchildren for rotation from a node. + static U32 gatherRotationCandidates(TreeNode* parent, Vector*>& outNodes, U32 maxCandidates = 8) + { + outNodes.clear(); + for (U32 i = 0; i < parent->size() && outNodes.size() < maxCandidates; ++i) { + TreeNode* child = (*parent)[i]; + if (!child) continue; + if (child->isLeaf()) { + outNodes.push_back_unique(child); + } + else { + for (U32 j = 0; j < child->size() && outNodes.size() < maxCandidates; ++j) { + TreeNode* grandChild = (*child)[j]; + if (grandChild) outNodes.push_back_unique(grandChild); + } + } + } + return outNodes.size(); + } + + // Finds the best way to pair/group N nodes into two groups of groupSize each, minimizing cost. + // CostFunc: F32 (*costFunc)(const TreeNode*, const TreeNode*) + static bool findBestRotationPairing( + const Vector*>& nodes, + U32 groupSize, + F32(*costFunc)(const TreeNode*, const TreeNode*), + Vector& outGroupA, + Vector& outGroupB, + F32& outBestCost) + { + const U32 n = nodes.size(); + if (n < 2 * groupSize) return false; + + outBestCost = F32_MAX; + bool found = false; + + Vector indices; + indices.setSize(n); + for (U32 i = 0; i < n; ++i) indices[i] = i; + + Vector groupA, groupB; + groupA.setSize(groupSize); + groupB.setSize(groupSize); + + while (true) { + for (U32 i = 0; i < groupSize; ++i) + groupA[i] = indices[i]; + U32 bIdx = 0; + for (U32 i = groupSize; i < n; ++i) + groupB[bIdx++] = indices[i]; + if (bIdx == groupSize) { + F32 costA = costFunc(nodes[groupA[0]], nodes[groupA[1]]); + F32 costB = costFunc(nodes[groupB[0]], nodes[groupB[1]]); + F32 totalCost = costA + costB; + if (totalCost < outBestCost) { + outBestCost = totalCost; + outGroupA = groupA; + outGroupB = groupB; + found = true; + } + } + + S32 k = n - 2; + while (k >= 0 && indices[k] >= indices[k + 1]) --k; + if (k < 0) break; + S32 l = n - 1; + while (indices[k] >= indices[l]) --l; + U32 tmp = indices[k]; indices[k] = indices[l]; indices[l] = tmp; + for (U32 i = k + 1, j = n - 1; i < j; ++i, --j) { + tmp = indices[i]; indices[i] = indices[j]; indices[j] = tmp; + } + } + return found; + } + + //converters + + template + Derived* getChildAs(U32 i) { return static_cast(getChild(i)); } + template + const Derived* getChildAs(U32 i) const { return static_cast(getChild(i)); } + + template + Vector getChildrenAs() const + { + Vector out; + for (U32 i = 0; i < this->size(); ++i) + if ((*this)[i]) + out.push_back(static_cast((*this)[i])); + return out; + } }; class TreeObject : public SimObject {