Updated PropSet to use C++ map instead of homemade BST code. I originally

wrote this back in 2000 or so, after I had just finished an algorithms
design course, and insisted on coding my own ADT.  I've come to realize
that in most cases, rolling your own is just a waste of time :)  Related
to this, the 'listrominfo' commandline argument now correctly shows all
the ROM info built in to Stella internal database.

Bumped version # to 2.8.  I just need to check out OSX, and then we
should be ready for the next release.


git-svn-id: svn://svn.code.sf.net/p/stella/code/trunk@1757 8b62c5a3-ac7e-4cc8-8f21-d9a121418aba

Todo.txt

@@ -68,10 +68,6 @@ Stephen Anthony at stephena@users.sourceforge.net.
 
   * More support for copy and paste.
 
-  * Fix PropSet to properly delete entries when 'Default' is pressed in
-    the GameInfoDialog; perhaps move to C++ maps instead of the current
-    home-made BST code.
-
   * Fix Props.cxx (or GameInfoDialog) to properly deal with quotes and
     backslashes in strings meant to be saved to the properties file.
 

src/common/Version.hxx

@@ -19,7 +19,7 @@
 #ifndef VERSION_HXX
 #define VERSION_HXX
 
-#define STELLA_BASE_VERSION "2.7.8_svn"
+#define STELLA_BASE_VERSION "2.8"
 
 #ifdef NIGHTLY_BUILD
   #define STELLA_VERSION STELLA_BASE_VERSION "pre-" NIGHTLY_BUILD

src/emucore/PropsSet.cxx

@@ -17,6 +17,7 @@
 //============================================================================
 
 #include <sstream>
+#include <map>
 
 #include "bspf.hxx"
 
@@ -30,20 +31,59 @@
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 PropertiesSet::PropertiesSet(OSystem* osystem)
   : myOSystem(osystem),
