simplify and clarify error handling

stepmania/src/RageSoundReader_FileReader.cpp

@@ -16,7 +16,7 @@
 #include "RageSoundReader_Vorbisfile.h"
 #endif
 
-SoundReader_FileReader *SoundReader_FileReader::TryOpenFile( CString filename, CString &error, CString format )
+SoundReader_FileReader *SoundReader_FileReader::TryOpenFile( CString filename, CString &error, CString format, bool &bKeepTrying )
 {
 	SoundReader_FileReader *Sample = NULL;
 
@@ -46,13 +46,37 @@ SoundReader_FileReader *SoundReader_FileReader::TryOpenFile( CString filename, C
 
 	LOG->Trace( "Format %s failed: %s", format.c_str(), err.c_str() );
 
-	/* If OPEN_MATCH_BUT_FAIL, the error is important; otherwise it's probably
+	/*
-		* just a "unknown file"-ish error. */
+	 * The file failed to open, or failed to read.  This indicates a problem that will
-	if( ret == OPEN_MATCH_BUT_FAIL )
+	 * affect all readers, so don't waste time trying more readers. (OPEN_IO_ERROR)
+	 *
+	 * Errors fall in two categories:
+	 * OPEN_UNKNOWN_FILE_FORMAT: Data was successfully read from the file, but it's the
+	 * wrong file format.  The error message always looks like "unknown file format" or
+	 * "Not Vorbis data"; ignore it so we always give a consistent error message, and
+	 * continue trying other file formats.
+	 * 
+	 * OPEN_FATAL_ERROR: Either the file was opened successfully and appears to be the
+	 * correct format, but a fatal format-specific error was encountered that will probably
+	 * not be fixed by using a different reader (for example, an Ogg file that doesn't
+	 * actually contain any audio streams); or the file failed to open or read ("I/O
+	 * error", "permission denied"), in which case all other readers will probably fail,
+	 * too.  The returned error is used, and no other formats will be tried.
+	 */
+	bKeepTrying = (ret == OPEN_MATCH_BUT_FAIL);
+	switch( ret )
 	{
-		if( error != "" )
+	case OPEN_UNKNOWN_FILE_FORMAT:
-			error += "; ";
+		bKeepTrying = true;
-		error += ssprintf("%s: %s", format.c_str(), err.c_str() );
+		error = "Unknown file format";
+		break;
+
+	case OPEN_FATAL_ERROR:
+		/* The file matched, but failed to load.  We know it's this type of data;
+		 * don't bother trying the other file types. */
+		bKeepTrying = false;
+		error = err;
+		break;
 	}
 
 	return NULL;
@@ -79,18 +103,20 @@ SoundReader *SoundReader_FileReader::OpenFile( CString filename, CString &error
 
 	error = "";
 
+	bool bKeepTrying = true;
+
 	/* If the extension matches a format, try that first. */
 	if( FileTypes.find(format) != FileTypes.end() )
 	{
-	    SoundReader_FileReader *NewSample = TryOpenFile( filename, error, format );
+	    SoundReader_FileReader *NewSample = TryOpenFile( filename, error, format, bKeepTrying );
 		if( NewSample )
 			return NewSample;
 		FileTypes.erase( format );
 	}
 
-	for( set<CString>::iterator it = FileTypes.begin(); it != FileTypes.end(); ++it )
+	for( set<CString>::iterator it = FileTypes.begin(); bKeepTrying && it != FileTypes.end(); ++it )
 	{
-	    SoundReader_FileReader *NewSample = TryOpenFile( filename, error, *it );
+	    SoundReader_FileReader *NewSample = TryOpenFile( filename, error, *it, bKeepTrying );
 		if( NewSample )
 		{
 			LOG->Warn("File \"%s\" is really %s", filename.c_str(), it->c_str());
@@ -98,6 +124,5 @@ SoundReader *SoundReader_FileReader::OpenFile( CString filename, CString &error
 		}
 	}
 
-	error = "Unknown file format";
 	return NULL;
 }

stepmania/src/RageSoundReader_FileReader.h

@@ -12,12 +12,19 @@ public:
 	 * the file.
 	 *
 	 * If the file can not be opened at all, or contains no data, return OPEN_MATCH_BUT_FAIL. */
-	enum OpenResult { OPEN_OK, OPEN_NO_MATCH, OPEN_MATCH_BUT_FAIL };
+	enum OpenResult
+	{
+		OPEN_OK,
+		OPEN_UNKNOWN_FILE_FORMAT=1,
+		OPEN_NO_MATCH=1, // deprecated
+		OPEN_FATAL_ERROR=2,
+		OPEN_MATCH_BUT_FAIL=2, // deprecated
+	};
 	virtual OpenResult Open(CString filename) = 0;
 
 	static SoundReader *OpenFile( CString filename, CString &error );
 private:
-	static SoundReader_FileReader *TryOpenFile( CString filename, CString &error, CString format );
+	static SoundReader_FileReader *TryOpenFile( CString filename, CString &error, CString format, bool &bKeepTrying );
 };
 
 #endif