fix crash on exit editor

move management of Song resources out of Song and into SongManager

stepmania/src/ScreenEdit.cpp

@@ -1336,7 +1336,7 @@ void ScreenEdit::HandleScreenMessage( const ScreenMessage SM )
 	{
 	case SM_GoToNextScreen:
 		// Reload song from disk to discard changes.
-		GAMESTATE->m_pCurSong->RevertFromDisk( true );
+		SONGMAN->RevertFromDisk( GAMESTATE->m_pCurSong, true );
 		/* We might do something with m_pSteps (eg. UpdateTextInfo) before we end up
 		 * in ScreenEditMenu, and m_pSteps might be invalid due to RevertFromDisk. */
 		m_pSteps = GAMESTATE->m_pCurSteps[PLAYER_1];
@@ -1411,7 +1411,7 @@ void ScreenEdit::HandleScreenMessage( const ScreenMessage SM )
 		id.FromSteps( m_pSteps );
 
 		GAMESTATE->m_pCurSteps[PLAYER_1] = NULL; /* make RevertFromDisk not try to reset it */
-		GAMESTATE->m_pCurSong->RevertFromDisk();
+		SONGMAN->RevertFromDisk( GAMESTATE->m_pCurSong );
 
 		CString sMessage = "Reloaded from disk.";
 		Steps *pSteps = id.ToSteps( GAMESTATE->m_pCurSong, false );

stepmania/src/ScreenGameplay.cpp

@@ -1831,11 +1831,7 @@ void RevertChanges( void* papSongsQueue )
 	vector<Song*>& apSongsQueue = *(vector<Song*>*)papSongsQueue;
 	FOREACH( Song*, apSongsQueue, pSong )
 	{
-		(*pSong)->RevertFromDisk();
+		SONGMAN->RevertFromDisk( *pSong );
-
-		// We need to regen any Courses that have any of the songs we just reloaded.
-		// Regen all Courses for now.
-		SONGMAN->Invalidate( *pSong );
 	}
 }
 

stepmania/src/Song.cpp

@@ -24,8 +24,6 @@
 #include "RageSurface.h"
 #include "NoteDataUtil.h"
 #include "ProfileManager.h"
-#include "StageStats.h"
-#include "StepsUtil.h"
 #include "Foreach.h"
 
 #include "NotesLoaderSM.h"
@@ -103,13 +101,6 @@ void Song::Reset()
 
 	Song empty;
 	*this = empty;
-
-	/* Courses cache Steps pointers.  On the off chance that this isn't the last
-	 * thing this screen does, clear that cache. */
-	/* TODO: Don't make Song depend on SongManager.  This is breaking 
-	 * encapsulation and placing confusing limitation on what can be done in 
-	 * SONGMAN->Invalidate(). -Chris */
-	SONGMAN->Invalidate( this );
 }
 
 
@@ -292,72 +283,6 @@ bool Song::LoadFromSongDir( CString sDir )
 		return true;	// do load this song
 }
 
-
-/* If bAllowNotesLoss is true, any global notes pointers which no longer exist
- * (or exist but couldn't be matched) will be set to NULL.  This is used when
- * reverting out of the editor.  If false, this is unexpected and will assert.
- * This is used when reverting out of gameplay, in which case we may have StageStats,
- * etc. which may cause hard-to-trace crashes down the line if we set them to NULL. */
-void Song::RevertFromDisk( bool bAllowNotesLoss )
-{
-	// Ugly:  When we re-load the song, the Steps* will change.
-	// Fix GAMESTATE->m_CurSteps, g_CurStageStats, g_vPlayedStageStats[] after reloading.
-	/* XXX: This is very brittle.  However, we must know about all globals uses of Steps*,
-	 * so we can check to make sure we didn't lose any steps which are referenced ... */
-	StepsID OldCurSteps[NUM_PLAYERS];
-	StepsID OldCurStageStats[NUM_PLAYERS];
-	vector<StepsID> OldPlayedStageStats[NUM_PLAYERS];
-	FOREACH_PlayerNumber( p )
-	{
-		Steps* pCurSteps = GAMESTATE->m_pCurSteps[p];
-		Steps* pCurStageStats = g_CurStageStats.pSteps[p];
-
-		OldCurSteps[p].FromSteps( pCurSteps );
-		OldCurStageStats[p].FromSteps( pCurStageStats );
-		for( unsigned i = 0; i < g_vPlayedStageStats.size(); ++i )
-		{
-			const StageStats &ss = g_vPlayedStageStats[i];;
-			OldPlayedStageStats[p].push_back( StepsID() );
-			OldPlayedStageStats[p][i].FromSteps( ss.pSteps[p] );
-		}
-	}
-
-	const CString dir = GetSongDir();
-
-	/* Erase all existing data. */
-	Reset();
-
-	FILEMAN->FlushDirCache( dir );
-
-	const bool OldVal = PREFSMAN->m_bFastLoad;
-	PREFSMAN->m_bFastLoad = false;
-
-	LoadFromSongDir( dir );	
-	/* XXX: reload edits? */
-
-	PREFSMAN->m_bFastLoad = OldVal;
-
-	FOREACH_PlayerNumber( p )
-	{
-		CHECKPOINT;
-		if( GAMESTATE->m_pCurSong == this )
-			GAMESTATE->m_pCurSteps[p] = OldCurSteps[p].ToSteps( this, bAllowNotesLoss );
-		CHECKPOINT;
-		if( g_CurStageStats.pSong == this )
-			g_CurStageStats.pSteps[p] = OldCurStageStats[p].ToSteps( this, bAllowNotesLoss );
-		CHECKPOINT;
-		for( unsigned i = 0; i < g_vPlayedStageStats.size(); ++i )
-		{
-			CHECKPOINT_M(ssprintf("%i", i));
-			if( g_vPlayedStageStats[i].pSong == this )
-				g_vPlayedStageStats[i].pSteps[p] = OldPlayedStageStats[p][i].ToSteps( this, bAllowNotesLoss );
-		}
-	}
-
-	StepsID::Invalidate( this );
-}
-
-
 static void GetImageDirListing( CString sPath, CStringArray &AddTo, bool bReturnPathToo=false )
 {
 	GetDirListing( sPath + ".png", AddTo, false, bReturnPathToo ); 

stepmania/src/SongManager.cpp

@@ -29,6 +29,7 @@
 #include "UnlockSystem.h"
 #include "CatalogXml.h"
 #include "Foreach.h"
+#include "StageStats.h"
 
 SongManager*	SONGMAN = NULL;	// global and accessable from anywhere in our program
 
@@ -677,6 +678,74 @@ void SongManager::Invalidate( Song *pStaleSong )
 	StepsID::Invalidate( pStaleSong );
 }
 
