Merge pull request #195 from Unity-Technologies/ordered-tag-queries

Fixed issues with determinism in Unity Simulation

Author: sleal-unity

com.unity.perception/CHANGELOG.md

@@ -13,15 +13,25 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
 ### Added
 
+Added Register() and Unregister() methods to the RandomizerTag API so users can implement RandomizerTag compatible GameObject caching
+
 ### Changed
 
+Switched accessibility of scenario MonoBehaviour lifecycle functions (Awake, Start, Update) from private to protected to enable users to define their own overrides when deriving the Scenario class.
+
+The GameObjectOneWayCache has been made public for users to cache GameObjects within their own custom Randomizers.
+
 ### Deprecated
 
 ### Removed
 
 ### Fixed
 
-Fixed the math offsetting the iteration index of each Unity Simulation instance directly after they deserialize their app-params
+Fixed the math offsetting the iteration index of each Unity Simulation instance directly after they deserialize their app-params.
+
+The RandomizerTagManager now uses a LinkedHashSet data structure to register tags to preserve insertion order determinism in Unity Simulation.
+
+GameObjectOneWayCache now correctly registers and unregisters RandomizerTags on cached GameObjects.
 
 ## [0.7.0-preview.1] - 2021-02-01
 

com.unity.perception/Runtime/Randomization/Randomizers/LinkedHashSet.cs

@@ -0,0 +1,77 @@
+using System.Collections;
+using System.Collections.Generic;
+
+namespace UnityEngine.Perception.Randomization.Randomizers
+{
+    /// <summary>
+    /// This collection has the properties of a HashSet that also preserves insertion order. As such, this data
+    /// structure demonstrates the following time complexities:
+    /// O(1) lookup, O(1) insertion, O(1) removal, and O(n) traversal
+    /// </summary>
+    /// <typeparam name="T">The item type to store in this collection</typeparam>
+    class LinkedHashSet<T> : ICollection<T>
+    {
+        readonly IDictionary<T, LinkedListNode<T>> m_Dictionary;
+        readonly LinkedList<T> m_LinkedList;
+
+        public LinkedHashSet() : this(EqualityComparer<T>.Default) { }
+
+        public LinkedHashSet(IEqualityComparer<T> comparer)
+        {
+            m_Dictionary = new Dictionary<T, LinkedListNode<T>>(comparer);
+            m_LinkedList = new LinkedList<T>();
+        }
+
+        public int Count => m_Dictionary.Count;
+
+        public bool IsReadOnly => m_Dictionary.IsReadOnly;
+
+        void ICollection<T>.Add(T item)
+        {
+            Add(item);
+        }
+
+        public bool Add(T item)
+        {
+            if (m_Dictionary.ContainsKey(item)) return false;
+            var node = m_LinkedList.AddLast(item);
+            m_Dictionary.Add(item, node);
+            return true;
+        }
+
+        public void Clear()
+        {
+            m_LinkedList.Clear();
+            m_Dictionary.Clear();
+        }
+
+        public bool Remove(T item)
+        {
+            var found = m_Dictionary.TryGetValue(item, out var node);
+            if (!found) return false;
+            m_Dictionary.Remove(item);
+            m_LinkedList.Remove(node);
+            return true;
+        }
+
+        public IEnumerator<T> GetEnumerator()
+        {
+            return m_LinkedList.GetEnumerator();
+        }
+
+        IEnumerator IEnumerable.GetEnumerator()
+        {
+            return GetEnumerator();
+        }
+
+        public bool Contains(T item)
+        {
+            return m_Dictionary.ContainsKey(item);
+        }
+
+        public void CopyTo(T[] array, int arrayIndex)
+        {
+            m_LinkedList.CopyTo(array, arrayIndex);
+        }
+    }
+}

com.unity.perception/Runtime/Randomization/Randomizers/LinkedHashSet.cs.meta

