Merge pull request #1437 from AtomicGameEngine/JME-ATOMIC-1431

Add a significant string registry for StringHash, use string instead of hash for variant map XML keys when possible

Source/Atomic/Math/StringHash.cpp

@@ -29,6 +29,12 @@
 
 #include "../DebugNew.h"
 
+// ATOMIC BEGIN
+
+#include "../Container/HashMap.h"
+
+// ATOMIC END
+
 namespace Atomic
 {
 
@@ -69,4 +75,40 @@ String StringHash::ToString() const
     return String(tempBuffer);
 }
 
+// ATOMIC BEGIN
+
+// Lookup for significant strings, not a member of StringHash so don't need to drag hashmap into header
+static HashMap<unsigned, String> gSignificantLookup;
+
+void StringHash::RegisterSignificantString(const char* str)
+{
+    unsigned hash = Calculate(str);
+
+    if (gSignificantLookup.Contains(hash))
+        return;
+
+    gSignificantLookup[hash] = String(str);
+
+}
+
+void StringHash::RegisterSignificantString(const String& str)
+{
+    RegisterSignificantString(str.CString());
+}
+
+bool StringHash::GetSignificantString(unsigned hash, String& strOut)
+{
+    if (!gSignificantLookup.TryGetValue(hash, strOut))
+    {
+        strOut.Clear();
+        return false;
+    }
+
+    return true;
+
+}
+
+// ATOMIC END
+
+
 }

Source/Atomic/Math/StringHash.h

@@ -106,9 +106,23 @@ public:
     /// Zero hash.
     static const StringHash ZERO;
 
+    // ATOMIC BEGIN
+
+    /// Register significant C string, which can be looked up via hash, note that the lookup is case insensitive
+    static void RegisterSignificantString(const char* str);
+
+    /// Register significant string, which can be looked up via hash, note that the lookup is case insensitive
+    static void RegisterSignificantString(const String& str);
+
+    /// Get a significant string from a case insensitive hash value
+    static bool GetSignificantString(unsigned hash, String& strOut);
+
+    // ATOMIC END
+
 private:
     /// Hash value.
     unsigned value_;
+
 };
 
 }

Source/Atomic/Resource/XMLElement.cpp

@@ -462,7 +462,20 @@ bool XMLElement::SetVariantMap(const VariantMap& value)
         XMLElement variantElem = CreateChild("variant");
         if (!variantElem)
             return false;
-        variantElem.SetUInt("hash", i->first_.Value());
+
+// ATOMIC BEGIN
+        // Check for significant string, to make variant map XML more friendly than using a hash
+        String sigString;
+        if (StringHash::GetSignificantString(i->first_.Value(), sigString))
+        {
+            variantElem.SetString("name", sigString);
+        }
+        else
+        {
+            variantElem.SetUInt("hash", i->first_.Value());
+        }
+// ATOMIC END
+
         variantElem.SetVariant(i->second_);
     }
 

Source/Atomic/Script/ScriptComponent.cpp

@@ -28,7 +28,9 @@
 namespace Atomic
 {
 
-ScriptComponent::ScriptComponent(Context* context) : Component(context)
+ScriptComponent::ScriptComponent(Context* context) : Component(context),
+    loading_(false)
+
 {
 
 }
@@ -36,7 +38,9 @@ ScriptComponent::ScriptComponent(Context* context) : Component(context)
 void ScriptComponent::RegisterObject(Context* context)
 {
     ATOMIC_ACCESSOR_ATTRIBUTE("Is Enabled", IsEnabled, SetEnabled, bool, true, AM_DEFAULT);
-    ATOMIC_ATTRIBUTE("FieldValues", VariantMap, fieldValues_, Variant::emptyVariantMap, AM_FILE);
+
+    ATOMIC_ACCESSOR_ATTRIBUTE("FieldValues", GetFieldValuesAttr, SetFieldValuesAttr, VariantMap, Variant::emptyVariantMap, AM_FILE);
+
 }
 
 ScriptComponent::~ScriptComponent()
@@ -44,4 +48,32 @@ ScriptComponent::~ScriptComponent()
 
 }
 
