Fixes for property setting via atomic test-and-set:

- Moved property testing, so that it happens before any propertySet events are
  dispatched. In the previous version, testing happened while processing the
  set event on the server - but since by that time client events have already been 
  sent, it introduced the possibility of short-lived inconsistencies between client 
  and server data models.   

- Introduced a separate EZ API call for test and set - not only does it perform
  the test, but unlike regular set, it does not cache the new value ahead of time. 
  Instead the new value will have to arrive from the server, at some future point.

- Trimmed PropertySetEvent and other handlers back down - they don't need to
  carry any of the test info around, after it's already been performed. Also 
  cut redundant testing on the clients.




git-svn-id: svn+ssh://src.earth.threerings.net/vilya/trunk@195 c613c5cb-e716-0410-b11b-feb51c14d237

src/as/com/threerings/ezgame/EZGameControl.as

@@ -153,24 +153,31 @@ public class EZGameControl extends EventDispatcher
     }
 
     /**
-     * Set a property that will be distributed.
+     * Set a property that will be distributed. 
      */
     public function set (propName :String, value :Object, index :int = -1) :void
     {
-        callEZCode("setProperty_v2", propName, value, index, false);
+        callEZCode("setProperty_v1", propName, value, index);
     }
 
     /**
-     * Set a property that will be distributed, but only if it was null before.
+     * Set a property that will be distributed, but only if it's equal
+     * to the specified test value.
+     *
+     * Please note that, unlike in the standard set() function, the property
+     * will not be updated right away, but will require a request to the server
+     * and a response back. For this reason, there may be a considerable delay
+     * between calling testAndSet, and seeing the result of the update.
      *
      * The operation is 'atomic', in the sense that testing and setting take place
      * during the same server event. In comparison, a separate 'get' followed by
      * a 'set' operation would involve two events with two network round-trips,
      * and no guarantee that the value won't change between the events.
      */