@@ -9,60 +9,76 @@ namespace UnityEngine.Perception.Randomization.Randomizers.Utilities
     /// Facilitates object pooling for a pre-specified collection of prefabs with the caveat that objects can be fetched
     /// from the cache but not returned. Every frame, the cache needs to be reset, which will return all objects to the pool
     /// </summary>
-    class GameObjectOneWayCache
+    public class GameObjectOneWayCache
     {
         static ProfilerMarker s_ResetAllObjectsMarker = new ProfilerMarker("ResetAllObjects");
 
-        // Objects will reset to this origin when not being used
         Transform m_CacheParent;
         Dictionary<int, int> m_InstanceIdToIndex;
-        List<GameObject>[] m_InstantiatedObjects;
+        List<CachedObjectData>[] m_InstantiatedObjects;
         int[] m_NumObjectsActive;
         int NumObjectsInCache { get; set; }
+
+        /// <summary>
+        /// The number of active cache objects in the scene
+        /// </summary>
         public int NumObjectsActive { get; private set; }
 
+        /// <summary>
+        /// Creates a new GameObjectOneWayCache
+        /// </summary>
+        /// <param name="parent">The parent object all cached instances will be parented under</param>
+        /// <param name="prefabs">The prefabs to cache</param>
         public GameObjectOneWayCache(Transform parent, GameObject[] prefabs)
         {
             m_CacheParent = parent;
             m_InstanceIdToIndex = new Dictionary<int, int>();
-            m_InstantiatedObjects = new List<GameObject>[prefabs.Length];
+            m_InstantiatedObjects = new List<CachedObjectData>[prefabs.Length];
             m_NumObjectsActive = new int[prefabs.Length];
 
             var index = 0;
             foreach (var prefab in prefabs)
             {
                 var instanceId = prefab.GetInstanceID();
                 m_InstanceIdToIndex.Add(instanceId, index);
-                m_InstantiatedObjects[index] = new List<GameObject>();
+                m_InstantiatedObjects[index] = new List<CachedObjectData>();
                 m_NumObjectsActive[index] = 0;
                 ++index;
             }
         }
 
+        /// <summary>
+        /// Retrieves an existing instance of the given prefab from the cache if available.
+        /// Otherwise, instantiate a new instance of the given prefab.
+        /// </summary>
+        /// <param name="prefab"></param>
+        /// <returns></returns>
+        /// <exception cref="ArgumentException"></exception>
         public GameObject GetOrInstantiate(GameObject prefab)
         {
             if (!m_InstanceIdToIndex.TryGetValue(prefab.GetInstanceID(), out var index))
-            {
                 throw new ArgumentException($"Prefab {prefab.name} (ID: {prefab.GetInstanceID()}) is not in cache.");
-            }
 
             ++NumObjectsActive;
             if (m_NumObjectsActive[index] < m_InstantiatedObjects[index].Count)
             {
                 var nextInCache = m_InstantiatedObjects[index][m_NumObjectsActive[index]];
                 ++m_NumObjectsActive[index];
-                return nextInCache;
-            }
-            else
-            {
-                ++NumObjectsInCache;
-                var newObject = Object.Instantiate(prefab, m_CacheParent);
-                ++m_NumObjectsActive[index];
-                m_InstantiatedObjects[index].Add(newObject);
-                return newObject;
+                foreach (var tag in nextInCache.randomizerTags)
+                    tag.Register();
+                return nextInCache.instance;
             }
+
+            ++NumObjectsInCache;
+            var newObject = Object.Instantiate(prefab, m_CacheParent);
+            ++m_NumObjectsActive[index];
+            m_InstantiatedObjects[index].Add(new CachedObjectData(newObject));
+            return newObject;
         }
 
+        /// <summary>
+        /// Return all active cache objects back to an inactive state
+        /// </summary>
         public void ResetAllObjects()
         {
             using (s_ResetAllObjectsMarker.Auto())
@@ -71,13 +87,27 @@ public void ResetAllObjects()
                 for (var i = 0; i < m_InstantiatedObjects.Length; ++i)
                 {
                     m_NumObjectsActive[i] = 0;
-                    foreach (var obj in m_InstantiatedObjects[i])
+                    foreach (var cachedObjectData in m_InstantiatedObjects[i])
                     {
                         // Position outside the frame
-                        obj.transform.localPosition = new Vector3(10000, 0, 0);
+                        cachedObjectData.instance.transform.localPosition = new Vector3(10000, 0, 0);
+                        foreach (var tag in cachedObjectData.randomizerTags)
+                            tag.Unregister();
                     }
                 }
             }
         }