+bool ScriptComponent::Load(Deserializer& source, bool setInstanceDefault)
+{
+    loading_ = true;
+    bool success = Component::Load(source, setInstanceDefault);
+    loading_ = false;
+
+    return success;
+}
+
+bool ScriptComponent::LoadXML(const XMLElement& source, bool setInstanceDefault)
+{
+    loading_ = true;
+    bool success = Component::LoadXML(source, setInstanceDefault);
+    loading_ = false;
+
+    return success;
+}
+
+const VariantMap& ScriptComponent::GetFieldValuesAttr() const
+{
+    return fieldValues_;
+}
+
+void ScriptComponent::SetFieldValuesAttr(const VariantMap& value)
+{
+    fieldValues_ = value;
+}
+
 }

Source/Atomic/Script/ScriptComponent.h

@@ -32,7 +32,7 @@ class ScriptComponentFile;
 
 class ATOMIC_API ScriptComponent : public Component
 {
-    ATOMIC_OBJECT(ScriptComponent, Component);
+    ATOMIC_OBJECT(ScriptComponent, Component)
 
 public:
 
@@ -49,11 +49,19 @@ public:
 
     VariantMap& GetFieldValues() { return fieldValues_; }
 
+    bool Load(Deserializer& source, bool setInstanceDefault);
+    bool LoadXML(const XMLElement& source, bool setInstanceDefault);
 
 protected:
 
+    const VariantMap& GetFieldValuesAttr() const;
+    void SetFieldValuesAttr(const VariantMap& value);
+
     VariantMap fieldValues_;
 
+    bool loading_;
+
+
 };
 
 }

Source/Atomic/Script/ScriptComponentFile.h

@@ -42,11 +42,14 @@ struct FieldInfo
 
     FieldInfo(const String& name, VariantType variantType, const String& resourceTypeName = String::EMPTY, bool isArray = false, unsigned fixedArraySize = 0)
     {
-        name_ = name;        
+        name_ = name;
         variantType_ = variantType;
         resourceTypeName_ = resourceTypeName;
         isArray_ = isArray;
         fixedArraySize_ = fixedArraySize;
+
+        // register field name as significant, for serialization
+        StringHash::RegisterSignificantString(name);
     }
 
     String name_;

Source/AtomicJS/Javascript/JSComponent.cpp

@@ -125,8 +125,7 @@ JSComponent::JSComponent(Context* context) :
     started_(false),
     destroyed_(false),
     scriptClassInstance_(false),
-    delayedStartCalled_(false),
-    loading_(false)
+    delayedStartCalled_(false)
 {
     vm_ = JSVM::GetJSVM(NULL);
 }
@@ -157,10 +156,6 @@ void JSComponent::SetUpdateEventMask(unsigned char mask)
     }
 }
 
-void JSComponent::ApplyAttributes()
-{
-}
-
 bool JSComponent::IsInstanceInitialized() {
     return instanceInitialized_;
 }
@@ -505,24 +500,6 @@ void JSComponent::HandlePhysicsPostStep(StringHash eventType, VariantMap& eventD
 }
 #endif
 
-bool JSComponent::Load(Deserializer& source, bool setInstanceDefault)
-{
-    loading_ = true;
-    bool success = Component::Load(source, setInstanceDefault);
-    loading_ = false;
-
-    return success;
-}
-
-bool JSComponent::LoadXML(const XMLElement& source, bool setInstanceDefault)
-{
-    loading_ = true;
-    bool success = Component::LoadXML(source, setInstanceDefault);
-    loading_ = false;
-
-    return success;
-}
-
 bool JSComponent::MatchScriptName(const String& path)
 {
     if (componentFile_.Null())

Source/AtomicJS/Javascript/JSComponent.h

@@ -58,10 +58,6 @@ public:
     /// Register object factory.
     static void RegisterObject(Context* context);
 
-    bool Load(Deserializer& source, bool setInstanceDefault);
-    bool LoadXML(const XMLElement& source, bool setInstanceDefault);
-    void ApplyAttributes();
-
     /// Match script name
     bool MatchScriptName(const String& path);
 

Source/AtomicJS/Javascript/JSSceneSerializable.cpp

@@ -117,7 +117,9 @@ namespace Atomic
                     const FieldMap& fields = file->GetFields(className);
                     const HashMap<String, Vector<EnumInfo>>& enums = file->GetEnums(className);
 
-                    if (fieldInfo = fields[name])
+                    fieldInfo = fields[name];
+
+                    if (fieldInfo)
                     {
                         variantType = fieldInfo->variantType_;