-    public function testAndSet (propName :String, value :Object, index :int = -1) :void
+    public function testAndSet (
+        propName :String, newValue :Object, testValue :Object, index :int = -1) :void
     {
-        callEZCode("setProperty_v2", propName, value, index, true);
+        callEZCode("testAndSetProperty_v1", propName, newValue, testValue, index);
     }
 
     /**

src/as/com/threerings/ezgame/client/EZGameService.as

@@ -67,7 +67,7 @@ public interface EZGameService extends InvocationService
     function setCookie (arg1 :Client, arg2 :ByteArray, arg3 :InvocationService_InvocationListener) :void;
 
     // from Java interface EZGameService
-    function setProperty (arg1 :Client, arg2 :String, arg3 :Object, arg4 :int, arg5 :Boolean, arg6 :InvocationService_InvocationListener) :void;
+    function setProperty (arg1 :Client, arg2 :String, arg3 :Object, arg4 :int, arg5 :Boolean, arg6 :Object, arg7 :InvocationService_InvocationListener) :void;
 
     // from Java interface EZGameService
     function setTicker (arg1 :Client, arg2 :String, arg3 :int, arg4 :InvocationService_InvocationListener) :void;

src/as/com/threerings/ezgame/client/GameControlBackend.as

@@ -72,7 +72,7 @@ public class GameControlBackend
     {
         _ctx = ctx;
         _ezObj = ezObj;
-        _gameData = new GameData(setProperty_v2, _ezObj.getUserProps());
+        _gameData = new GameData(setProperty_v1, _ezObj.getUserProps());
 
         _ezObj.addListener(this);
         _ctx.getClient().getClientObject().addListener(_userListener);
@@ -136,7 +136,8 @@ public class GameControlBackend
         o["gameData"] = _gameData;
 
         // functions
-        o["setProperty_v2"] = setProperty_v2;
+        o["setProperty_v1"] = setProperty_v1;
+        o["testAndSetProperty_v1"] = testAndSetProperty_v1;
         o["mergeCollection_v1"] = mergeCollection_v1;
         o["setTicker_v1"] = setTicker_v1;
         o["sendChat_v1"] = sendChat_v1;
@@ -163,20 +164,33 @@ public class GameControlBackend
         o["getPlayers_v1"] = getPlayers_v1;
     }
 
-    public function setProperty_v2 (
-        propName :String, value :Object, index :int, testAndSet :Boolean) :void
+    public function setProperty_v1 (
+        propName :String, value :Object, index :int) :void
     {
         validatePropertyChange(propName, value, index);
 
         var encoded :Object = EZObjectMarshaller.encode(value, (index == -1));
         _ezObj.ezGameService.setProperty(
-            _ctx.getClient(), propName, encoded, index, testAndSet,
-            createLoggingConfirmListener("setProperty"));
+            _ctx.getClient(), propName, encoded, index,
+            false, null, createLoggingConfirmListener("setProperty"));
 
         // set it immediately in the game object
-        _ezObj.applyPropertySet(propName, value, index, testAndSet);
+        _ezObj.applyPropertySet(propName, value, index);
     }
 
+    public function testAndSetProperty_v1 (
+        propName :String, value :Object, testValue :Object, index :int) :void
+    {
+        validatePropertyChange(propName, value, index);
+
+        var encodedValue :Object = EZObjectMarshaller.encode(value, (index == -1));
+        var encodedTestValue :Object = EZObjectMarshaller.encode(testValue, (index == -1));
+        _ezObj.ezGameService.setProperty(
+            _ctx.getClient(), propName, encodedValue, index,
+            true, encodedTestValue, createLoggingConfirmListener("setProperty"));
+    }
+        
+
     public function mergeCollection_v1 (
         srcColl :String, intoColl :String) :void
     {

src/as/com/threerings/ezgame/client/GameData.as

@@ -67,7 +67,7 @@ public class GameData extends Proxy
 
     override flash_proxy function setProperty (propName :*, value :*) :void
     {
-        _propSetFn(String(propName), value, -1, false);
+        _propSetFn(String(propName), value, -1);
     }
 
     override flash_proxy function deleteProperty (propName :*) :Boolean

src/as/com/threerings/ezgame/data/EZGameMarshaller.as

@@ -178,12 +178,12 @@ public class EZGameMarshaller extends InvocationMarshaller
     public static const SET_PROPERTY :int = 11;
 
     // from interface EZGameService
-    public function setProperty (arg1 :Client, arg2 :String, arg3 :Object, arg4 :int, arg5 :Boolean, arg6 :InvocationService_InvocationListener) :void
+    public function setProperty (arg1 :Client, arg2 :String, arg3 :Object, arg4 :int, arg5 :Boolean, arg6 :Object, arg7 :InvocationService_InvocationListener) :void
     {
-        var listener6 :InvocationMarshaller_ListenerMarshaller = new InvocationMarshaller_ListenerMarshaller();
-        listener6.listener = arg6;
+        var listener7 :InvocationMarshaller_ListenerMarshaller = new InvocationMarshaller_ListenerMarshaller();
+        listener7.listener = arg7;
         sendRequest(arg1, SET_PROPERTY, [
-            arg2, arg3, Integer.valueOf(arg4), langBoolean.valueOf(arg5), listener6
+            arg2, arg3, Integer.valueOf(arg4), langBoolean.valueOf(arg5), arg6, listener7
         ]);
     }
 

src/as/com/threerings/ezgame/data/EZGameObject.as

@@ -98,30 +98,26 @@ public class EZGameObject extends GameObject
      * @return the old value
      */
     public function applyPropertySet (
-        propName :String, value :Object, index :int, testAndSet :Boolean)
-        :Object
+        propName :String, value :Object, index :int) :Object
     {
         var oldValue :Object = _props[propName];
-        if ((testAndSet && oldValue == null) || ! testAndSet)
-        {
-            if (index >= 0) {
-                // set an array element
-                var arr :Array = (oldValue as Array);
-                if (arr == null) {
-                    arr = [];
-                    _props[propName] = arr;
-                }
-                oldValue = arr[index];
-                arr[index] = value;
-                
-            } else if (value != null) {
-                // normal property set
-                _props[propName] = value;
-                
-            } else {
-                // remove a property
-                delete _props[propName];
+        if (index >= 0) {
+            // set an array element
+            var arr :Array = (oldValue as Array);
+            if (arr == null) {
+                arr = [];
+                _props[propName] = arr;
             }
+            oldValue = arr[index];
+            arr[index] = value;
+
+        } else if (value != null) {
+            // normal property set
+            _props[propName] = value;
+
+        } else {
+            // remove a property
+            delete _props[propName];
         }
         return oldValue;
     }

src/as/com/threerings/ezgame/data/PropertySetEvent.as

@@ -35,7 +35,7 @@ public class PropertySetEvent extends NamedEvent
     override public function applyToObject (target :DObject) :Boolean
     {
         _oldValue =
-            EZGameObject(target).applyPropertySet(_name, _data, _index, _testAndSet);
+            EZGameObject(target).applyPropertySet(_name, _data, _index);
         return true;
     }
 
@@ -69,7 +69,6 @@ public class PropertySetEvent extends NamedEvent
         super.readObject(ins);
         _index = ins.readInt();
         _data = EZObjectMarshaller.decode(ins.readObject());
-        _testAndSet = ins.readBoolean();
     }
 
     // from interface Streamable
@@ -78,7 +77,6 @@ public class PropertySetEvent extends NamedEvent
         super.writeObject(out);
         out.writeInt(_index);
         out.writeObject(_data);
-        out.writeBoolean(_testAndSet);
     }
 
     override protected function notifyListener (listener :Object) :void
