Advertisement Jump to content
Sign in to follow this  
iraxef

[PATCH] access typeId of values stored in CScriptDictionary

This topic is 1783 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

We have a use-case for sending a dictionary back to C++. In order to then extract the values out of the CScriptDictionary, it's necessary to be able to access the type id of the values. Please consider something along these lines for inclusion in the dictionary add-on:

Index: scriptdictionary.cpp
===================================================================
--- scriptdictionary.cpp	(revision 1855)
+++ scriptdictionary.cpp	(working copy)
@@ -408,6 +408,32 @@
 	return array;
 }

+// Get a map of keys to their value type id
+std::map<std::string, int> CScriptDictionary::GetKeyToValueTypeIdMap() const
+{
+	std::map<std::string, int> keyToTypeId;
+
+	for ( map<std::string, valueStruct>::const_iterator iter = dict.begin();
+	      iter != dict.end();
+	      ++iter )
+	{
+		keyToTypeId[iter->first] = iter->second.typeId;
+	}
+
+	return keyToTypeId;
+}
+
+// Get the type id of a key's value
+int CScriptDictionary::GetTypeIdForKey(const std::string &key) const
+{
+	map<std::string, valueStruct>::const_iterator it = dict.find(key);
+
+	if( it != dict.end() )
+		return it->second.typeId;
+	else
+		return 0;
+}
+
 //--------------------------------------------------------------------------
 // Generic wrappers

Index: scriptdictionary.h
===================================================================
--- scriptdictionary.h	(revision 1855)
+++ scriptdictionary.h	(working copy)
@@ -72,6 +72,12 @@
 	// Get an array of all keys
 	CScriptArray *GetKeys() const;

+	// Get a map of keys to their value type id
+	std::map<std::string, int> GetKeyToValueTypeIdMap() const;
+
+	// Get the type id of a key's value
+	int GetTypeIdForKey(const std::string &key) const;
+
 	// Garbage collections behaviours
 	int GetRefCount();
 	void SetGCFlag();

Thank you very much.

Edited by iraxef

Share this post


Link to post
Share on other sites
Advertisement

Having GetKeyToValueTypeIdMap avoids an extra map lookup.

// variant one:
auto keysAndTypes = dictionary.GetKeyToValueTypeIdMap();
for ( keysAndTypes )
{
  based on the type, call the right overload of dictionary.Get()
}

// variant two:
for ( dictionary )
{
  call GetTypeIdForKey for this key
  with that type id, call the right Get()
}

With variant one, you go through the map once, and then have one further lookup per key.

With variant two, you go through the map once, but have two lookups per key. This is unnecessary.

 

We actually only use GetKeyToValueTypeIdMap(), but provided GetTypeIdKey() for convenience/completeness.

Share this post


Link to post
Share on other sites

Sounds like what you really need is an iterator for the internal std::map that exposes both the type id and a method for obtaining the correct value.

 

This way you would iterate over the map once, and already have access to both the value and type id without an extra lookup.

Share this post


Link to post
Share on other sites

I didn't think you'd want to make valueStruct public, but something along these lines would be fine:

    // The structure for holding the values
    struct valueStruct
    {
        union
        {
            asINT64 valueInt;
            double  valueFlt;
            void   *valueObj;
        };
        int   typeId;
    };

    typedef std::map<std::string, valueStruct>  tDictContainer;

    tDictContainer::const_iterator GetContainerBegin() const { return dict.begin(); }
    tDictContainer::const_iterator GetContainerEnd() const { return dict.end(); }

protected:

    ...

Share this post


Link to post
Share on other sites

Perhaps not exposing the valueStruct directly, but having an iterator that knows how to interpret the valueStruct sounds like a good idea. Something like this:

class CDictionaryIterator {
public:
   void operator++();                             // move to next key-value pair
   bool operator==(const CDictionaryIterator &);  // compare position with other iterator
   
   int getTypeId();                               // get the type id of the value
   void *getAddressOfValue();                     // get the address of the value that can be manually cast to the type represented by the type id
   bool getValue(void *value, int typeId);        // assigns the value with appropriate cast (similarly to how the CDictionary::Get(key, value, typeId) does it)
   const string &getKey();                        // get the key
 
protected:
   // Internally it uses an iterator to access the internal map of the dictionary
   std::map<std::string, CDictionary::valueStruct>::iterator it;
};

Then the dictionary should have begin() and end() methods to allow the iterator to be used just like a normal STL iterator.

 

Obviously the dictionary iterator should be a child class of the dictionary, and should share the same logic for converting the value.

Edited by Andreas Jonsson

Share this post


Link to post
Share on other sites

Thanks for the suggestion. I tested this and it worked for my use-case:

Index: scriptdictionary.h
===================================================================
--- scriptdictionary.h	(revision 1855)
+++ scriptdictionary.h	(working copy)
@@ -105,6 +105,39 @@

 	// TODO: optimize: Use C++11 std::unordered_map instead
 	std::map<std::string, valueStruct> dict;
+public:
+    class CDictionaryIterator
+    {
+    public:
+        CDictionaryIterator(
+            const CScriptDictionary& _dict,
+            std::map<std::string, CScriptDictionary::valueStruct>::const_iterator _it)
+            : dict(_dict)
+            , it(_it)
+        {}
+
+        void operator++() { ++it; }
+
+        bool operator==(const CDictionaryIterator &other) const { return it == other.it; }
+        bool operator!=(const CDictionaryIterator &other) const { return it != other.it; }
+
+        const std::string &GetKey() const { return it->first; }
+        int GetTypeId() const             { return it->second.typeId; }
+        void *GetAddressOfValue() const   { return it->second.valueObj; }
+
+        bool GetValue(asINT64 &value) const { return dict.Get(GetKey(), value); }
+        bool GetValue(double &value) const  { return dict.Get(GetKey(), value); }
+        bool GetValue(void *value) const    { return dict.Get(GetKey(), value, GetTypeId()); }
+
+    protected:
+        CDictionaryIterator();
+
+        std::map<std::string, CScriptDictionary::valueStruct>::const_iterator it;
+        const CScriptDictionary& dict;
+    };
+
+    CDictionaryIterator begin() const { return CDictionaryIterator(*this, dict.begin()); }
+    CDictionaryIterator end() const   { return CDictionaryIterator(*this, dict.end()); }
 };

 // This function will determine the configuration of the engine

Share this post


Link to post
Share on other sites

I added the iterator in revision 1863. 

 

I made some further tweaks to it so now supports C++11 range-for loops too, and I removed the unnecessary look-up in the map by having the iterator directly use the valueStruct to return the value.

 

void function(CScriptDictionary *dict)
{
   for( auto it : *dict )
   {
      string key = it.GetKey();
      int typeId = it.GetTypeId();
      asINT64 value;
      it.GetValue(value);
   }
}

Share this post


Link to post
Share on other sites

Thanks for this!

 

A postfix increment signature would typically not return a void, but something like const CScriptDictionary::CIterator &. You could either do that, or use ++m_it under the hood to avoid a temporary (?).
 
Could you help me understand why GetValue(void *value, int typeId) takes a typeId instead of implicitly using the iterator's current GetTypeId() ? Why make me provide the typeId when the iterator already has it?

Share this post


Link to post
Share on other sites

GetValue needs an explicitly informed typeId because it is possible to get the value using a different type than what is actually stored in the dictionary. In this case the dictionary will attempt to convert the stored value to the desired type. If the conversion is not successful, i.e. the types are not compatible, GetValue will return false.

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

GameDev.net is your game development community. Create an account for your GameDev Portfolio and participate in the largest developer community in the games industry.

Sign me up!