+/* If bAllowNotesLoss is true, any global notes pointers which no longer exist
+ * (or exist but couldn't be matched) will be set to NULL.  This is used when
+ * reverting out of the editor.  If false, this is unexpected and will assert.
+ * This is used when reverting out of gameplay, in which case we may have StageStats,
+ * etc. which may cause hard-to-trace crashes down the line if we set them to NULL. */
+void SongManager::RevertFromDisk( Song *pSong, bool bAllowNotesLoss )
+{
+	// Ugly:  When we re-load the song, the Steps* will change.
+	// Fix GAMESTATE->m_CurSteps, g_CurStageStats, g_vPlayedStageStats[] after reloading.
+	/* XXX: This is very brittle.  However, we must know about all globals uses of Steps*,
+	 * so we can check to make sure we didn't lose any steps which are referenced ... */
+	StepsID OldCurSteps[NUM_PLAYERS];
+	StepsID OldCurStageStats[NUM_PLAYERS];
+	vector<StepsID> OldPlayedStageStats[NUM_PLAYERS];
+	FOREACH_PlayerNumber( p )
+	{
+		Steps* pCurSteps = GAMESTATE->m_pCurSteps[p];
+		Steps* pCurStageStats = g_CurStageStats.pSteps[p];
+
+		OldCurSteps[p].FromSteps( pCurSteps );
+		OldCurStageStats[p].FromSteps( pCurStageStats );
+		for( unsigned i = 0; i < g_vPlayedStageStats.size(); ++i )
+		{
+			const StageStats &ss = g_vPlayedStageStats[i];;
+			OldPlayedStageStats[p].push_back( StepsID() );
+			OldPlayedStageStats[p][i].FromSteps( ss.pSteps[p] );
+		}
+	}
+
+	const CString dir = pSong->GetSongDir();
+	FILEMAN->FlushDirCache( dir );
+
+	/* Erase existing data and reload. */
+	pSong->Reset();
+	const bool OldVal = PREFSMAN->m_bFastLoad;
+	PREFSMAN->m_bFastLoad = false;
+	pSong->LoadFromSongDir( dir );	
+	/* XXX: reload edits? */
+	PREFSMAN->m_bFastLoad = OldVal;
+
+
+	/* Courses cache Steps pointers.  On the off chance that this isn't the last
+	 * thing this screen does, clear that cache. */
+	/* TODO: Don't make Song depend on SongManager.  This is breaking 
+	 * encapsulation and placing confusing limitation on what can be done in 
+	 * SONGMAN->Invalidate(). -Chris */
+	this->Invalidate( pSong );
+	StepsID::Invalidate( pSong );
+
+
+	FOREACH_PlayerNumber( p )
+	{
+		CHECKPOINT;
+		if( GAMESTATE->m_pCurSong == pSong )
+			GAMESTATE->m_pCurSteps[p] = OldCurSteps[p].ToSteps( pSong, bAllowNotesLoss );
+		CHECKPOINT;
+		if( g_CurStageStats.pSong == pSong )
+			g_CurStageStats.pSteps[p] = OldCurStageStats[p].ToSteps( pSong, bAllowNotesLoss );
+		CHECKPOINT;
+		for( unsigned i = 0; i < g_vPlayedStageStats.size(); ++i )
+		{
+			CHECKPOINT_M(ssprintf("%i", i));
+			if( g_vPlayedStageStats[i].pSong == pSong )
+				g_vPlayedStageStats[i].pSteps[p] = OldPlayedStageStats[p][i].ToSteps( pSong, bAllowNotesLoss );
+		}
+	}
+}
+
 void SongManager::RegenerateNonFixedCourses()
 {
 	for( unsigned i=0; i < m_pCourses.size(); i++ )

stepmania/src/SongManager.h

@@ -27,6 +27,7 @@ public:
 	void Cleanup();
 
 	void Invalidate( Song *pStaleSong );
+	void RevertFromDisk( Song *pSong, bool bAllowNotesLoss=false );
 
 	void RegenerateNonFixedCourses();
 	void SetPreferences();

stepmania/src/song.h

@@ -61,7 +61,6 @@ public:
 	NotesLoader *MakeLoader( CString sDir ) const;
 
 	bool LoadFromSongDir( CString sDir );
-	void RevertFromDisk( bool bAllowNotesLoss=false );
 
 	void TidyUpData();	// call after loading to clean up invalid data
 	void ReCalculateRadarValuesAndLastBeat();	// called by TidyUpData, and after saving