ECS refactoring (breaks game code)

- Remove requirement of an Engine instance in ComponentMapper instances,
as the Engine from the entity passed to get() can be used. Allows for
static initialization of ComponentMappers.

- Store instances of ComponentMapper in a static Map within the
ComponentMapper class itself, rather that inside of Engine.

- Name bit/bits consistently. Some cases of bits were renamed to bit,
such as ComponentType getBits() is now getBit(). Change usage of Mask to
just Bits for consistency.

- Modify access modifiers and field finality where appropriate

- Add comments to public methods in Engine

core/src/com/me/common/ecs/BaseSystem.java

@@ -2,7 +2,7 @@ package com.me.common.ecs;
 
 public abstract class BaseSystem {
 
-    protected Engine engine;
+    protected final Engine engine;
 
     public BaseSystem(Engine engine) {
         this.engine = engine;

core/src/com/me/common/ecs/ComponentBag.java

@@ -1,38 +1,33 @@
 package com.me.common.ecs;
 
-public class ComponentBag {
+class ComponentBag {
 
     private Component[] items;
     private int size;
 
-    public ComponentBag() {
+    ComponentBag() {
         this.items = new Component[16];
         this.size = items.length;
     }
 
-    public Component get(int index) {
+    Component get(int index) {
         if (index >= size) {
             return null;
         }
         return items[index];
     }
 
-    public boolean contains(int index) {
-        return index < size && items[index] != null;
-    }
-
-    public void insert(int index, Component item) {
+    void insert(int index, Component item) {
         if (index >= size) {
             grow((int) (index * 1.5f));
         }
         items[index] = item;
     }
 
-    public void remove(int index) {
-        if (index >= size) {
-            throw new IndexOutOfBoundsException("index must be < size");
+    void remove(int index) {
+        if (index < size) {
+            items[index] = null;
         }
-        items[index] = null;
     }
 
     private void grow(int newSize) {

core/src/com/me/common/ecs/ComponentMapper.java

@@ -1,23 +1,36 @@
 package com.me.common.ecs;
 
-public class ComponentMapper<T extends Component> {
+import java.util.HashMap;
+import java.util.Map;
 
-    private Engine engine;
-    private ComponentType type;
-    private Class<T> typeClass;
+public final class ComponentMapper<T extends Component> {
 
-    public ComponentMapper(Engine engine, Class<T> typeClass) {
-        this.engine = engine;
+    private static final Map<Class<? extends Component>, ComponentMapper> mappers = new HashMap<>();
+
+    private final ComponentType type;
+    private final Class<T> typeClass;
+
+    private ComponentMapper(Class<T> typeClass) {
         this.type = ComponentType.getComponentType(typeClass);
         this.typeClass = typeClass;
     }
 
     public T get(Entity entity) {
-        return typeClass.cast(engine.getEntityComponent(entity, type));
+        return typeClass.cast(entity.getComponent(type));
     }
 
     public boolean has(Entity entity) {
-        return type.isTypeInMask(entity.componentBits);
+        return type.isInBits(entity.componentBits);
+    }
+
+    @SuppressWarnings("unchecked")
+    public static <T extends Component> ComponentMapper<T> getFor(Class<T> typeClass) {
+        ComponentMapper<T> mapper = mappers.get(typeClass);
+        if (mapper == null) {
+            mapper = new ComponentMapper<>(typeClass);
+            mappers.put(typeClass, mapper);
+        }
+        return mapper;
     }
 
 }

core/src/com/me/common/ecs/ComponentType.java

@@ -6,64 +6,44 @@ import java.util.Map;
 final class ComponentType {
 
     private static final Map<Class<? extends Component>, ComponentType> types = new HashMap<>();
-    private static final ComponentType[] typeById = new ComponentType[Long.SIZE];
 
     private static long nextBit = 1l;
     private static int nextId = 0;
 
-    protected long bits;
-    protected int id;
+    final long bit;
+    final int id;
 
     private ComponentType() {
-        this.bits = nextBit;
+        this.bit = nextBit;
         this.id = nextId++;
 
-        this.nextBit <<= 1;
+        nextBit <<= 1;
     }
 
-    protected long getBits() {
+    boolean isInBits(long bits) {
+        return (bits & bit) == bit;
+    }
+
+    @SafeVarargs
+    static long getBitsFor(Class<? extends Component>... components) {
+        long bits = 0l;
+        for (Class<? extends Component> clazz : components) {
+            bits |= getComponentType(clazz).bit;
+        }
         return bits;
     }
 
-    protected int getId() {
-        return id;
-    }
-
-    protected static long getMaskBits(Class<? extends Component>... components) {
-        long mask = 0l;
-        for (Class<? extends Component> clazz : components) {
-            mask |= getTypeBits(clazz);
-        }
-        return mask;
-    }
-
-    protected static long getTypeBits(Class<? extends Component> component) {
-        return getComponentType(component).bits;
-    }
-
-    protected static int getTypeId(Class<? extends Component> component) {
-        return getComponentType(component).id;
-    }
-
-    protected static ComponentType getById(int id) {
-        return typeById[id];
-    }
-
-    protected boolean isTypeInMask(long mask) {
-        return (bits & mask) == bits;
-    }
-
-    protected static void registerComponentType(Class<? extends Component> component) {
+    static ComponentType registerComponentType(Class<? extends Component> component) {
         ComponentType type = types.get(component);
         if (type != null) {
             throw new IllegalArgumentException(component.getName() + " has already been registered.");
         }
         type = new ComponentType();
         types.put(component, type);
-        typeById[type.id] = type;
+        return type;
     }
 
-    protected static ComponentType getComponentType(Class<? extends Component> component) {
+    static ComponentType getComponentType(Class<? extends Component> component) {
         ComponentType type = types.get(component);
         if (type == null) {
             throw new IllegalArgumentException(component.getName() + " has not been registered.");
@@ -71,8 +51,4 @@ final class ComponentType {
         return type;
     }
 
-    protected static int getRegisteredComponentTypeCount() {
-        return types.size();
-    }
-
 }

core/src/com/me/common/ecs/Engine.java

@@ -4,46 +4,51 @@ import com.badlogic.gdx.utils.Array;
 import com.me.common.ecs.event.Event;
 import com.me.common.ecs.event.Listener;
 
-import java.util.HashMap;
-import java.util.Map;
-
 public class Engine {
 
-    private ComponentBag[] components;
-    protected Array<BaseSystem> systems;
+    private final EntityManager entityManager;
+    private final ComponentBag[] components;
+    private final Array<BaseSystem> systems;
 
-    private EntityManager entityManager;
-    private ListenerRegistry listenerRegistry;
-
-    private Map<Class<? extends Component>, ComponentMapper> componentMappers;
+    private final ListenerRegistry listenerRegistry;
 
     public Engine() {
         this.entityManager = new EntityManager(this);
-
+        this.components = new ComponentBag[Long.SIZE];
         this.systems = new Array<>();
+
         this.listenerRegistry = new ListenerRegistry();
-        this.componentMappers = new HashMap<>();
     }
 
+    /**
+     * Register a Component class.
+     * @param clazz
+     */
     public void registerComponentClass(Class<? extends Component> clazz) {
-        ComponentType.registerComponentType(clazz);
+        ComponentType type = ComponentType.registerComponentType(clazz);
+        components[type.id] = new ComponentBag();
     }
 
+    /**
+     * Register a BaseSystem.
+     * @param system
+     */
     public void registerSystem(BaseSystem system) {
-        this.systems.add(system);
+        systems.add(system);
     }
 
+    /**
+     * Register an event Listener.
+     * @param listener
+     */
     public void registerListener(Listener listener) {
         listenerRegistry.registerListener(listener);
     }
 
-    public void ready() {
-        this.components = new ComponentBag[ComponentType.getRegisteredComponentTypeCount()];
-        for (int i = 0; i < components.length; i++) {
-            components[i] = new ComponentBag();
-        }
-    }
-
+    /**
+     * Process all systems
+     * @param dt the time (in seconds) passed since the last update
+     */
     public void update(float dt) {
         entityManager.update();
 
@@ -54,61 +59,70 @@ public class Engine {
         }
     }
 
+    /**
+     * @return all entities known by the engine - both those active and inactive.
+     */
     public Array<Entity> getEntities() {
-        return entityManager.entities;
+        return entityManager.getEntities();
     }
 
-    public int getEntityCount() {
-        return entityManager.entities.size;
+    /**
+     * @return all currently registered systems.
+     */
+    public Array<BaseSystem> getSystems() {
+        return systems;
     }
 
+    /**
+     * @return a new Entity object which can have fresh components added to it
+     */
     public Entity createEntity() {
         return entityManager.create();
     }
 
+    /**
+     * Calls the given event, dispatching it to all registered Listeners that have an EventHandler
+     * for this particular event.
+     * @param event the event to call
+     */
     public void callEvent(Event event) {
         listenerRegistry.callEvent(event);
     }
 
-    protected void refreshEntity(Entity entity) {
+    void refreshEntity(Entity entity) {
         entityManager.queueRefresh(entity);
     }
 
-    protected void removeAllEntityComponents(int entityId) {
-        for (int i = 0; i < components.length; i++) {
-            components[i].insert(entityId, null);
+    void removeAllEntityComponents(int entityId) {
+        for (ComponentBag bag : components) {
+            if (bag == null) {
+                // The rest of the array should also be null given that we never nullify
+                // an index after it has been set, and they are set sequentially. So, break.
+                break;
+            }
+            bag.remove(entityId);
         }
     }
 
-    protected void addEntityComponent(Entity entity, Component component) {
+    void addEntityComponent(Entity entity, Component component) {
         ComponentType type = ComponentType.getComponentType(component.getClass());
         components[type.id].insert(entity.id, component);
         entity.addComponentType(type);
     }
 
 
-    protected void removeEntityComponent(Entity entity, Component component) {
+    void removeEntityComponent(Entity entity, Component component) {
         ComponentType type = ComponentType.getComponentType(component.getClass());
         components[type.id].remove(entity.id);
         entity.removeComponentType(type);
     }
 
-    protected <T extends Component> T getEntityComponent(Entity entity, Class<T> clazz) {
-        return clazz.cast(components[ComponentType.getTypeId(clazz)].get(entity.id));
+    <T extends Component> T getEntityComponent(Entity entity, Class<T> clazz) {
+        return clazz.cast(getEntityComponent(entity, ComponentType.getComponentType(clazz)));
     }
 
-    protected Component getEntityComponent(Entity entity, ComponentType type) {
+    Component getEntityComponent(Entity entity, ComponentType type) {
         return components[type.id].get(entity.id);
     }
 
-    @SuppressWarnings("unchecked")
-    public <T extends Component> ComponentMapper<T> getComponentMapper(Class<T> typeClass) {
-        ComponentMapper<T> mapper = componentMappers.get(typeClass);
-        if (mapper == null) {
-            mapper = new ComponentMapper<>(this, typeClass);
-            componentMappers.put(typeClass, mapper);
-        }
-        return mapper;
-    }
-
 }

core/src/com/me/common/ecs/Entity.java

@@ -5,15 +5,15 @@ public final class Entity {
     private static int nextId = 0;
     private static long nextUniqueId = 0;
 
-    private Engine engine;
-    protected final int id;
+    private final Engine engine;
+    final int id;
 
-    protected long componentBits;
-    protected long systemEnabledBits;
+    long componentBits;
+    long systemEnabledBits;
+    boolean pendingRefresh;
 
-    protected boolean active;
-    protected boolean removed;
-    protected boolean pendingRefresh;
+    boolean active;
+    boolean removed;
 
     private long uniqueId;
 
@@ -30,6 +30,10 @@ public final class Entity {
         return uniqueId;
     }
 
+    public Engine getEngine() {
+        return engine;
+    }
+
     public boolean isActive() {
         return active;
     }
@@ -60,18 +64,6 @@ public final class Entity {
         }
     }
 
-    public Engine getEngine() {
-        return this.engine;
-    }
-
-    public boolean hasComponent(Class<? extends Component> clazz) {
-        return ComponentType.getComponentType(clazz).isTypeInMask(componentBits);
-    }
-
-    public <T extends Component> T getComponent(Class<T> clazz) {
-        return engine.getEntityComponent(this, clazz);
-    }
-
     public void addComponent(Component component) {
         engine.addEntityComponent(this, component);
         refresh();
@@ -82,39 +74,49 @@ public final class Entity {
         refresh();
     }
 
-    protected void addComponentType(ComponentType type) {
-        componentBits |= type.bits;
-    }
-
-    protected void removeComponentType(ComponentType type) {
-        componentBits &= ~type.bits;
-    }
-
-    protected void addSystemEnabledBit(long bit) {
-        systemEnabledBits |= bit;
-    }
-
-    protected void removeSystemEnabledBit(long bit) {
-        systemEnabledBits &= ~bit;
-    }
-
-    public void refresh() {
+    private void refresh() {
         if (!pendingRefresh) {
             engine.refreshEntity(this);
             pendingRefresh = true;
         }
     }
 
-    protected void reset() {
+    public boolean hasComponent(Class<? extends Component> clazz) {
+        return ComponentType.getComponentType(clazz).isInBits(componentBits);
+    }
+
+    public <T extends Component> T getComponent(Class<T> clazz) {
+        return engine.getEntityComponent(this, clazz);
+    }
+
+    // Internal getComponent method accessed only by ComponentMapper
+    Component getComponent(ComponentType type) {
+        return engine.getEntityComponent(this, type);
+    }
+
+    void addComponentType(ComponentType type) {
+        componentBits |= type.bit;
+    }
+
+    void removeComponentType(ComponentType type) {
+        componentBits &= ~type.bit;
+    }
+
+    void addSystemEnabledBit(long systemBit) {
+        systemEnabledBits |= systemBit;
+    }
+
+    void removeSystemEnabledBit(long systemBit) {
+        systemEnabledBits &= ~systemBit;
+    }
+
+    void reset() {
         componentBits = 0;
         systemEnabledBits = 0;
         active = false;
         removed = false;
         pendingRefresh = false;
-    }
-
-    protected void updateUniqueId() {
-        this.uniqueId = nextUniqueId++;
+        uniqueId = nextUniqueId++;
     }
 
     public String toString() {

core/src/com/me/common/ecs/EntityManager.java

@@ -4,11 +4,11 @@ import com.badlogic.gdx.utils.Array;
 
 final class EntityManager {
 
-    private Engine engine;
+    private final Engine engine;
 
-    protected Array<Entity> entities;
-    protected Array<Entity> removedEntities;
-    protected Array<Entity> toRefresh;
+    private final Array<Entity> entities;
+    private final Array<Entity> removedEntities;
+    private final Array<Entity> toRefresh;
 
     EntityManager(Engine engine) {
         this.engine = engine;
@@ -18,7 +18,11 @@ final class EntityManager {
         this.toRefresh = new Array<>(false, 16);
     }
 
-    public Entity create() {
+    Array<Entity> getEntities() {
+        return entities;
+    }
+
+    Entity create() {
         Entity entity;
         if (!removedEntities.isEmpty()) {
             entity = removedEntities.removeIndex(0);
@@ -26,12 +30,11 @@ final class EntityManager {
             entity = new Entity(engine);
         }
         entity.reset();
-        entity.updateUniqueId();
         entities.add(entity);
         return entity;
     }
 
-    public void update() {
+    void update() {
         if (toRefresh.isEmpty()) {
             return;
         }
@@ -44,12 +47,12 @@ final class EntityManager {
         toRefresh.clear();
     }
 
-    public void queueRefresh(Entity entity) {
+    void queueRefresh(Entity entity) {
         toRefresh.add(entity);
     }
 
     private void refreshEntity(Entity entity) {
-        for (BaseSystem system : engine.systems) {
+        for (BaseSystem system : engine.getSystems()) {
             if (!(system instanceof EntitySystem)) {
                 continue;
             }
@@ -57,7 +60,7 @@ final class EntityManager {
             ((EntitySystem) system).refresh(entity);
         }
 
-        if (entity.removed) {
+        if (entity.isRemoved()) {
             engine.removeAllEntityComponents(entity.id);
             entities.removeValue(entity, true);
             removedEntities.add(entity);

core/src/com/me/common/ecs/EntitySystem.java

@@ -10,15 +10,16 @@ public abstract class EntitySystem extends BaseSystem {
     private static final Map<Class<? extends EntitySystem>, Long> systemBits = new HashMap<>();
     private static long nextBit = 1l;
 
-    private long systemBit;
-    private long typeBits;
+    private final long systemBit;
+    private final long componentBits;
 
-    protected Array<Entity> entities;
+    private final Array<Entity> entities;
 
+    @SafeVarargs
     public EntitySystem(Engine engine, Class<? extends Component>... components) {
         super(engine);
         this.systemBit = getBitFor(getClass());
-        this.typeBits = ComponentType.getMaskBits(components);
+        this.componentBits = ComponentType.getBitsFor(components);
         this.entities = new Array<>(true, 16, Entity.class);
     }
 
@@ -37,13 +38,13 @@ public abstract class EntitySystem extends BaseSystem {
 
     public abstract void processEntity(Entity entity, float dt);
 
-    protected void refresh(Entity entity) {
+    void refresh(Entity entity) {
         boolean enabled = (entity.systemEnabledBits & systemBit) == systemBit;
-        boolean interested = (entity.componentBits & typeBits) == typeBits;
+        boolean interested = (entity.componentBits & componentBits) == componentBits;
 
         if (interested && !enabled && entity.active && !entity.removed) {
             add(entity);
-        } else if (enabled && (!interested || !entity.active || entity.removed)) {
+        } else if (enabled && (!interested || !entity.active) || entity.removed) {
             remove(entity);
         }
     }
@@ -59,7 +60,7 @@ public abstract class EntitySystem extends BaseSystem {
     }
 
 
-    static long getBitFor(Class<? extends EntitySystem> es) {
+    private static long getBitFor(Class<? extends EntitySystem> es) {
         Long bits = systemBits.get(es);
         if (bits == null) {
             bits = nextBit;

core/src/com/me/common/ecs/ListenerRegistry.java

@@ -11,15 +11,15 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
-public class ListenerRegistry {
+class ListenerRegistry {
 
     private final Map<Class<? extends Event>, List<RegisteredListener>> registeredListeners;
 
-    protected ListenerRegistry() {
+    ListenerRegistry() {
         this.registeredListeners = new HashMap<>();
     }
 
-    protected void registerListener(Listener listener) {
+    void registerListener(Listener listener) {
         for (Method method : listener.getClass().getMethods()) {
             EventHandler eh = method.getAnnotation(EventHandler.class);
             if (eh == null) {
@@ -37,10 +37,9 @@ public class ListenerRegistry {
             List<RegisteredListener> executors = getRegisteredListeners(eventClass);
             executors.add(new RegisteredListener(listener, method, eventClass, eh));
         }
-
     }
 
-    protected void callEvent(Event event) {
+    void callEvent(Event event) {
         List<RegisteredListener> listeners = getRegisteredListeners(event.getClass());
         if (listeners.isEmpty()) {
             return;

core/src/com/me/common/ecs/event/RegisteredListener.java

@@ -4,11 +4,10 @@ import java.lang.reflect.Method;
 
 public class RegisteredListener {
 
-    private Listener listener;
-    private Method method;
-    private Class<? extends Event> eventType;
-
-    public EventHandler eh;
+    private final Listener listener;
+    private final Method method;
+    private final Class<? extends Event> eventType;
+    private final EventHandler eh;
 
     public RegisteredListener(Listener listener, Method method, Class<? extends Event> eventType, EventHandler eh) {
         this.listener = listener;