Loading from ZIP files now works in all cases, and then some.

That is, it has all the functionality of past versions, as well
as improvements to launching from the commandline (an archive
containing multiple ROMs will now open the virtual directory
in the ROM launcher.


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

Changes.txt

@@ -23,6 +23,26 @@
       - The 'volume clipping' option has been removed, since in 16-bit
         mode it's no longer needed.
       - The 'Tia freq' option has been removed.
+      - Selecting more common sample rates (other than 31400) now works
+        much better, but there are still a few ROMS (like Quadrun) where
+        31400Hz still works best.
+
+  * Many changes to handling ZIP archives:
+      - Files in multiple levels are now recognized.  This fixes issues
+        in Windows where such files couldn't be loaded at all, and in all
+        systems where ROMs with the same name (but different directories)
+        weren't being recognized.
+      - ZIP contents are now handled more intelligently.  Archives
+        containing only one ROM are automatically loaded, whereas those
+        with multiple files are treated as directories.
+      - Opening an archive from the commandline now works as in the UI,
+        where opening a multi-ROM archive will pop up the UI and show the
+        archive contents (as a directory).
+      - The ZIP code behind the scenes is now much faster by making use
+        of caching (the old code was actually from 1998!).
+      - This new 'archive' infrastructure may eventually lead to 7-Zip
+        support, as well as 'virtual' formats (such as showing the list
+        of files for 2in1/4in1/8in1/etc within the UI.
 
   * Improved bankswitch autodetection for X07 ROMs (although there's only
     two known ROMs in existence, so the detection probably isn't robust).
@@ -33,9 +53,6 @@
   * Fixed regression in RIOT INTIM reads; at least one known ROM
     (Mr. Roboto Berzerk hack) wasn't working properly.
 
-  * Fixed ZIP file handling in Windows when the archive contained
-    multiple files; in several cases the ROM wasn't being loaded at all.
-
   * Updated included PNG and ZLIB libraries to latest stable version.
 
 -Have fun!

src/common/FSNodeZIP.cxx

@@ -30,7 +30,8 @@ FilesystemNodeZIP::FilesystemNodeZIP()
 {
   // We need a name, else the node is invalid
   _path = _shortPath = _virtualFile = "";
-  _isValid = _isDirectory = _isFile = _isVirtual = false;
+  _isValid = false;
+  _numFiles = 0;
 
   AbstractFSNode* tmp = 0;
   _realNode = Common::SharedPtr<AbstractFSNode>(tmp);
@@ -39,22 +40,46 @@ FilesystemNodeZIP::FilesystemNodeZIP()
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 FilesystemNodeZIP::FilesystemNodeZIP(const string& p)
 {
+  _path = _shortPath = _virtualFile = "";
+  _isValid = false;
+
   // Extract ZIP file and virtual file (if specified)
   size_t pos = BSPF_findIgnoreCase(p, ".zip");
   if(pos == string::npos)
-  {
-    // Not a ZIP file
-    _path = _shortPath = _virtualFile = "";
-    _isValid = _isDirectory = _isFile = _isVirtual = false;
     return;
-  }
 
   _zipFile = p.substr(0, pos+4);
+
+  // Open file at least once to initialize the virtual file count
+  ZipHandler& zip = OSystem::zip(_zipFile);
+  _numFiles = zip.romFiles();
+  if(_numFiles == 0)
+    return;
+
+  // We always need a virtual file
+  // Either one is given, or we use the first one
   if(pos+5 < p.length())
     _virtualFile = p.substr(pos+5);
+  else if(_numFiles == 1)
+  {
+    bool found = false;
+    while(zip.hasNext() && !found)
+    {
+      const std::string& file = zip.next();
+      if(BSPF_endsWithIgnoreCase(file, ".a26") ||
+         BSPF_endsWithIgnoreCase(file, ".bin") ||
+         BSPF_endsWithIgnoreCase(file, ".rom"))
+      {
+        _virtualFile = file;
+        found = true;
+      }
+    }
+    if(!found)
+      return;
+  }
 
-  AbstractFSNode* tmp = FilesystemNodeFactory::create(
+  AbstractFSNode* tmp =
-                            _zipFile, FilesystemNodeFactory::SYSTEM);
+    FilesystemNodeFactory::create(_zipFile, FilesystemNodeFactory::SYSTEM);
   _realNode = Common::SharedPtr<AbstractFSNode>(tmp);
 
   setFlags(_zipFile, _virtualFile, _realNode);
@@ -82,27 +107,22 @@ void FilesystemNodeZIP::setFlags(const string& zipfile,
   // Is a file component present?
   if(_virtualFile.size() != 0)
   {
-    _isVirtual = true;
     _path += ("/" + _virtualFile);
     _shortPath += ("/" + _virtualFile);
+    _numFiles = 1;
   }
-  else
+  _isValid = _realNode->isFile() && _realNode->isReadable();
-    _isVirtual = false;
-
-  _isDirectory = !_isVirtual;
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 bool FilesystemNodeZIP::getChildren(AbstractFSList& myList, ListMode mode,
                                     bool hidden) const
 {
-  assert(_isDirectory);
-
   // Files within ZIP archives don't contain children
-  if(_isVirtual)
+  if(!isDirectory() || !_isValid)
     return false;
 
-  ZipHandler& zip = OSystem::zip(_path);
+  ZipHandler& zip = OSystem::zip(_zipFile);
   while(zip.hasNext())
   {
     FilesystemNodeZIP entry(_path, zip.next(), _realNode);
@@ -115,33 +135,16 @@ bool FilesystemNodeZIP::getChildren(AbstractFSList& myList, ListMode mode,
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 bool FilesystemNodeZIP::read(uInt8*& image, uInt32& size) const
 {
-cerr << "FilesystemNodeZIP::read" << endl
+  if(!_isValid)
-  << "  zfile: " << _zipFile << endl
+    return false;
-  << "  vfile: " << _virtualFile << endl
-  << endl;
 
   ZipHandler& zip = OSystem::zip(_zipFile);
+
   bool found = false;
   while(zip.hasNext() && !found)
-  {
+    found = zip.next() == _virtualFile;
-const string& next = zip.next();
-cerr << "searching: " << next << endl;
-    found = _virtualFile == next;
-  }
 
-  if(found)
+  return found ? zip.decompress(image, size) : false;
-  {
-cerr << "decompressing ...\n";
-    return zip.decompress(image, size);
-  }
-  else
-    return false;
-}
-
-// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-bool FilesystemNodeZIP::isAbsolute() const
-{
-  return false;
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -

src/common/FSNodeZIP.hxx

@@ -28,6 +28,10 @@
 
 /*
  * Implementation of the Stella file system API based on ZIP archives.
+ * ZIP archives are treated as directories if the contain more than one ROM
+ * file, as a file if they contain a single ROM file, and as neither if the
+ * archive is empty.  Hence, if a ZIP archive isn't a directory *or* a file,
+ * it is invalid.
  *
  * Parts of this class are documented in the base interface class, AbstractFSNode.
  */
@@ -47,15 +51,15 @@ class FilesystemNodeZIP : public AbstractFSNode
      */
     FilesystemNodeZIP(const string& path);
 
-    bool exists() const { return false; }
+    bool exists() const      { return _realNode->exists();     }
     const string& getName() const { return _virtualFile; }
     const string& getPath() const { return _path;        }
     string getShortPath() const   { return _shortPath;   }
-    bool isDirectory() const { return _isDirectory; }
+    bool isDirectory() const { return _numFiles > 1;           }
-    bool isFile() const      { return _isFile;      }
+    bool isFile() const      { return _numFiles == 1;          }
-    bool isReadable() const  { return false; }
+    bool isReadable() const  { return _realNode->isReadable(); }
     bool isWritable() const  { return false; }
-    bool isAbsolute() const;
+    bool isAbsolute() const  { return _realNode->isAbsolute(); }
 
     //////////////////////////////////////////////////////////
     // For now, ZIP files cannot be modified in any way
@@ -79,10 +83,8 @@ class FilesystemNodeZIP : public AbstractFSNode
     Common::SharedPtr<AbstractFSNode> _realNode;
     string _zipFile, _virtualFile;
     string _path, _shortPath;
-    bool _isDirectory;
-    bool _isFile;
     bool _isValid;
-    bool _isVirtual;
+    uInt32 _numFiles;
 };
 
 #endif

src/common/Version.hxx

@@ -22,7 +22,7 @@
 
 #include <cstdlib>
 
-#define STELLA_VERSION "3.8_pre"
+#define STELLA_VERSION "3.8_alpha1"
 #define STELLA_BUILD atoi("$Rev$" + 6)
 
 #endif

src/common/ZipHandler.cxx

@@ -17,12 +17,12 @@
 // $Id$
 //============================================================================
 
-#include "ZipHandler.hxx"
-
 #include <cctype>
 #include <cstdlib>
 #include <zlib.h>
 
+#include "ZipHandler.hxx"
+
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 ZipHandler::ZipHandler()
   : myZip(NULL)
@@ -69,13 +69,18 @@ string ZipHandler::next()
 {
   if(myZip)
   {
+    bool valid = false;
     const zip_file_header* header = NULL;
     do {
       header = zip_file_next_file(myZip);
-    }
-    while(header && header->uncompressed_length == 0);
 
-    return header ? header->filename : EmptyString;
+      // Ignore zero-length files and '__MACOSX' virtual directories
+      valid = header && (header->uncompressed_length > 0) &&
+              !BSPF_startsWithIgnoreCase(header->filename, "__MACOSX");
+    }
+    while(!valid && myZip->cd_pos < myZip->ecd.cd_size);
+
+    return valid ? header->filename : EmptyString;
   }
   else
     return EmptyString;
@@ -246,6 +251,16 @@ ZipHandler::zip_error ZipHandler::zip_file_open(const char *filename, zip_file *
   newzip->filename = string;
   *zip = newzip;
 
+  // Count ROM files (we do it at this level so it will be cached)
+  while(hasNext())
+  {
+    const std::string& file = next();
+    if(BSPF_endsWithIgnoreCase(file, ".a26") ||
+       BSPF_endsWithIgnoreCase(file, ".bin") ||
+       BSPF_endsWithIgnoreCase(file, ".rom"))
+      (*zip)->romfiles++;
+  }
+
   return ZIPERR_NONE;
 
 error:

src/common/ZipHandler.hxx

@@ -81,7 +81,11 @@ class ZipHandler
 
     // Decompress the currently selected file, return false on any errors
     bool decompress(uInt8*& image, uInt32& length);
-    
+
+    // Answer the number of ROM files found in the archive
+    // Currently, this means files with extension a26/bin/rom
+    uInt16 romFiles() const { return myZip ? myZip->romfiles : 0; }
+
   private:
     // Replaces functionaity of various osd_xxxx functions
     static bool stream_open(const char* filename, fstream** stream, uInt64& length);
@@ -153,6 +157,7 @@ class ZipHandler
       const char*     filename;   /* copy of ZIP filename (for caching) */
       fstream*        file;       /* C++ fstream file handle */
       uInt64          length;     /* length of zip file */
+      uInt16          romfiles;   /* number of ROM files in central directory */
 
       zip_ecd         ecd;        /* end of central directory */
 

src/common/bspf.hxx

@@ -169,9 +169,12 @@ inline bool BSPF_startsWithIgnoreCase(const char* s1, const char* s2)
 // Test whether the first string ends with the second one (case insensitive)
 inline bool BSPF_endsWithIgnoreCase(const string& s1, const string& s2)
 {
-  return (s1.length() >= s2.length()) ?
+  if(s1.length() >= s2.length())
-      (BSPF_findIgnoreCase(s1, s2, s1.length() - s2.length()) != string::npos) :
+  {
-      false;
+    const char* end = s1.c_str() + s1.length() - s2.length();
+    return BSPF_equalsIgnoreCase(end, s2.c_str());
+  }
+  return false;
 }
 
 // Test whether the first string contains the second one (case insensitive)

src/gui/LauncherDialog.cxx

@@ -342,14 +342,12 @@ void LauncherDialog::updateListing(const string& nameToSelect)
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 void LauncherDialog::loadDirListing()
 {
-  if(!myCurrentNode.isDirectory() && !myCurrentNode.exists())
+  if(!myCurrentNode.isDirectory())
     return;
 
   FSList files;
   files.reserve(2048);
-
+  myCurrentNode.getChildren(files, FilesystemNode::kListAll);
-  if(myCurrentNode.isDirectory())
-    myCurrentNode.getChildren(files, FilesystemNode::kListAll);
 
   // Add '[..]' to indicate previous folder
   if(myCurrentNode.hasParent())
@@ -480,50 +478,6 @@ bool LauncherDialog::matchPattern(const string& s, const string& pattern) const
   return false;
 }
 
-#if 0
-// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-int LauncherDialog::filesInArchive(const string& archive)
-{
-  int count = -1;
-
-  unzFile tz;
-  if((tz = unzOpen(archive.c_str())) != NULL)
-  {
-    count = 0;
-    if(unzGoToFirstFile(tz) == UNZ_OK)
-    {
-      unz_file_info ufo;
-
-      for(;;)  // Loop through all files for valid 2600 images
-      {
-        // Longer filenames might be possible, but I don't
-        // think people would name files that long in zip files...
-        char filename[1024];
-
-        unzGetCurrentFileInfo(tz, &ufo, filename, 1024, 0, 0, 0, 0);
-        filename[1023] = '\0';
-
-        if(strlen(filename) >= 4 &&
-           !BSPF_startsWithIgnoreCase(filename, "__MACOSX"))
-        {
-          // Grab 3-character extension
-          const char* ext = filename + strlen(filename) - 4;
-
-          if(BSPF_equalsIgnoreCase(ext, ".a26") || BSPF_equalsIgnoreCase(ext, ".bin") ||
-             BSPF_equalsIgnoreCase(ext, ".rom"))
-            ++count;
-        }
-
-        // Scan the next file in the zip
-        if(unzGoToNextFile(tz) != UNZ_OK)
-          break;
-      }
-    }
-  }
-  return count;
-}
-#endif
-
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 void LauncherDialog::handleKeyDown(StellaKey key, StellaMod mod, char ascii)
 {
@@ -563,17 +517,16 @@ void LauncherDialog::handleCommand(CommandSender* sender, int cmd,
       {
         const FilesystemNode romnode(myGameList->path(item));
 
-        int numFilesInArchive = -1;//filesInArchive(rom);
+        // If a node isn't a file or directory, there's nothing we can
-        bool isArchive = false;//!myGameList->isDir(item) && BSPF_endsWithIgnoreCase(rom, ".zip");
+        // do with it
-
+        if(!romnode.isDirectory() && !romnode.isFile())
-        // Directory's should be selected (ie, enter them and redisplay)
-        // Archives should be entered if they contain more than 1 file
-        if(isArchive && numFilesInArchive < 1)
         {
-          instance().frameBuffer().showMessage("Archive does not contain any valid ROM files",
+          instance().frameBuffer().showMessage(
-                                               kMiddleCenter, true);
+            "Invalid file or doesn't contain any valid ROM files",
+            kMiddleCenter, true);
         }
-        else if(/*(isArchive && numFilesInArchive > 1) ||*/ romnode.isDirectory())
+        // Directory's should be selected (ie, enter them and redisplay)
+        else if(romnode.isDirectory())
         {
           string dirname = "";
           if(myGameList->name(item) == " [..]")