Fix SongManager::Invalidate. Call it from places it is needed:

ScreenEdit::RevertFromDisk, ScreenEdit's Save routine, and
ScreenEditMenu's Delete Steps routine.  This fixes the bug where
deleting a step chart sometimes causes interesting behavior
but usually just causes a CTD next time you run Oni mode.  It also makes
sure data precalculated from the songs is correctly recalculated when
ScreenEdit changes a song.

stepmania/src/Course.cpp

@@ -192,24 +192,33 @@ RString Course::GetCacheFilePath() const
 void Course::Init()
 {
 	m_bIsAutogen = false;
-	m_bRepeat = false;
-	m_bShuffle = false;
-	m_iLives = -1;
-	m_bSortByMeter = false;
-	m_vEntries.clear();
-	FOREACH_Difficulty(dc)
-		m_iCustomMeter[dc] = -1;
-	m_vEntries.clear();
 	m_sPath = "";
-	m_sGroupName = "";
+
 	m_sMainTitle = "";
 	m_sMainTitleTranslit = "";
 	m_sSubTitle = "";
 	m_sSubTitleTranslit = "";
+
 	m_sBannerPath = "";
 	m_sCDTitlePath = "";
+	m_sGroupName = "";
+
+	m_bRepeat = false;
+	m_bShuffle = false;
+	m_iLives = -1;
+	FOREACH_Difficulty(dc)
+		m_iCustomMeter[dc] = -1;
+	m_bSortByMeter = false;
+
+	m_vEntries.clear();
+
+	m_SortOrder_TotalDifficulty = 0;
+	m_SortOrder_Ranking = 0;
+
 	m_LoadedFromProfile = ProfileSlot_Invalid;
+	m_TrailCache.clear();
 	m_iTrailCacheSeed = 0;
+	m_RadarCache.clear();
 }
 
 bool Course::IsPlayableIn( StepsType st ) const

stepmania/src/ScreenEdit.cpp

@@ -2583,6 +2583,7 @@ void ScreenEdit::HandleScreenMessage( const ScreenMessage SM )
 		m_pSteps->SetSavedToDisk( true );
 		CopyToLastSave();
 		SetDirty( false );
+		SONGMAN->Invalidate( GAMESTATE->m_pCurSong );
 
 		if( m_CurrentAction == save_on_exit )
 			ScreenPrompt::Prompt( SM_DoExit, SAVE_SUCCESSFUL );
@@ -2605,6 +2606,8 @@ void ScreenEdit::HandleScreenMessage( const ScreenMessage SM )
 		// If these steps have never been saved, then we should delete them.
 		// If the user created them in the edit menu and never bothered
 		// to save them, then they aren't wanted.
+		// FIXME: This logic fails if the user starts a new steps, changes to 
+		// a different stepchart, and then exits without saving
 		Steps* pSteps = GAMESTATE->m_pCurSteps[PLAYER_1];
 		if( !pSteps->GetSavedToDisk() )
 		{
@@ -3481,7 +3484,8 @@ void ScreenEdit::RevertFromDisk()
 	m_pSteps = pNewSteps;
 
 	CopyToLastSave();
-	SetDirty(false);
+	SetDirty( false );
+	SONGMAN->Invalidate( GAMESTATE->m_pCurSong );
 }
 
 void ScreenEdit::SaveUndo()

stepmania/src/ScreenEditMenu.cpp

@@ -74,6 +74,7 @@ void ScreenEditMenu::HandleScreenMessage( const ScreenMessage SM )
 		Steps* pStepsToDelete = GAMESTATE->m_pCurSteps[PLAYER_1];
 		bool bSaveSong = !pStepsToDelete->WasLoadedFromProfile();
 		pSong->DeleteSteps( pStepsToDelete );
+		SONGMAN->Invalidate( pSong );
 
 		/* Only save to the main .SM file if the steps we're deleting were loaded
 		 * from it. */

stepmania/src/SongManager.cpp

@@ -859,105 +859,20 @@ void SongManager::Cleanup()
 	}
 }
 
