Sfoglia il codice sorgente

Added unit test for binarx diff (untested)

Marko Pintera 10 anni fa
parent
commit
bdcfc5302f

+ 3 - 1
BansheeEditor/Include/BsEditorPrerequisites.h

@@ -108,6 +108,8 @@ namespace BansheeEngine
 		TID_TestComponentB = 40009,
 		TID_TestComponentB = 40009,
 		TID_PlatformInfo = 40010,
 		TID_PlatformInfo = 40010,
 		TID_WinPlatformInfo = 40011,
 		TID_WinPlatformInfo = 40011,
-		TID_BuildData = 40012
+		TID_BuildData = 40012,
+		TID_TestObjectA = 40013,
+		TID_TestObjectB = 40014
 	};
 	};
 }
 }

+ 1 - 0
BansheeEditor/Include/BsEditorTestSuite.h

@@ -67,5 +67,6 @@ namespace BansheeEngine
 
 
 	private:
 	private:
 		void SceneObjectRecord_UndoRedo();
 		void SceneObjectRecord_UndoRedo();
+		void BinaryDiff();
 	};
 	};
 }
 }

+ 259 - 1
BansheeEditor/Source/BsEditorTestSuite.cpp

@@ -4,6 +4,9 @@
 #include "BsUndoRedo.h"
 #include "BsUndoRedo.h"
 #include "BsRTTIType.h"
 #include "BsRTTIType.h"
 #include "BsGameObjectRTTI.h"
 #include "BsGameObjectRTTI.h"
+#include "BsBinarySerializer.h"
+#include "BsMemorySerializer.h"
+#include "BsBinaryDiff.h"
 
 
 namespace BansheeEngine
 namespace BansheeEngine
 {
 {
@@ -101,9 +104,188 @@ namespace BansheeEngine
 		return TestComponentB::getRTTIStatic();
 		return TestComponentB::getRTTIStatic();
 	}
 	}
 
 