-    myRoot(NULL),
     mySize(0)
 {
   const string& props = myOSystem->propertiesFile();
-  load(props, true);    // do save these properties
+  load(props);
-
-  if(myOSystem->settings().getBool("showinfo"))
-    cout << "User game properties: \'" << props << "\'\n";
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 PropertiesSet::~PropertiesSet()
 {
-  deleteNode(myRoot);
+  myExternalProps.clear();
+  myTempProps.clear();
+}
+
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+void PropertiesSet::load(const string& filename)
+{
+  if(myOSystem->settings().getBool("showinfo"))
+    cout << "User game properties: \'" << filename << "\'\n";
+
+  ifstream in(filename.c_str(), ios::in);
+
+  // Loop reading properties
+  for(;;)
+  {
+    // Make sure the stream is still good or we're done 
+    if(!in)
+      break;
+
+    // Get the property list associated with this profile
+    Properties prop;
+    prop.load(in);
+
+    // If the stream is still good then insert the properties
+    if(in)
+      insert(prop);
+  }
+  if(in)
+    in.close();
+}
+
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+bool PropertiesSet::save(const string& filename) const
+{
+  ofstream out(filename.c_str(), ios::out);
+  if(!out)
+    return false;
+
+  // Only save those entries in the external list
+  for(PropsList::const_iterator i = myExternalProps.begin();
+      i != myExternalProps.end(); ++i)
+    i->second.save(out);
+
+  return true;
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
@@ -53,29 +93,33 @@ void PropertiesSet::getMD5(const string& md5, Properties& properties,
   properties.setDefaults();
   bool found = false;
 
-  // First check our dynamic BST for the object
+  // There are three lists to search when looking for a properties entry,
-  if(!useDefaults && myRoot != 0)
+  // which must be done in the following order
-  {
+  // If 'useDefaults' is specified, only use the built-in list
-    TreeNode* current = myRoot;
+  //
-    while(current)
+  //  'save': entries previously inserted that are saved on program exit
-    {
+  //  'temp': entries previously inserted that are discarded
-      const string& currentMd5 = current->props->get(Cartridge_MD5);
+  //  'builtin': the defaults compiled into the program
-      if(currentMd5 == md5)
-      {
-        // We only report a node as found if it's been marked as valid.
-        // Invalid nodes are those that should be removed, and are
-        // essentially treated as if they're not present.
-        found = current->valid;
-        break;
-      }
-      else if(md5 < currentMd5)
-        current = current->left;
-      else 
-        current = current->right;
-    }
 
-    if(found)
+  // First check properties from external file
-      properties = *(current->props);
+  if(!useDefaults)
+  {
+    // Check external list
+    PropsList::const_iterator iter = myExternalProps.find(md5);
+    if(iter != myExternalProps.end())
+    {
+      properties = iter->second;
+      found = true;
+    }
+    else  // Search temp list
+    {
+      iter = myTempProps.find(md5);
+      if(iter != myTempProps.end())
+      {
+        properties = iter->second;
+        found = true;
+      }
+    }
   }
 
   // Otherwise, search the internal database using binary search
@@ -107,151 +151,71 @@ void PropertiesSet::getMD5(const string& md5, Properties& properties,
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 void PropertiesSet::insert(const Properties& properties, bool save)
 {
+  // Note that the following code is optimized for insertion when an item
+  // doesn't already exist, and when the external properties file is
+  // relatively small (which is the case with current versions of Stella,
+  // as the properties are built-in)
+  // If an item does exist, it will be removed and insertion done again
+  // This shouldn't be a speed issue, as insertions will only fail with
+  // duplicates when you're changing the current ROM properties, which
+  // most people tend not to do
+
   // Since the PropSet is keyed by md5, we can't insert without a valid one
-  if(properties.get(Cartridge_MD5) == "")
+  const string& md5 = properties.get(Cartridge_MD5);
+  if(md5 == "")
     return;
 
-  insertNode(myRoot, properties, save);
+  // The status of 'save' determines which list to save to
+  PropsList& list = save ? myExternalProps : myTempProps;
+
+  pair<PropsList::iterator,bool> ret;
+  ret = list.insert(make_pair(md5, properties));
+  if(ret.second == false)
+  {
+    // Remove old item and insert again
+    list.erase(ret.first);
+    list.insert(make_pair(md5, properties));
+  }
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 void PropertiesSet::removeMD5(const string& md5)
 {
-  // We only remove from the dynamic BST
+  // We only remove from the external list
-  if(myRoot != 0)
+  myExternalProps.erase(md5);
-  {
-    TreeNode* current = myRoot;
-    while(current)
-    {
-      const string& currentMd5 = current->props->get(Cartridge_MD5);
-      if(currentMd5 == md5)
-      {
-        current->valid = false;  // Essentially, this node doesn't exist
-        break;
-      }
-      else if(md5 < currentMd5)
-        current = current->left;
-      else 
-        current = current->right;
-    }
-  }
-}
-
-// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-void PropertiesSet::insertNode(TreeNode* &t, const Properties& properties,
-                               bool save)
-{
-  if(t)
-  {
-    string md5 = properties.get(Cartridge_MD5);
-    string currentMd5 = t->props->get(Cartridge_MD5);
-
-    if(md5 < currentMd5)
-      insertNode(t->left, properties, save);
-    else if(md5 > currentMd5)
-      insertNode(t->right, properties, save);
-    else
-    {
-      delete t->props;
-      t->props = new Properties(properties);
-      t->save = save;
-      t->valid = true;
-    }
-  }
-  else
-  {
-    t = new TreeNode;
-    t->props = new Properties(properties);
-    t->left = 0;
-    t->right = 0;
-    t->save = save;
-    t->valid = true;
-
-    ++mySize;
-  }
-}
-
-// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-void PropertiesSet::deleteNode(TreeNode *node)
-{
-  if(node)
-  {
-    deleteNode(node->left);
-    deleteNode(node->right);
-    delete node->props;
-    delete node;
-  }
-}
-
-// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-void PropertiesSet::load(const string& filename, bool save)
-{
-  ifstream in(filename.c_str(), ios::in);
-
-  // Loop reading properties
-  for(;;)
-  {
-    // Make sure the stream is still good or we're done 
-    if(!in)
-      break;
-
-    // Get the property list associated with this profile
-    Properties prop;
-    prop.load(in);
-
-    // If the stream is still good then insert the properties
-    if(in)
-      insert(prop, save);
-  }
-  if(in)
-    in.close();
-}
-
-// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-bool PropertiesSet::save(const string& filename) const
-{
-  ofstream out(filename.c_str(), ios::out);
-  if(!out)
-    return false;
-
-  saveNode(out, myRoot);
-  out.close();
-  return true;
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 void PropertiesSet::print() const
 {
-  cout << size() << endl;
+  // We only look at the external properties and the built-in ones;
-  printNode(myRoot);  // FIXME - print out internal properties as well
+  // the temp properties are ignored
-}
+  // Also, any properties entries in the external file override the built-in
+  // ones
+  // The easiest way to merge the lists is to create another temporary one
+  // This isn't fast, but I suspect this method isn't used too often (or at all)
 
-// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+  PropsList list;
-void PropertiesSet::saveNode(ostream& out, TreeNode *node) const
+
-{
+  // First insert all external props
-  if(node)
+  list = myExternalProps;
+
+  // Now insert all the built-in ones
+  // Note that if we try to insert a duplicate, the insertion will fail
+  // This is fine, since a duplicate in the built-in list means it should
+  // be overrided anyway (and insertion shouldn't be done)
+  Properties properties;
+  for(int i = 0; i < DEF_PROPS_SIZE; ++i)
   {
-    if(node->valid && node->save)
+    properties.setDefaults();
-      node->props->save(out);
+    for(int p = 0; p < LastPropType; ++p)
-    saveNode(out, node->left);
+      if(DefProps[i][p][0] != 0)
-    saveNode(out, node->right);
+        properties.set((PropertyType)p, DefProps[i][p]);
-  }
-}
 
-// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+    list.insert(make_pair(DefProps[i][Cartridge_MD5], properties));
-void PropertiesSet::printNode(TreeNode *node) const
-{
-  if(node)
-  {
-    if(node->valid && node->save)
-      node->props->print();
-    printNode(node->left);
-    printNode(node->right);
   }
-}
 
-// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+  // Now, print the resulting list
-uInt32 PropertiesSet::size() const
+  for(PropsList::const_iterator i = list.begin(); i != list.end(); ++i)
-{
+    i->second.print();
-  return mySize;
 }

src/emucore/PropsSet.hxx

@@ -19,7 +19,7 @@
 #ifndef PROPERTIES_SET_HXX
 #define PROPERTIES_SET_HXX
 
-#include <fstream>
+#include <map>
 
 #include "bspf.hxx"
 
@@ -27,9 +27,9 @@ class OSystem;
 class Properties;
 
 /**
-  This class maintains a sorted collection of properties.  The objects
+  This class maintains an ordered collection of properties, maintained
-  are maintained in a binary search tree sorted by md5, since this is
+  in a C++ map and accessible by ROM md5.  The md5 is used since this is
-  the attribute most likely to be present in each entry in stella.pro
+  the attribute which must be present in each entry in stella.pro
   and least likely to change.  A change in MD5 would mean a change in
   the game rom image (essentially a different game) and this would
   necessitate a new entry in the stella.pro file anyway.
@@ -52,25 +52,12 @@ class PropertiesSet
 
   public:
     /** 
-      Get the property from the set with the given MD5.
+      Load properties from the specified file, and create an internal
-
+      searchable list.
-      @param md5         The md5 of the property to get
-      @param properties  The property with the given MD5, or the default
-                         properties if not found
-      @param defaults    Use the built-in defaults, ignoring any external properties
-    */
-    void getMD5(const string& md5, Properties& properties,
-                bool useDefaults = false) const;
-
-    /** 
-      Load properties from the specified file.  Use the given 
-      defaults properties as the defaults for any properties loaded.
 
       @param filename  Full pathname of input file to use
-      @param save      Indicates whether to set the 'save' tag for
-                       these properties
     */
-    void load(const string& filename, bool save);
+    void load(const string& filename);
 
     /**
       Save properties to the specified file.
@@ -82,15 +69,27 @@ class PropertiesSet
     */
     bool save(const string& filename) const;
 
+    /**
+      Get the property from the set with the given MD5.
+
+      @param md5         The md5 of the property to get
+      @param properties  The properties with the given MD5, or the default
+                         properties if not found
+      @param defaults    Use the built-in defaults, ignoring any properties
+                         from an external file
+    */
+    void getMD5(const string& md5, Properties& properties,
+                bool useDefaults = false) const;
+
     /**
       Insert the properties into the set.  If a duplicate is inserted
       the old properties are overwritten with the new ones.
 
       @param properties  The collection of properties
-      @param save        Indicates whether to set the 'save' tag for
+      @param save        Indicates whether the properties should be saved
-                         this property
+                         when the program exits
     */
-    void insert(const Properties& properties, bool save);
+    void insert(const Properties& properties, bool save = true);
 
     /**
       Marks the property with the given MD5 as being removed.
@@ -99,65 +98,23 @@ class PropertiesSet
     */
     void removeMD5(const string& md5);
 
-    /**
-      Get the number of properties in the collection.
-
-      @return  The number of properties in the collection
-    */
-    uInt32 size() const;
-
     /**
       Prints the contents of the PropertiesSet as a flat file.
     */
     void print() const;
 
   private:
-    struct TreeNode {
+    typedef map<string, Properties> PropsList;
-      Properties* props;
-      TreeNode* left;
-      TreeNode* right;
-      bool save;
-      bool valid;
-    };
 
-    /**
-      Insert a node in the bst, keeping the tree sorted.
-
-      @param node        The current subroot of the tree
-      @param properties  The collection of properties
-      @param save        Indicates whether to set the 'save' tag for
-                         this property
-    */
-    void insertNode(TreeNode* &node, const Properties& properties, bool save);
-
-    /**
-      Deletes a node from the bst.  Does not preserve bst sorting.
-
-      @param node  The current subroot of the tree
-    */
-    void deleteNode(TreeNode *node);
-
-    /**
-      Save current node properties to the specified output stream 
-
-      @param out   The output stream to use
-      @param node  The current subroot of the tree
-    */
-    void saveNode(ostream& out, TreeNode* node) const;
-
-    /**
-      Prints the current node properties
-
-      @param node  The current subroot of the tree
-    */
-    void printNode(TreeNode* node) const;
-
-  private:
     // The parent system for this object
     OSystem* myOSystem;
 
-    // The root of the BST
+    // The properties read from an external 'stella.pro' file
-    TreeNode* myRoot;
+    PropsList myExternalProps;
+
+    // The properties temporarily inserted by the program, which should
+    // be discarded when the program ends
+    PropsList myTempProps;
 
     // The size of the properties bst (i.e. the number of properties in it)
     uInt32 mySize;

src/gui/GameInfoDialog.cxx

@@ -473,7 +473,7 @@ void GameInfoDialog::saveConfig()
   if(myDefaultsSelected)
     instance().propSet().removeMD5(myGameProperties.get(Cartridge_MD5));
   else
-    instance().propSet().insert(myGameProperties, true);
+    instance().propSet().insert(myGameProperties);
 
   // In any event, inform the Console and save the properties
   if(&instance().console())