Commits

Michael Ludwig committed ba2b64f

Improve version handling for components and remove getIfModified().

Comments (0)

Files changed (6)

 
 ## Release Notes
 
+### 1.6.1
+* Remove `Entity.getIfModified()` method because its semantics were vague and unhelpful.
+* Make version numbers unique within a component type.
+* Simplify `isEnabled()` and `getVersion()` logic for invalid components.
+
 ### 1.6.0
 * Completely replace old Controller API with a multi-threading oriented Task 
   and Job API.

src/main/java/com/lhkbob/entreri/ComponentData.java

      * @return True if the component is enabled, false if disabled or invalid
      */
     public final boolean isEnabled() {
-        return index != 0 && owner.isEnabled(index);
+        return owner.isEnabled(index);
     }
 
     /**
      * associated component's version so comparing a previously cached version
      * number can be used to determine when changes have been made.
      * <p>
-     * If the
+     * Additionally, for a given component type, versions will be unique. Thus
+     * it is possible to identify when the components are replaced by new
+     * components as well.
      * 
-     * @return The current version
+     * @return The current version, or a negative number if the data is invalid
      */
     public final int getVersion() {
         return owner.getVersion(index);

src/main/java/com/lhkbob/entreri/ComponentRepository.java

     private final IntProperty componentIdProperty;
     private final IntProperty componentVersionProperty;
     private int idSeq;
+    private int versionSeq;
 
     /**
      * Create a ComponentRepository for the given system, that will store
         componentVersionProperty = decorate(new IntProperty.Factory(0));
 
         idSeq = 1; // start at 1, just like entity id sequences
+        versionSeq = 0;
+
+        // initialize enabled and version for the 0th index
+        enabledProperty.set(false, 0);
+        componentVersionProperty.set(-1, 0);
     }
 
     /**
 
     /**
      * Set whether or not the component at <tt>componentIndex</tt> is enabled.
+     * This does nothing if the index is 0, preserving the guarantee that
+     * invalid component is considered disabled.
      * 
      * @param componentIndex The component index
      * @param enabled True if the component is enabled
      */
     public void setEnabled(int componentIndex, boolean enabled) {
-        enabledProperty.set(enabled, componentIndex);
+        if (componentIndex != 0) {
+            enabledProperty.set(enabled, componentIndex);
+        }
     }
 
     /**
     }
 
     /**
-     * Increment the component's version at the given index.
+     * Increment the component's version at the given index. This does nothing
+     * if the index is 0, preserving the guarantee that an invalid component has
+     * a negative version.
      * 
      * @param componentIndex
      */
     public void incrementVersion(int componentIndex) {
-        componentVersionProperty.set(componentVersionProperty.get(componentIndex) + 1,
-                                     componentIndex);
+        if (componentIndex != 0) {
+            // clamp it to be above 0, instead of going negative
+            int newVersion = (0xefffffff & (versionSeq++));
+            componentVersionProperty.set(newVersion, componentIndex);
+        }
     }
 
     /*
         // this is needed because we might be overwriting a previously removed
         // component, or the factory might be doing something tricky
         for (int i = 0; i < declaredProperties.size(); i++) {
-            declaredProperties.get(i).setValue(componentIndex);
+            declaredProperties.get(i).setDefaultValue(componentIndex);
         }
 
         for (int i = 0; i < decoratedProperties.size(); i++) {
-            decoratedProperties.get(i).setValue(componentIndex);
+            decoratedProperties.get(i).setDefaultValue(componentIndex);
         }
 
         // although there could be a custom PropertyFactory for setting the id,
         // it's easier to assign a new id here
         componentIdProperty.set(idSeq++, componentIndex);
 
+        // start with a unique version as well
+        incrementVersion(componentIndex);
+
         // ensure required components are added as well
         Entity entity = system.getEntityByIndex(entityIndex);
         for (int i = 0; i < requiredTypes.length; i++) {
         IndexedDataStore newStore = prop.getDataStore().create(size);
         prop.setDataStore(newStore);
         for (int i = 1; i < size; i++) {
-            pstore.setValue(i);
+            pstore.setDefaultValue(i);
         }
 
         decoratedProperties.add(pstore);
             swap = null;
         }
 
-        void setValue(int index) {
+        void setDefaultValue(int index) {
             P prop = getProperty();
             if (prop != null) {
                 creator.setDefaultValue(prop, index);

src/main/java/com/lhkbob/entreri/Entity.java

 
     /**
      * <p>
-     * Get the component of type T attached to this entity only if the entity
-     * has a component of type T, it is enabled, and it's version is different
-     * than <tt>priorVersion</tt>. This method is a convenience method to easily
-     * retrieve the component data only if the data has changed.
-     * <p>
-     * If the entity has a component of type T, that is enabled, and has a
-     * version not equal to <tt>priorVersion</tt>, then <tt>data</tt> will be
-     * set to access the component and true will be returned. In any other case,
-     * false is returned. Unlike {@link #get(ComponentData)}, it's not possible
-     * to inspect the data instance to determine why false was returned.
-     * 
-     * @param <T> The component data type
-     * @param data The data instance used to access the component if available
-     * @param priorVersion The previously valid version
-     * @return True if the entity has an enabled component of type T that has
-     *         been modified since <tt>priorVersion</tt>
-     * @throws NullPointerException if data is null
-     * @throws IllegalArgumentException if data was created by another entity
-     *             system
-     */
-    public <T extends ComponentData<T>> boolean getIfModified(T data, int priorVersion) {
-        if (data.owner.getEntitySystem() != getEntitySystem()) {
-            throw new IllegalArgumentException("ComponentData was not created by expected EntitySystem");
-        }
-
-        ComponentRepository<T> ci = data.owner;
-        int componentIndex = ci.getComponentIndex(index);
-
-        if (ci.getVersion(componentIndex) != priorVersion) {
-            return data.setFast(componentIndex) && ci.isEnabled(componentIndex);
-        } else {
-            // note that in this situation, the component data is invalid
-            return false;
-        }
-    }
-
-    /**
-     * <p>
      * Add a new Component with a data type T to this Entity. If there already
      * exists a component of type T, it is removed first, and a new one is
      * instantiated.

src/test/java/com/lhkbob/entreri/ComponentDataTest.java

     }
 
     @Test
-    public void testInitialVersion() {
-        EntitySystem system = new EntitySystem();
-
-        Entity e = system.addEntity();
-        Assert.assertEquals(0, e.add(IntComponent.class).getData().getVersion());
-        // ensure that overrides properly set it too
-        Assert.assertEquals(0, e.add(IntComponent.class).getData().getVersion());
-    }
-
-    @Test
     public void testVersionUpdate() {
         EntitySystem system = new EntitySystem();
         Entity e = system.addEntity();
         cd.updateVersion();
         Assert.assertEquals(1, cd.getVersion());
     }
+
+    @Test
+    public void testUniqueVersionUpdate() {
+        EntitySystem system = new EntitySystem();
+        IntComponent cd1 = system.addEntity().add(IntComponent.class).getData();
+        IntComponent cd2 = system.addEntity().add(IntComponent.class).getData();
+
+        Assert.assertEquals(0, cd1.getVersion());
+        Assert.assertEquals(1, cd2.getVersion());
+
+        cd1.updateVersion();
+        cd2.updateVersion();
+
+        Assert.assertEquals(2, cd1.getVersion());
+        Assert.assertEquals(3, cd2.getVersion());
+    }
+
+    @Test
+    public void testInvalidComponentVersion() {
+        EntitySystem system = new EntitySystem();
+        IntComponent cd = system.createDataInstance(IntComponent.class);
+
+        // sanity check
+        Assert.assertEquals(0, cd.getIndex());
+
+        int oldVersion = cd.getVersion();
+        Assert.assertTrue(oldVersion < 0);
+        cd.updateVersion();
+        Assert.assertEquals(oldVersion, cd.getVersion());
+    }
+
+    @Test
+    public void testInvalidComponentEnabled() {
+        EntitySystem system = new EntitySystem();
+        IntComponent cd = system.createDataInstance(IntComponent.class);
+
+        // sanity check
+        Assert.assertEquals(0, cd.getIndex());
+
+        Assert.assertFalse(cd.isEnabled());
+        cd.setEnabled(true);
+        Assert.assertFalse(cd.isEnabled());
+    }
 }

src/test/java/com/lhkbob/entreri/EntityTest.java

     }
 
     @Test
-    public void testGetIfModified() {
-        EntitySystem system = new EntitySystem();
-        Entity e = system.addEntity();
-
-        IntComponent cd = e.add(IntComponent.class).getData();
-        IntComponent access = system.createDataInstance(IntComponent.class);
-
-        Assert.assertEquals(0, cd.getVersion());
-        Assert.assertFalse(e.getIfModified(access, 0));
-
-        cd.updateVersion();
-        Assert.assertTrue(e.getIfModified(access, 0));
-    }
-
-    @Test
     public void testAddWithRequiredComponents() {
         EntitySystem system = new EntitySystem();
         Entity e = system.addEntity();