+	struct TestObjectB : IReflectable
+	{
+		UINT32 intA = 100;
+		String strA = "100";
+
+		/************************************************************************/
+		/* 								RTTI		                     		*/
+		/************************************************************************/
+	public:
+		friend class TestObjectBRTTI;
+		static RTTITypeBase* getRTTIStatic();
+		virtual RTTITypeBase* getRTTI() const;
+	};
+
+	struct TestObjectA : IReflectable
+	{
+		TestObjectA()
+		{
+			arrStrA = { "10", "11", "12" };
+			arrStrB = { "13", "14", "15" };
+			arrStrC = { "16", "17", "18" };
+
+			arrObjA = { TestObjectB(), TestObjectB(), TestObjectB() };
+			arrObjB = { TestObjectB(), TestObjectB(), TestObjectB() };
+
+			arrObjPtrA = { bs_shared_ptr<TestObjectB>(), bs_shared_ptr<TestObjectB>(), bs_shared_ptr<TestObjectB>() };
+			arrObjPtrB = { bs_shared_ptr<TestObjectB>(), bs_shared_ptr<TestObjectB>(), bs_shared_ptr<TestObjectB>() };
+		}
+
+		UINT32 intA = 5;
+		String strA = "5";
+		String strB = "7";
+
+		TestObjectB objA;
+		TestObjectB objB;
+
+		SPtr<TestObjectB> objPtrA = bs_shared_ptr<TestObjectB>();
+		SPtr<TestObjectB> objPtrB = bs_shared_ptr<TestObjectB>();
+		SPtr<TestObjectB> objPtrC = bs_shared_ptr<TestObjectB>();
+		SPtr<TestObjectB> objPtrD = nullptr;
+
+		Vector<String> arrStrA;
+		Vector<String> arrStrB;
+		Vector<String> arrStrC;
+
+		Vector<TestObjectB> arrObjA;
+		Vector<TestObjectB> arrObjB;
+
+		Vector<SPtr<TestObjectB>> arrObjPtrA;
+		Vector<SPtr<TestObjectB>> arrObjPtrB;
+
+		/************************************************************************/
+		/* 								RTTI		                     		*/
+		/************************************************************************/
+	public:
+		friend class TestObjectARTTI;
+		static RTTITypeBase* getRTTIStatic();
+		virtual RTTITypeBase* getRTTI() const;
+	};
+
+	class TestObjectARTTI : public RTTIType < TestObjectA, IReflectable, TestObjectARTTI >
+	{
+	private:
+		BS_PLAIN_MEMBER(intA);
+		BS_PLAIN_MEMBER(strA);
+		BS_PLAIN_MEMBER(strB);
+
+		BS_REFL_MEMBER(objA);
+		BS_REFL_MEMBER(objB);
+
+		BS_REFLPTR_MEMBER(objPtrA);
+		BS_REFLPTR_MEMBER(objPtrB);
+		BS_REFLPTR_MEMBER(objPtrC);
+		BS_REFLPTR_MEMBER(objPtrD);
+
+		BS_PLAIN_MEMBER_VEC(arrStrA);
+		BS_PLAIN_MEMBER_VEC(arrStrB);
+		BS_PLAIN_MEMBER_VEC(arrStrC);
+
+		BS_REFL_MEMBER_VEC(arrObjA);
+		BS_REFL_MEMBER_VEC(arrObjB);
+
+		BS_REFLPTR_MEMBER_VEC(arrObjPtrA);
+		BS_REFLPTR_MEMBER_VEC(arrObjPtrB);
+
+	public:
+		TestObjectARTTI()
+		{
+			BS_ADD_PLAIN_FIELD(intA, 0);
+			BS_ADD_PLAIN_FIELD(strA, 1);
+			BS_ADD_PLAIN_FIELD(strB, 2);
+
+			BS_ADD_REFL_FIELD(objA, 3);
+			BS_ADD_REFL_FIELD(objB, 4);
+
+			BS_ADD_REFLPTR_FIELD(objPtrA, 5);
+			BS_ADD_REFLPTR_FIELD(objPtrB, 6);
+			BS_ADD_REFLPTR_FIELD(objPtrC, 7);
+			BS_ADD_REFLPTR_FIELD(objPtrD, 8);
+
+			BS_ADD_PLAIN_FIELD_ARR(arrStrA, 9);
+			BS_ADD_PLAIN_FIELD_ARR(arrStrB, 10);
+			BS_ADD_PLAIN_FIELD_ARR(arrStrC, 11);
+
+			BS_ADD_REFL_FIELD_ARR(arrObjA, 12);
+			BS_ADD_REFL_FIELD_ARR(arrObjB, 13);
+
+			BS_ADD_REFLPTR_FIELD_ARR(arrObjPtrA, 14);
+			BS_ADD_REFLPTR_FIELD_ARR(arrObjPtrB, 15);
+		}
+
+		virtual const String& getRTTIName()
+		{
+			static String name = "TestObjectA";
+			return name;
+		}
+
+		virtual UINT32 getRTTIId()
+		{
+			return TID_TestObjectA;
+		}
+
+		virtual std::shared_ptr<IReflectable> newRTTIObject()
+		{
+			return bs_shared_ptr<TestObjectA>();
+		}
+	};
+
+	class TestObjectBRTTI : public RTTIType < TestObjectB, IReflectable, TestObjectBRTTI >
+	{
+	private:
+		BS_PLAIN_MEMBER(intA);
+		BS_PLAIN_MEMBER(strA);
+
+	public:
+		TestObjectBRTTI()
+		{
+			BS_ADD_PLAIN_FIELD(intA, 0);
+			BS_ADD_PLAIN_FIELD(strA, 1);
+		}
+
+		virtual const String& getRTTIName()
+		{
+			static String name = "TestObjectB";
+			return name;
+		}
+
+		virtual UINT32 getRTTIId()
+		{
+			return TID_TestObjectB;
+		}
+
+		virtual std::shared_ptr<IReflectable> newRTTIObject()
+		{
+			return bs_shared_ptr<TestObjectB>();
+		}
+	};
+
+	RTTITypeBase* TestObjectB::getRTTIStatic()
+	{
+		return TestObjectBRTTI::instance();
+	}
+
+	RTTITypeBase* TestObjectB::getRTTI() const
+	{
+		return TestObjectB::getRTTIStatic();
+	}
+
+	RTTITypeBase* TestObjectA::getRTTIStatic()
+	{
+		return TestObjectARTTI::instance();
+	}
+
+	RTTITypeBase* TestObjectA::getRTTI() const
+	{
+		return TestObjectA::getRTTIStatic();
+	}
+
 	EditorTestSuite::EditorTestSuite()
 	EditorTestSuite::EditorTestSuite()
 	{
 	{
-		BS_ADD_TEST(EditorTestSuite::SceneObjectRecord_UndoRedo)
+		BS_ADD_TEST(EditorTestSuite::SceneObjectRecord_UndoRedo);
+		BS_ADD_TEST(EditorTestSuite::BinaryDiff);
 	}
 	}
 
 
 	void EditorTestSuite::SceneObjectRecord_UndoRedo()
 	void EditorTestSuite::SceneObjectRecord_UndoRedo()
