Phase3 ecs hardening

ICE/Components/include/Component.h

@@ -10,6 +10,8 @@
 #include <cassert>
 #include <memory>
 #include <typeindex>
+#include <unordered_map>
+#include <vector>
 
 namespace ICE {
 
@@ -34,9 +36,9 @@ class ComponentArray : public IComponentArray {
         entityToIndexMap[entity] = newIndex;
         indexToEntityMap[newIndex] = entity;
         if (size < componentArray.size()) {
-            componentArray[newIndex] = component;
+            componentArray[newIndex] = std::move(component);
         } else {
-            componentArray.push_back(component);
+            componentArray.push_back(std::move(component));
         }
         size++;
     }
@@ -106,6 +108,9 @@ class ComponentManager {
         auto const& type = typeid(T);
 
         assert(componentTypes.find(type) == componentTypes.end() && "Registering component type more than once.");
+        // The signature is a bitset<64>; a component type index >= 64 would throw from
+        // signature.set() at runtime.
+        assert(nextComponentType < 64 && "Too many component types registered (max 64).");
 
         // Add this component type to the component type map
         componentTypes.insert({type, nextComponentType});
@@ -126,19 +131,20 @@ class ComponentManager {
 
     template<typename T>
     void addComponent(Entity entity, T component) {
-        // Add a component to the array for an entity
-        getComponentArray<T>()->insertData(entity, component);
+        // Add a component to the array for an entity; move to avoid an extra copy of
+        // potentially heavy components.
+        getComponentArray<T>().insertData(entity, std::move(component));
     }
 
     template<typename T>
     void removeComponent(Entity entity) {
-         getComponentArray<T>()->removeData(entity);
+        getComponentArray<T>().removeData(entity);
     }
 
     template<typename T>
     T* getComponent(Entity entity) {
         // Get a reference to a component from the array for an entity
-        return getComponentArray<T>()->getData(entity);
+        return getComponentArray<T>().getData(entity);
     }
 
     // Returns nullptr if the type is not registered or the entity has no such component,
@@ -172,12 +178,12 @@ class ComponentManager {
     // The component type to be assigned to the next registered component - starting at 0
     ComponentType nextComponentType{};
 
-    // Convenience function to get the statically casted pointer to the ComponentArray of type T.
+    // Reference to the ComponentArray of type T. Returns a reference (not a shared_ptr
+    // copy) to avoid an atomic refcount round-trip on every component access. .at() throws
+    // std::out_of_range for an unregistered type instead of operator[] inserting a null.
     template<typename T>
-    std::shared_ptr<ComponentArray<T>> getComponentArray() {
-        // .at() throws std::out_of_range for an unregistered component type instead of
-        // operator[] inserting a null shared_ptr that later null-derefs.
-        return std::static_pointer_cast<ComponentArray<T>>(componentArrays.at(typeid(T)));
+    ComponentArray<T>& getComponentArray() {
+        return static_cast<ComponentArray<T>&>(*componentArrays.at(typeid(T)));
     }
 };
 }  // namespace ICE

ICE/Components/include/SkeletonPoseComponent.h

@@ -1,6 +1,12 @@
 #pragma once
 
-#include <Component.h> 
+#include <Asset.h>
+#include <Component.h>
+
+#include <Eigen/Dense>
+#include <string>
+#include <unordered_map>
+#include <vector>
 
 namespace ICE {
 struct SkeletonPoseComponent : public Component {

ICE/Entity/include/Entity.h

@@ -14,7 +14,12 @@
 
 namespace ICE {
 using Entity = std::uint32_t;
-using Signature = std::bitset<32>;
+// 64 bits allows up to 64 component types (8 built-in + custom). See registerComponent's
+// bound check.
+using Signature = std::bitset<64>;
+
+// Reserved "no entity" / scene-graph-root sentinel.
+constexpr Entity NULL_ENTITY = 0;
 
 class EntityManager {
    public:
@@ -39,11 +44,21 @@ class EntityManager {
     }
 
     void releaseEntity(Entity e) {
-        signatures[e].reset();
+        // Guard against releasing a non-alive/already-released entity: that used to enqueue
+        // the same id twice (later handed out as two live entities) and underflow the count.
+        // Presence of a signature entry is the aliveness proxy.
+        if (!signatures.contains(e)) {
+            return;
+        }
+        signatures.erase(e);  // erase, not reset: the map was growing monotonically
         releasedEntities.push(e);
-        entityCount--;
+        if (entityCount > 0) {
+            entityCount--;
+        }
     }
 
+    bool isAlive(Entity e) const { return e != NULL_ENTITY && signatures.contains(e); }
+
     void setSignature(Entity e, Signature s) { signatures[e] = s; }
 
     Signature getSignature(Entity e) const {

ICE/Scene/include/Scene.h

@@ -4,6 +4,7 @@
 
 #pragma once
 
+#include <AssetBank.h>
 #include <Entity.h>
 #include <Registry.h>
 #include <SceneGraph.h>
@@ -16,23 +17,23 @@ class Renderer;
 
 class Scene {
    public:
-    Scene(const std::string& name);
+    Scene(const std::string &name);
 
-    bool setAlias(Entity entity, const std::string& newName);
-    std::string getAlias(Entity e);
+    bool setAlias(Entity entity, const std::string &newName);
+    std::string getAlias(Entity e) const;
 
     std::shared_ptr<SceneGraph> getGraph() const;
 
     std::string getName() const;
-    void setName(const std::string& name);
+    void setName(const std::string &name);
 
     std::shared_ptr<Registry> getRegistry() const;
     Entity createEntity();
-    Entity spawnTree(AssetUID model_id, const std::shared_ptr<AssetBank>& bank);
+    Entity spawnTree(AssetUID model_id, const std::shared_ptr<AssetBank> &bank);
 
-    void addEntity(Entity e, const std::string& alias, Entity parent);
+    void addEntity(Entity e, const std::string &alias, Entity parent);
     void removeEntity(Entity e);
-    bool hasEntity(Entity e);
+    bool hasEntity(Entity e) const;
 
    private:
     std::string name;

ICE/Scene/src/Scene.cpp

@@ -30,8 +30,11 @@ bool Scene::setAlias(Entity entity, const std::string &newName) {
     return true;
 }
 
-std::string Scene::getAlias(Entity e) {
-    return aliases[e];
+std::string Scene::getAlias(Entity e) const {
+    // find, not operator[]: the latter inserted an empty alias for unknown entities, which
+    // (combined with the old alias-based hasEntity) made a nonexistent entity "exist".
+    auto it = aliases.find(e);
+    return it == aliases.end() ? std::string() : it->second;
 }
 
 std::shared_ptr<Registry> Scene::getRegistry() const {
@@ -125,8 +128,10 @@ void Scene::removeEntity(Entity e) {
     m_graph->removeEntity(e);
 }
 
-bool Scene::hasEntity(Entity e) {
-    return aliases.contains(e);
+bool Scene::hasEntity(Entity e) const {
+    // Aliveness is owned by the registry, not the alias map (an entity can be alive without
+    // an alias, and getAlias no longer fabricates entries).
+    return registry->isAlive(e);
 }
 
 }  // namespace ICE

ICE/Storage/CMakeLists.txt

@@ -3,17 +3,14 @@ project(storage)
 
 message(STATUS "Building ${PROJECT_NAME} module")
 
-add_library(${PROJECT_NAME} STATIC)
+# Header-only after removing the empty Filesystem stub (JsonParser is the only content).
+add_library(${PROJECT_NAME} INTERFACE)
 
-target_sources(${PROJECT_NAME} PRIVATE
-    src/Filesystem.cpp
-)
-
-target_link_libraries(${PROJECT_NAME} PUBLIC
+target_link_libraries(${PROJECT_NAME} INTERFACE
     nlohmann_json
 )
 
-target_include_directories(${PROJECT_NAME} PUBLIC
+target_include_directories(${PROJECT_NAME} INTERFACE
         $<INSTALL_INTERFACE:include>
         $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>)
 

ICE/Storage/include/JsonParser.h

@@ -29,7 +29,7 @@ namespace ICE {
             return j;
         }
 
-        inline Eigen::Matrix4f readMat4(const json& j) {
+        static Eigen::Matrix4f readMat4(const json& j) {
             Eigen::Matrix4f mat;
             if (!j.is_array() || j.size() != 16)
                 throw std::runtime_error("Invalid JSON for Eigen::Matrix4f");

ICE/System/include/AnimationSystem.h

@@ -1,5 +1,6 @@
 #pragma once
 
+#include <AssetBank.h>
 #include <Registry.h>
 
 #include "Animation.h"
@@ -16,12 +17,12 @@ struct BonePose {
 
 class AnimationSystem : public System {
    public:
-    AnimationSystem(const std::shared_ptr<Registry>& reg, const std::shared_ptr<AssetBank>& bank);
+    AnimationSystem(const std::shared_ptr<Registry> &reg, const std::shared_ptr<AssetBank> &bank);
     void update(double delta) override;
 
     int updateOrder() const override { return AnimationSystemOrder; }
 
-    std::vector<Signature> getSignatures(const ComponentManager& comp_manager) const override {
+    std::vector<Signature> getSignatures(const ComponentManager &comp_manager) const override {
         Signature signature;
         signature.set(comp_manager.getComponentType<AnimationComponent>());
         signature.set(comp_manager.getComponentType<SkeletonPoseComponent>());
@@ -30,7 +31,7 @@ class AnimationSystem : public System {
 
    private:
     template<typename T>
-    size_t findKeyIndex(double animationTime, const std::vector<T>& keys) {
+    size_t findKeyIndex(double animationTime, const std::vector<T> &keys) {
         for (size_t i = 0; i < keys.size() - 1; ++i) {
             if (animationTime < keys[i + 1].timeStamp) {
                 return i;
@@ -39,20 +40,21 @@ class AnimationSystem : public System {
         return keys.size() - 1;
     }
 
-    void updateSkeleton(const std::shared_ptr<Model>& model, double time, SkeletonPoseComponent* pose, const Animation& anim);
-    void finalizePose();
+    void updateSkeleton(const std::shared_ptr<Model> &model, double time, SkeletonPoseComponent *pose, const Animation &anim);
+    void finalizePose(Entity e);
 
-    BonePose sampleBonePose(const std::string& boneName, const Animation& anim, double time, const std::shared_ptr<Model>& model);
-    static BonePose blendPoses(const BonePose& a, const BonePose& b, float factor);
+    BonePose sampleBonePose(const std::string &boneName, const Animation &anim, double time, const std::shared_ptr<Model> &model);
+    static BonePose blendPoses(const BonePose &a, const BonePose &b, float factor);
 
-    Eigen::Vector3f interpolatePosition(double timeInTicks, const BoneAnimation& track);
-    Eigen::Vector3f interpolateScale(double timeInTicks, const BoneAnimation& track);
-    Eigen::Quaternionf interpolateRotation(double time, const BoneAnimation& track);
+    Eigen::Vector3f interpolatePosition(double timeInTicks, const BoneAnimation &track);
+    Eigen::Vector3f interpolateScale(double timeInTicks, const BoneAnimation &track);
+    Eigen::Quaternionf interpolateRotation(double time, const BoneAnimation &track);
 
-    void applyTransforms(const Model::Node* node, const Eigen::Matrix4f& parentTransform, const Model::Skeleton& skeleton, double time,
-                         SkeletonPoseComponent* pose, const Animation& anim, const std::vector<Model::Node>& allModelNodes);
+    void applyTransforms(const Model::Node *node, const Eigen::Matrix4f &parentTransform, const Model::Skeleton &skeleton, double time,
+                         SkeletonPoseComponent *pose, const Animation &anim, const std::vector<Model::Node> &allModelNodes);
 
-    std::shared_ptr<Registry> m_registry;
+    // Non-owning back-reference (the Registry owns this system) to avoid an ownership cycle.
+    Registry* m_registry = nullptr;
     std::shared_ptr<AssetBank> m_asset_bank;
 };
 }  // namespace ICE

ICE/System/include/Registry.h

@@ -6,12 +6,10 @@
 #define ICE_REGISTRY_H
 
 #include <AnimationComponent.h>
-#include <CameraComponent.h>
 #include <Component.h>
 #include <Entity.h>
 #include <LightComponent.h>
 #include <RenderComponent.h>
-#include <RenderSystem.h>
 #include <SkeletonPoseComponent.h>
 #include <SkinningComponent.h>
 #include <SkyboxComponent.h>
@@ -27,7 +25,6 @@ class Registry {
         componentManager.registerComponent<TransformComponent>();
         componentManager.registerComponent<RenderComponent>();
         componentManager.registerComponent<LightComponent>();
-        componentManager.registerComponent<CameraComponent>();
         componentManager.registerComponent<SkyboxComponent>();
         componentManager.registerComponent<AnimationComponent>();
         componentManager.registerComponent<SkeletonPoseComponent>();
@@ -63,27 +60,34 @@ class Registry {
         systemManager.entityDestroyed(e);
     }
 
-    std::vector<Entity> getEntities() const { return entities; }
+    const std::vector<Entity>& getEntities() const { return entities; }
+
+    bool isAlive(Entity e) const { return entityManager.isAlive(e); }
 
     template<typename T>
-    bool entityHasComponent(Entity e) {
+    bool entityHasComponent(Entity e) const {
         return entityManager.getSignature(e).test(componentManager.getComponentType<T>());
     }
 
+    // WARNING: the returned pointer is only valid until the next structural change to the
+    // T component storage. addComponent<T>/removeComponent<T> on ANY entity can reallocate
+    // or swap-move the backing vector, invalidating outstanding T* handles. Do not cache
+    // component pointers across such changes -- re-fetch instead (see the editor Inspector).
     template<typename T>
     T *getComponent(Entity e) {
         return componentManager.getComponent<T>(e);
     }
 
-    // Returns nullptr instead of asserting when the entity has no component of type T.
+    // Returns nullptr instead of asserting when the entity has no component of type T. Same
+    // pointer-invalidation caveat as getComponent applies.
     template<typename T>
     T *tryGetComponent(Entity e) {
         return componentManager.tryGetComponent<T>(e);
     }
 
     template<typename T>
     void addComponent(Entity e, T component) {
-        componentManager.addComponent<T>(e, component);
+        componentManager.addComponent<T>(e, std::move(component));
         auto signature = entityManager.getSignature(e);
         signature.set(componentManager.getComponentType<T>(), true);
         entityManager.setSignature(e, signature);