@@ -101,9 +99,6 @@ public class PropertySetEvent extends NamedEvent
     /** The client-side data that is assigned to this property. */
     protected var _data :Object;
 
-    /** When true, the property will only be set if its previous value was null. */
-    protected var _testAndSet :Boolean;
-
     /** The old value. */
     protected var _oldValue :Object;
 }

src/java/com/threerings/ezgame/EZGame.java

@@ -28,14 +28,16 @@ public interface EZGame
     public void set (String propName, Object value, int index);
 
     /**
-     * Set a property that will be distributed. 
+     * Set a property that will be distributed, if the previous value
+     * matches the test value.
      */
-    public void set (String propName, Object value, boolean testAndSet);
+    public void testAndSet (String propName, Object value, Object testValue);
 
     /**
-     * Set a property that will be distributed. 
+     * Set a property that will be distributed, if the previous value
+     * matches the test value.
      */
-    public void set (String propName, Object value, int index, boolean testAndSet);
+    public void testAndSet (String propName, Object value, Object testValue, int index);
 
     /**
      * Register an object to receive whatever events it should receive,

src/java/com/threerings/ezgame/client/EZGameService.java

@@ -20,7 +20,7 @@ public interface EZGameService extends InvocationService
      */
     public void setProperty (
         Client client, String propName, Object value, int index,
-        boolean testAndSet, InvocationListener listener);
+        boolean testAndSet, Object testValue, InvocationListener listener);
 
     /**
      * Request to end the turn, possibly futzing the next turn holder unless

src/java/com/threerings/ezgame/client/GameObjectImpl.java

@@ -61,34 +61,40 @@ public class GameObjectImpl
     // from EZGame
     public void set (String propName, Object value)
     {
-        set(propName, value, -1, false);
-    }
-
-    // from EZGame
-    public void set (String propName, Object value, boolean testAndSet)
-    {
-        set(propName, value, -1, testAndSet);
+        set(propName, value, -1);
     }
 
     // from EZGame
     public void set (String propName, Object value, int index)
-    {
-        set(propName, value, index, false);
-    }
-    
-    // from EZGame
-    public void set (String propName, Object value, int index, boolean testAndSet)
     {
         validatePropertyChange(propName, value, -1);
 
         Object encoded = EZObjectMarshaller.encode(value);
         Object reconstituted = EZObjectMarshaller.decode(encoded);
         _ezObj.ezGameService.setProperty(
-            _ctx.getClient(), propName, encoded, index, testAndSet,
+            _ctx.getClient(), propName, encoded, index, false, null,
             createLoggingListener("setProperty"));
 
         // set it immediately in the game object
-        _ezObj.applyPropertySet(propName, reconstituted, index, testAndSet);
+        _ezObj.applyPropertySet(propName, reconstituted, index);
+    }
+    
+    // from EZGame
+    public void testAndSet (String propName, Object value, Object testValue)
+    {
+        testAndSet(propName, value, testValue, -1);
+    }
+
+    // from EZGame
+    public void testAndSet (
+        String propName, Object value, Object testValue, int index)
+    {
+        validatePropertyChange(propName, value, -1);
+
+        Object encoded = EZObjectMarshaller.encode(value);
+        _ezObj.ezGameService.setProperty(
+            _ctx.getClient(), propName, encoded, index, true, testValue,
+            createLoggingListener("testAndSet"));
     }
 
     // from EZGame

src/java/com/threerings/ezgame/data/EZGameMarshaller.java

@@ -171,12 +171,12 @@ public class EZGameMarshaller extends InvocationMarshaller
     public static final int SET_PROPERTY = 11;
 
     // from interface EZGameService
-    public void setProperty (Client arg1, String arg2, Object arg3, int arg4, boolean arg5, InvocationService.InvocationListener arg6)
+    public void setProperty (Client arg1, String arg2, Object arg3, int arg4, boolean arg5, Object arg6, InvocationService.InvocationListener arg7)
     {
-        ListenerMarshaller listener6 = new ListenerMarshaller();
-        listener6.listener = arg6;
+        ListenerMarshaller listener7 = new ListenerMarshaller();
+        listener7.listener = arg7;
         sendRequest(arg1, SET_PROPERTY, new Object[] {
-            arg2, arg3, Integer.valueOf(arg4), Boolean.valueOf(arg5), listener6
+            arg2, arg3, Integer.valueOf(arg4), Boolean.valueOf(arg5), arg6, listener7
         });
     }
 

src/java/com/threerings/ezgame/data/EZGameObject.java

@@ -5,6 +5,7 @@ package com.threerings.ezgame.data;
 
 import java.io.IOException;
 
+import java.util.Arrays;
 import java.util.HashMap;
 import java.util.Map;
 
@@ -85,55 +86,93 @@ public class EZGameObject extends GameObject
     /**
      * Called by PropertySetEvent to effect the property update.
      */