@@ -147,4 +329,80 @@ namespace BansheeEngine
 		BS_TEST_ASSERT(!cmpExternal->ref2.isDestroyed());
 		BS_TEST_ASSERT(!cmpExternal->ref2.isDestroyed());
 		BS_TEST_ASSERT(cmpB1_1->val1 == "InitialValue");
 		BS_TEST_ASSERT(cmpB1_1->val1 == "InitialValue");
 	}
 	}
+
+	void EditorTestSuite::BinaryDiff()
+	{
+		SPtr<TestObjectA> orgObj = bs_shared_ptr<TestObjectA>();
+		SPtr<TestObjectA> newObj = bs_shared_ptr<TestObjectA>();
+
+		newObj->intA = 995;
+		newObj->strA = "potato";
+		newObj->arrStrB = { "orange", "carrot" };
+		newObj->arrStrC[2] = "banana";
+		newObj->objB.intA = 9940;
+		newObj->objPtrB->strA = "kiwi";
+		newObj->objPtrC = nullptr;
+		newObj->objPtrD = bs_shared_ptr<TestObjectB>();
+		newObj->arrObjB[1].strA = "starberry";
+		newObj->arrObjPtrB[0]->intA = 99100;
+
+		MemorySerializer ms;
+		UINT32 orgDataLength = 0;
+		UINT8* orgData = ms.encode(orgObj.get(), orgDataLength, &bs_alloc);
+
+		UINT32 newDataLength = 0;
+		UINT8* newData = ms.encode(newObj.get(), newDataLength, &bs_alloc);
+
+		UINT32 dummy = 0;
+		BinarySerializer bs;
+		SPtr<SerializedObject> orgSerialized = bs._decodeIntermediate(orgData, orgDataLength, dummy);
+		SPtr<SerializedObject> newSerialized = bs._decodeIntermediate(newData, newDataLength, dummy);
+
+		SPtr<SerializedObject> objDiff = BinaryDiff::generateDiff(orgSerialized, newSerialized);
+		BinaryDiff::applyDiff(orgObj, objDiff);
+
+		bs_free(orgData);
+		bs_free(newData);
+
+		BS_TEST_ASSERT(orgObj->intA == newObj->intA);
+		BS_TEST_ASSERT(orgObj->strA == newObj->strA);
+		BS_TEST_ASSERT(orgObj->strB == newObj->strB);
+
+		BS_TEST_ASSERT(orgObj->objA.intA == newObj->objA.intA);
+		BS_TEST_ASSERT(orgObj->objB.intA == newObj->objB.intA);
+		
+		BS_TEST_ASSERT(orgObj->objPtrA->strA == newObj->objPtrA->strA);
+		BS_TEST_ASSERT(orgObj->objPtrB->strA == newObj->objPtrB->strA);
+		BS_TEST_ASSERT(orgObj->objPtrD->strA == newObj->objPtrD->strA);
+		BS_TEST_ASSERT(orgObj->objPtrC == newObj->objPtrC);
+
+		BS_TEST_ASSERT(orgObj->arrStrA.size() == newObj->arrStrA.size());
+		for (UINT32 i = 0; i < (UINT32)orgObj->arrStrA.size(); i++)
+			BS_TEST_ASSERT(orgObj->arrStrA[i] == newObj->arrStrA[i]);
+
+		BS_TEST_ASSERT(orgObj->arrStrB.size() == newObj->arrStrB.size());
+		for (UINT32 i = 0; i < (UINT32)orgObj->arrStrB.size(); i++)
+			BS_TEST_ASSERT(orgObj->arrStrB[i] == newObj->arrStrB[i]);
+
+		BS_TEST_ASSERT(orgObj->arrStrC.size() == newObj->arrStrC.size());
+		for (UINT32 i = 0; i < (UINT32)orgObj->arrStrC.size(); i++)
+			BS_TEST_ASSERT(orgObj->arrStrC[i] == newObj->arrStrC[i]);
+
+		BS_TEST_ASSERT(orgObj->arrObjA.size() == newObj->arrObjA.size());
+		for (UINT32 i = 0; i < (UINT32)orgObj->arrObjA.size(); i++)
+			BS_TEST_ASSERT(orgObj->arrObjA[i].strA == newObj->arrObjA[i].strA);
+
+		BS_TEST_ASSERT(orgObj->arrObjB.size() == newObj->arrObjB.size());
+		for (UINT32 i = 0; i < (UINT32)orgObj->arrObjB.size(); i++)
+			BS_TEST_ASSERT(orgObj->arrObjB[i].strA == newObj->arrObjB[i].strA);
+
+		BS_TEST_ASSERT(orgObj->arrObjPtrA.size() == newObj->arrObjPtrA.size());
+		for (UINT32 i = 0; i < (UINT32)orgObj->arrObjPtrA.size(); i++)
+			BS_TEST_ASSERT(orgObj->arrObjPtrA[i]->intA == newObj->arrObjPtrA[i]->intA);
+
+		BS_TEST_ASSERT(orgObj->arrObjPtrB.size() == newObj->arrObjPtrB.size());
+		for (UINT32 i = 0; i < (UINT32)orgObj->arrObjPtrB.size(); i++)
+			BS_TEST_ASSERT(orgObj->arrObjPtrB[i]->intA == newObj->arrObjPtrB[i]->intA);
+	}
+
 }
 }