+
+        struct CachedObjectData
+        {
+            public GameObject instance;
+            public RandomizerTag[] randomizerTags;
+
+            public CachedObjectData(GameObject instance)
+            {
+                this.instance = instance;
+                randomizerTags = instance.GetComponents<RandomizerTag>();
+            }
+        }
     }
 }

com.unity.perception/Runtime/Randomization/Randomizers/RandomizerExamples/Utilities/GameObjectOneWayCache.cs

@@ -11,12 +11,34 @@ public abstract class RandomizerTag : MonoBehaviour
     {
         RandomizerTagManager tagManager => RandomizerTagManager.singleton;
 
-        void Awake()
+        /// <summary>
+        /// Awake is called when this RandomizerTag is created or instantiated
+        /// </summary>
+        protected virtual void Awake()
+        {
+            Register();
+        }
+
+        /// <summary>
+        /// OnDestroy is called when this RandomizerTag is destroyed
+        /// </summary>
+        protected virtual void OnDestroy()
+        {
+            Unregister();
+        }
+
+        /// <summary>
+        /// Registers this tag with the tagManager
+        /// </summary>
+        public void Register()
         {
             tagManager.AddTag(this);
         }
 
-        void OnDestroy()
+        /// <summary>
+        /// Unregisters this tag with the tagManager
+        /// </summary>
+        public void Unregister()
         {
             tagManager.RemoveTag(this);
         }

com.unity.perception/Runtime/Randomization/Randomizers/RandomizerTag.cs

@@ -14,7 +14,7 @@ public class RandomizerTagManager
         public static RandomizerTagManager singleton { get; } = new RandomizerTagManager();
 
         Dictionary<Type, HashSet<Type>> m_TypeTree = new Dictionary<Type, HashSet<Type>>();
-        Dictionary<Type, HashSet<RandomizerTag>> m_TagMap = new Dictionary<Type, HashSet<RandomizerTag>>();
+        Dictionary<Type, LinkedHashSet<RandomizerTag>> m_TagMap = new Dictionary<Type, LinkedHashSet<RandomizerTag>>();
 
         /// <summary>
         /// Enumerates over all RandomizerTags of the given type present in the scene
@@ -60,15 +60,15 @@ void AddTagTypeToTypeHierarchy(Type tagType)
             if (m_TypeTree.ContainsKey(tagType))
                 return;
 
-            m_TagMap.Add(tagType, new HashSet<RandomizerTag>());
+            m_TagMap.Add(tagType, new LinkedHashSet<RandomizerTag>());
             m_TypeTree.Add(tagType, new HashSet<Type>());
 
             var baseType = tagType.BaseType;
             while (baseType != null && baseType != typeof(RandomizerTag))
             {
                 if (!m_TypeTree.ContainsKey(baseType))
                 {
-                    m_TagMap.Add(baseType, new HashSet<RandomizerTag>());
+                    m_TagMap.Add(baseType, new LinkedHashSet<RandomizerTag>());
                     m_TypeTree[baseType] = new HashSet<Type> { tagType };
                 }
                 else
@@ -84,7 +84,7 @@ void AddTagTypeToTypeHierarchy(Type tagType)
 
         internal void RemoveTag<T>(T tag) where T : RandomizerTag
         {
-            var tagType = typeof(T);
+            var tagType = tag.GetType();
             if (m_TagMap.ContainsKey(tagType) && m_TagMap[tagType].Contains(tag))
                 m_TagMap[tagType].Remove(tag);
         }