-    public Object applyPropertySet (
-        String propName, Object data, int index, boolean testAndSet)
+    public Object applyPropertySet (String propName, Object data, int index)
     {
         Object oldValue = _props.get(propName);
-        if ((testAndSet && oldValue == null) || ! testAndSet)
-        {
-            if (index >= 0) {
-                if (isOnServer()) {
-                    byte[][] arr = (oldValue instanceof byte[][])
-                        ? (byte[][]) oldValue : null;
-                    if (arr == null || arr.length <= index) {
-                        // TODO: in case a user sets element 0 and element 90000,
-                        // we might want to store elements in a hash
-                        byte[][] newArr = new byte[index + 1][];
-                        if (arr != null) {
-                            System.arraycopy(arr, 0, newArr, 0, arr.length);
-                        }
-                        _props.put(propName, newArr);
-                        arr = newArr;
+        if (index >= 0) {
+            if (isOnServer()) {
+                byte[][] arr = (oldValue instanceof byte[][])
+                    ? (byte[][]) oldValue : null;
+                if (arr == null || arr.length <= index) {
+                    // TODO: in case a user sets element 0 and element 90000,
+                    // we might want to store elements in a hash
+                    byte[][] newArr = new byte[index + 1][];
+                    if (arr != null) {
+                        System.arraycopy(arr, 0, newArr, 0, arr.length);
                     }
-                    oldValue = arr[index];
-                    arr[index] = (byte[]) data;
-
-                } else {
-                    Object[] arr = (oldValue instanceof Object[])
-                        ? (Object[]) oldValue : null;
-                    if (arr == null || arr.length <= index) {
-                        Object[] newArr = new Object[index + 1];
-                        if (arr != null) {
-                            System.arraycopy(arr, 0, newArr, 0, arr.length);
-                        }
-                        _props.put(propName, newArr);
-                        arr = newArr;
-                    }
-                    oldValue = arr[index];
-                    arr[index] = data;
+                    _props.put(propName, newArr);
+                    arr = newArr;
                 }
-
-            } else if (data != null) {
-                _props.put(propName, data);
-
+                oldValue = arr[index];
+                arr[index] = (byte[]) data;
+                
             } else {
-                _props.remove(propName);
+                Object[] arr = (oldValue instanceof Object[])
+                    ? (Object[]) oldValue : null;
+                if (arr == null || arr.length <= index) {
+                    Object[] newArr = new Object[index + 1];
+                    if (arr != null) {
+                        System.arraycopy(arr, 0, newArr, 0, arr.length);
+                    }
+                    _props.put(propName, newArr);
+                    arr = newArr;
+                }
+                oldValue = arr[index];
+                arr[index] = data;
             }
+            
+        } else if (data != null) {
+            _props.put(propName, data);
+        } else {
+            _props.remove(propName);
         }
-
+    
         return oldValue;
     }
 
+    /**
+     * Compares whether the old value and the test value are the same.
+     */
+    public boolean testProperty (
+        String propName, int index, boolean testAndSet, Object testValue)
+    {
+        boolean result = false;
+        if (! isOnServer() ||   // if this is the client, don't test - only test on server
+            ! testAndSet)       // test was not requested
+        {
+            result = true;
+          
+        } else {       
+            Object oldValue = _props.get(propName);
+            
+            // test if both are null
+            if (testValue == null || oldValue == null) {
+                result = (oldValue == testValue);
+                
+            } else {
+
+                // if the old value is an array, extract the appropriate element first
+                if (index >= 0 && oldValue instanceof byte[][])
+                {
+                    byte[][] arr = (byte[][]) oldValue;
+                    if (arr != null) { oldValue = arr[index]; }
+                }
+                
+                // now perform byte comparison
+                if (oldValue instanceof byte[] &&
+                    testValue instanceof byte[])
+                {
+                    result = Arrays.equals (
+                        (byte[]) oldValue, (byte[]) testValue);
+                }
+            }
+        }
+        
+        return result;
+    }
+                
+        
+
     // AUTO-GENERATED: METHODS START
     /**
      * Requests that the <code>turnHolder</code> field be set to the

src/java/com/threerings/ezgame/data/PropertySetEvent.java

@@ -30,12 +30,11 @@ public class PropertySetEvent extends NamedEvent
      * Create a PropertySetEvent.
      */
     public PropertySetEvent (
-        int targetOid, String propName, Object value, int index, boolean testAndSet)
+        int targetOid, String propName, Object value, int index)
     {
         super(targetOid, propName);
         _data = value;
         _index = index;
-        _testAndSet = testAndSet;
     }
 
     /**
@@ -69,7 +68,7 @@ public class PropertySetEvent extends NamedEvent
         if (!ezObj.isOnServer()) {
             _data = EZObjectMarshaller.decode(_data);
         }
-        _oldValue = ezObj.applyPropertySet(_name, _data, _index, _testAndSet);
+        _oldValue = ezObj.applyPropertySet(_name, _data, _index);
         return true;
     }
 
@@ -95,9 +94,6 @@ public class PropertySetEvent extends NamedEvent
     /** The client-side data that is assigned to this property. */
     protected Object _data;
 
-    /** When true, the property will only be set if the old value was null. */
-    protected boolean _testAndSet;
-    
     /** The old value. */
     protected transient Object _oldValue;
 }

src/java/com/threerings/ezgame/server/EZGameDispatcher.java

@@ -129,7 +129,7 @@ public class EZGameDispatcher extends InvocationDispatcher
         case EZGameMarshaller.SET_PROPERTY:
             ((EZGameProvider)provider).setProperty(
                 source,
-                (String)args[0], (Object)args[1], ((Integer)args[2]).intValue(), ((Boolean)args[3]).booleanValue(), (InvocationService.InvocationListener)args[4]
+                (String)args[0], (Object)args[1], ((Integer)args[2]).intValue(), ((Boolean)args[3]).booleanValue(), (Object)args[4], (InvocationService.InvocationListener)args[5]
             );
             return;
 

src/java/com/threerings/ezgame/server/EZGameManager.java

@@ -127,11 +127,12 @@ public class EZGameManager extends GameManager
     // from EZGameProvider
     public void setProperty (
         ClientObject caller, String propName, Object data, int index,
-        boolean testAndSet, InvocationService.InvocationListener listener)
+        boolean testAndSet, Object testValue,
+        InvocationService.InvocationListener listener)
         throws InvocationException
     {
         validateUser(caller);
-        setProperty(propName, data, index, testAndSet);
+        setProperty(propName, data, index, testAndSet, testValue);
     }
 
     // from EZGameProvider
@@ -220,7 +221,7 @@ public class EZGameManager extends GameManager
                 }
 
                 if (playerId == 0) {
-                    setProperty(msgOrPropName, result, -1, false);
+                    setProperty(msgOrPropName, result, -1, false, null);
                     
                 } else {
                     sendPrivateMessage(playerId, msgOrPropName, result);
@@ -401,11 +402,15 @@ public class EZGameManager extends GameManager
      * Helper method to post a property set event.
      */
     protected void setProperty (
-        String propName, Object value, int index, boolean testAndSet)
+        String propName, Object value, int index,
+        boolean testAndSet, Object testValue)
     {
-        _gameObj.postEvent(
-            new PropertySetEvent(
-                _gameObj.getOid(), propName, value, index, testAndSet));
+        if (_gameObj.testProperty (propName, index, testAndSet, testValue))
+        {
+            _gameObj.postEvent(
+                new PropertySetEvent(
+                    _gameObj.getOid(), propName, value, index));
+        }
     }
 
     /**

src/java/com/threerings/ezgame/server/EZGameProvider.java

@@ -96,7 +96,7 @@ public interface EZGameProvider extends InvocationProvider
     /**
      * Handles a {@link EZGameService#setProperty} request.
      */
-    public void setProperty (ClientObject caller, String arg1, Object arg2, int arg3, boolean arg4, InvocationService.InvocationListener arg5)
+    public void setProperty (ClientObject caller, String arg1, Object arg2, int arg3, boolean arg4, Object arg5, InvocationService.InvocationListener arg6)
         throws InvocationException;
 
     /**