+ 51 - 1
BansheeUtility/Include/BsBinaryDiff.h

@@ -5,15 +5,38 @@
 
 
 namespace BansheeEngine
 namespace BansheeEngine
 {
 {
+	/**
+	 * @brief	Generates and applies "diffs". Diffs contain per-field differences between
+	 *			an original and new object. These differences can be saved and then applied
+	 *			to an original object to transform it to the new version.
+	 *
+	 * @note	Objects must be in intermediate serialized format generated by BinarySerializer.
+	 */
 	class BS_UTILITY_EXPORT BinaryDiff
 	class BS_UTILITY_EXPORT BinaryDiff
 	{
 	{
 		typedef UnorderedMap<SPtr<SerializedObject>, SPtr<SerializedObject>> ObjectMap;
 		typedef UnorderedMap<SPtr<SerializedObject>, SPtr<SerializedObject>> ObjectMap;
 		typedef UnorderedMap<SPtr<SerializedObject>, SPtr<IReflectable>> DiffObjectMap;
 		typedef UnorderedMap<SPtr<SerializedObject>, SPtr<IReflectable>> DiffObjectMap;
 	public:
 	public:
+		/**
+		 * @brief	Generates per-field differences between the provided original and new object. Any field
+		 *			or array entry that is different in the new object compared to the original will be output
+		 *			in the resulting object, with a full hierarchy of that field.
+		 *
+		 *			Will return null if there is no difference.
+		 */
 		static SPtr<SerializedObject> generateDiff(const SPtr<SerializedObject>& orgObj, const SPtr<SerializedObject>& newObj);
 		static SPtr<SerializedObject> generateDiff(const SPtr<SerializedObject>& orgObj, const SPtr<SerializedObject>& newObj);
+
+		/**
+		 * @brief	Applies a previously generated per-field differences to the provided object. This will
+		 *			essentially transform the original object the differences were generated for into the modified
+		 *			version.
+		 */
 		static void applyDiff(const SPtr<IReflectable>& object, const SPtr<SerializedObject>& diff);
 		static void applyDiff(const SPtr<IReflectable>& object, const SPtr<SerializedObject>& diff);
 
 
 	private:
 	private:
+		/**
+		 * @brief	Types of commands that are used when applying difference field values.
+		 */
 		enum DiffCommandType
 		enum DiffCommandType
 		{
 		{
 			Diff_Plain = 0x01,
 			Diff_Plain = 0x01,
@@ -25,7 +48,11 @@ namespace BansheeEngine
 			Diff_ObjectEnd = 0x07,
 			Diff_ObjectEnd = 0x07,
 			Diff_ArrayFlag = 0x10
 			Diff_ArrayFlag = 0x10
 		};
 		};
-
+		
+		/**
+		 * @brief	A command that is used for delaying writing to an object, it contains
+		 *			all necessary information for setting RTTI field values on an object.
+		 */
 		struct DiffCommand
 		struct DiffCommand
 		{
 		{
 			RTTIField* field;
 			RTTIField* field;
@@ -41,11 +68,34 @@ namespace BansheeEngine
 			};
 			};
 		};
 		};
 
 
+		/**
+		 * @brief	Recursive version of generateDiff(const SPtr<SerializedObject>&, const SPtr<SerializedObject>&).
+		 *
+		 * @see		generateDiff(const SPtr<SerializedObject>&, const SPtr<SerializedObject>&)
+		 */
 		static SPtr<SerializedObject> generateDiff(const SPtr<SerializedObject>& orgObj, const SPtr<SerializedObject>& newObj, ObjectMap& objectMap);
 		static SPtr<SerializedObject> generateDiff(const SPtr<SerializedObject>& orgObj, const SPtr<SerializedObject>& newObj, ObjectMap& objectMap);
+
+		/**
+		 * @brief	Generates a difference between data of a specific field type indiscriminately of the
+		 *			specific field type.
+		 *
+		 * @see		generateDiff(const SPtr<SerializedObject>&, const SPtr<SerializedObject>&)
+		 */
 		static SPtr<SerializedInstance> generateDiff(UINT32 fieldType, const SPtr<SerializedInstance>& orgData, 
 		static SPtr<SerializedInstance> generateDiff(UINT32 fieldType, const SPtr<SerializedInstance>& orgData, 
 			const SPtr<SerializedInstance>& newData, ObjectMap& objectMap);
 			const SPtr<SerializedInstance>& newData, ObjectMap& objectMap);
 
 
+		/**
+		 * @brief	Recursive version of applyDiff(const SPtr<IReflectable>& object, const SPtr<SerializedObject>& diff).
+		 *			Outputs a set of commands that then must be executed in order to actually apply the difference to the
+		 *			provided object.
+		 *
+		 * @see		applyDiff(const SPtr<IReflectable>& object, const SPtr<SerializedObject>& diff)
+		 */
 		static void applyDiff(const SPtr<IReflectable>& object, const SPtr<SerializedObject>& diff, DiffObjectMap& objectMap, Vector<DiffCommand>& diffCommands);
 		static void applyDiff(const SPtr<IReflectable>& object, const SPtr<SerializedObject>& diff, DiffObjectMap& objectMap, Vector<DiffCommand>& diffCommands);
+
+		/**
+		 * @brief	Helper method that clones any object with RTTI implemented.
+		 */
 		static SPtr<IReflectable> clone(IReflectable* object);
 		static SPtr<IReflectable> clone(IReflectable* object);
 	};
 	};
 }
 }