-/* Flush all Song*, Steps* and Course* caches.  This is called on reload, and when
+/* Flush all Song*, Steps* and Course* caches.  This is when a Song or its Steps
- * any of those are removed or changed.  This doesn't touch GAMESTATE and StageStats
+ * are removed or changed.  This doesn't touch GAMESTATE and StageStats
- * pointers, which are updated explicitly in Song::RevertFromDisk. */
+ * pointers.  Currently, the only time Steps are altered independantly of the
+ * Courses and Songs is in Edit Mode, which updates the other pointers it needs. */
 void SongManager::Invalidate( Song *pStaleSong )
 {
-	//
+	FOREACH( Course*, this->m_pCourses, pCourse )
-	// Save list of all old Course and Trail pointers
-	//
-	map<Course*,CourseID> mapOldCourseToCourseID;
-	typedef pair<TrailID,Course*> TrailIDAndCourse;
-	map<Trail*,TrailIDAndCourse> mapOldTrailToTrailIDAndCourse;
-	FOREACH_CONST( Course*, this->m_pCourses, pCourse )
 	{
-		CourseID id;
+		(*pCourse)->Invalidate( pStaleSong );
-		id.FromCourse( *pCourse );
-		mapOldCourseToCourseID[*pCourse] = id;
-		vector<Trail *> Trails;
-		(*pCourse)->GetAllCachedTrails( Trails );
-		FOREACH_CONST( Trail*, Trails, pTrail )
-		{
-			TrailID id;
-			id.FromTrail( *pTrail );
-			mapOldTrailToTrailIDAndCourse[*pTrail] = TrailIDAndCourse(id, *pCourse);
-		}
 	}
 
-	// It's a real pain to selectively invalidate only those Courses with 
+	UpdatePopular();
-	// dependencies on the stale Song.  So, instead, just reload all Courses.
+	UpdateShuffled();
-	// It doesn't take very long.
+	RefreshCourseGroupInfo();
-	FreeCourses();
-	InitCoursesFromDisk( NULL );
-	InitAutogenCourses();
-
-	// invalidate cache
-	StepsID::ClearCache();
-
-#define CONVERT_COURSE_POINTER( pCourse ) do { \
-	CourseID id = mapOldCourseToCourseID[pCourse]; /* this will always succeed */ \
-	pCourse = id.ToCourse(); \
-} while(false)
-
-	/* Ugly: We need the course pointer to restore a trail pointer, and both have
-	 * been invalidated.  We need to go through our mapping, and update the course
-	 * pointers, so we can use that to update trail pointers.  */
-	{
-		map<Trail*,TrailIDAndCourse>::iterator it;
-		for( it = mapOldTrailToTrailIDAndCourse.begin(); it != mapOldTrailToTrailIDAndCourse.end(); ++it )
-		{
-			TrailIDAndCourse &tidc = it->second;
-			CONVERT_COURSE_POINTER( tidc.second );
-		}
-	}
-
-	{
-		CourseID id = mapOldCourseToCourseID[GAMESTATE->m_pCurCourse]; /* this will always succeed */
-		GAMESTATE->m_pCurCourse.Set( id.ToCourse() );
-	}
-	CONVERT_COURSE_POINTER( GAMESTATE->m_pPreferredCourse );
-
-#define CONVERT_TRAIL_POINTER( pTrail ) do { \
-	if( pTrail != NULL ) { \
-		map<Trail*,TrailIDAndCourse>::iterator it; \
-		it = mapOldTrailToTrailIDAndCourse.find(pTrail); \
-		ASSERT_M( it != mapOldTrailToTrailIDAndCourse.end(), ssprintf("%p", pTrail.Get()) ); \
-		const TrailIDAndCourse &tidc = it->second; \
-		const TrailID &id = tidc.first; \
-		const Course *pCourse = tidc.second; \
-		pTrail.Set( id.ToTrail( pCourse, true ) ); \
-	} \
-} while(false)
-
-	FOREACH_PlayerNumber( pn )
-	{
-		CONVERT_TRAIL_POINTER( GAMESTATE->m_pCurTrail[pn] );
-	}
-}
-
-/* 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 CONVERT_STEPS_POINTER( Steps *&pSteps, const map<Steps*,StepsID> &mapOldStepsToStepsID, const Song *pSong, bool bAllowNotesLoss )
-{
-	if( pSteps == NULL )
-		return;
-
-	map<Steps*,StepsID>::const_iterator it = mapOldStepsToStepsID.find(pSteps);
-	if( it != mapOldStepsToStepsID.end() )
-		pSteps = it->second.ToSteps(pSong, bAllowNotesLoss);
-}
-void CONVERT_STEPS_POINTER( BroadcastOnChangePtr<Steps> &pSteps, const map<Steps*,StepsID> &mapOldStepsToStepsID, const Song *pSong, bool bAllowNotesLoss )
-{
-	if( pSteps == NULL )
-		return;
-
-	map<Steps*,StepsID>::const_iterator it = mapOldStepsToStepsID.find(pSteps);
-	if( it != mapOldStepsToStepsID.end() )
-		pSteps.Set( it->second.ToSteps(pSong, bAllowNotesLoss) );
 }
 
 void SongManager::RegenerateNonFixedCourses()