+ 52 - 5
BansheeUtility/Include/BsRTTIType.h

@@ -15,12 +15,56 @@
 
 
 namespace BansheeEngine
 namespace BansheeEngine
 {
 {
-#define BS_SETGET_MEMBER(name, type, parentType)								\
-	type##& get##name(parentType##* obj) { return obj->##name; }				\
-	void Set##name(parentType##* obj, type##& val) { obj->##name = val; } 
+#define BS_PLAIN_MEMBER(name)								\
+	decltype(OwnerType::##name)& get##name(OwnerType* obj) { return obj->##name; }				\
+	void set##name(OwnerType* obj, decltype(OwnerType::##name)& val) { obj->##name = val; } 
 
 
-#define BS_ADD_PLAINFIELD(name, id, parentType) \
-	addPlainField(#name, id##, &##parentType##::get##name, &##parentType##::Set##name);
+#define BS_REFL_MEMBER(name)								\
+	decltype(OwnerType::##name)& get##name(OwnerType* obj) { return obj->##name; }				\
+	void set##name(OwnerType* obj, decltype(OwnerType::##name)& val) { obj->##name = val; } 
+
+#define BS_REFLPTR_MEMBER(name)								\
+	decltype(OwnerType::##name) get##name(OwnerType* obj) { return obj->##name; }				\
+	void set##name(OwnerType* obj, decltype(OwnerType::##name) val) { obj->##name = val; } 
+
+#define BS_ADD_PLAIN_FIELD(name, id) \
+	addPlainField(#name, id##, &MyType::get##name, &MyType::set##name);
+
+#define BS_ADD_REFL_FIELD(name, id) \
+	addReflectableField(#name, id##, &MyType::get##name, &MyType::set##name);
+
+#define BS_ADD_REFLPTR_FIELD(name, id) \
+	addReflectablePtrField(#name, id##, &MyType::get##name, &MyType::set##name);
+
+#define BS_PLAIN_MEMBER_VEC(name)								\
+	std::common_type<decltype(OwnerType::##name)>::type::value_type& get##name(OwnerType* obj, UINT32 idx) { return obj->##name[idx]; }				\
+	void set##name(OwnerType* obj, UINT32 idx, std::common_type<decltype(OwnerType::##name)>::type::value_type& val) { obj->##name[idx] = val; }		\
+	UINT32 getSize##name(OwnerType* obj) { return (UINT32)obj->##name.size(); }	\
+	void setSize##name(OwnerType* obj, UINT32 val) { obj->##name.resize(val); }
+
+#define BS_REFL_MEMBER_VEC(name)								\
+	std::common_type<decltype(OwnerType::##name)>::type::value_type& get##name(OwnerType* obj, UINT32 idx) { return obj->##name[idx]; }				\
+	void set##name(OwnerType* obj, UINT32 idx, std::common_type<decltype(OwnerType::##name)>::type::value_type& val) { obj->##name[idx] = val; }		\
+	UINT32 getSize##name(OwnerType* obj) { return (UINT32)obj->##name.size(); }	\
+	void setSize##name(OwnerType* obj, UINT32 val) { obj->##name.resize(val); }
+
+#define BS_REFLPTR_MEMBER_VEC(name)								\
+	std::common_type<decltype(OwnerType::##name)>::type::value_type get##name(OwnerType* obj, UINT32 idx) { return obj->##name[idx]; }				\
+	void set##name(OwnerType* obj, UINT32 idx, std::common_type<decltype(OwnerType::##name)>::type::value_type val) { obj->##name[idx] = val; }		\
+	UINT32 getSize##name(OwnerType* obj) { return (UINT32)obj->##name.size(); }	\
+	void setSize##name(OwnerType* obj, UINT32 val) { obj->##name.resize(val); }
+
+#define BS_ADD_PLAIN_FIELD_ARR(name, id) \
+	addPlainArrayField(#name, id##, &MyType::get##name, &MyType::getSize##name, \
+	&MyType::set##name, &MyType::setSize##name);
+
+#define BS_ADD_REFL_FIELD_ARR(name, id) \
+	addReflectableArrayField(#name, id##, &MyType::get##name, &MyType::getSize##name, \
+	&MyType::set##name, &MyType::setSize##name);
+
+#define BS_ADD_REFLPTR_FIELD_ARR(name, id) \
+	addReflectablePtrArrayField(#name, id##, &MyType::get##name, &MyType::getSize##name, \
+	&MyType::set##name, &MyType::setSize##name);
 
 
 	/**
 	/**
 	 * @brief	Provides an interface for accessing fields of a certain class.
 	 * @brief	Provides an interface for accessing fields of a certain class.
@@ -803,6 +847,9 @@ namespace BansheeEngine
 		}	
 		}	
 
 
 	protected:
 	protected:
+		typedef Type OwnerType;
+		typedef MyRTTIType MyType;
+
 		virtual void initSerializableFields() {}
 		virtual void initSerializableFields() {}
 
 
 		/************************************************************************/
 		/************************************************************************/

+ 4 - 2
TODO.txt

@@ -28,10 +28,12 @@ return them in checkForModifications?
 Prefab diff
 Prefab diff
 
 
 IMMEDIATE:
 IMMEDIATE:
- - Test if Binary serialization still works properly
- - Document binary diff
  - Integrated native diff and managed diff
  - Integrated native diff and managed diff
  - Create a native only unit test for binary diff
  - Create a native only unit test for binary diff
+ - Test and debug native diff
+
+Is cloning broken?
+ - What if I clone a Reflectable that references a Reflectable by pointer? Then that will be cloned as well but it shouldn't be
 
 
 See "Prefabs" gdoc for later goals
 See "Prefabs" gdoc for later goals
 See "Brainstorm" gdoc for ideas about how to solve the ID issue (and see below)
 See "Brainstorm" gdoc for ideas about how to solve the ID issue (and see below)