From 13dd1a9bda2c0381c4d248621a73eca4602fcde2 Mon Sep 17 00:00:00 2001 From: Chris Danford Date: Wed, 11 Aug 2004 08:23:14 +0000 Subject: [PATCH] fix crash on exit editor move management of Song resources out of Song and into SongManager --- stepmania/src/ScreenEdit.cpp | 4 +- stepmania/src/ScreenGameplay.cpp | 6 +-- stepmania/src/Song.cpp | 75 -------------------------------- stepmania/src/SongManager.cpp | 69 +++++++++++++++++++++++++++++ stepmania/src/SongManager.h | 1 + stepmania/src/song.h | 1 - 6 files changed, 73 insertions(+), 83 deletions(-) diff --git a/stepmania/src/ScreenEdit.cpp b/stepmania/src/ScreenEdit.cpp index 104177ad84..79517e5f14 100644 --- a/stepmania/src/ScreenEdit.cpp +++ b/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 ); diff --git a/stepmania/src/ScreenGameplay.cpp b/stepmania/src/ScreenGameplay.cpp index fea1ab4b6e..e0363d65ed 100644 --- a/stepmania/src/ScreenGameplay.cpp +++ b/stepmania/src/ScreenGameplay.cpp @@ -1831,11 +1831,7 @@ void RevertChanges( void* papSongsQueue ) vector& apSongsQueue = *(vector*)papSongsQueue; FOREACH( Song*, apSongsQueue, pSong ) { - (*pSong)->RevertFromDisk(); - - // We need to regen any Courses that have any of the songs we just reloaded. - // Regen all Courses for now. - SONGMAN->Invalidate( *pSong ); + SONGMAN->RevertFromDisk( *pSong ); } } diff --git a/stepmania/src/Song.cpp b/stepmania/src/Song.cpp index fe67fdc3cf..84ba1ed2a2 100644 --- a/stepmania/src/Song.cpp +++ b/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 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 ); diff --git a/stepmania/src/SongManager.cpp b/stepmania/src/SongManager.cpp index 1c830a6a55..fb437c7f81 100644 --- a/stepmania/src/SongManager.cpp +++ b/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 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++ ) diff --git a/stepmania/src/SongManager.h b/stepmania/src/SongManager.h index 61ea8fb812..1dc428d05c 100644 --- a/stepmania/src/SongManager.h +++ b/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(); diff --git a/stepmania/src/song.h b/stepmania/src/song.h index 00fcdb3b5a..27f21337b6 100644 --- a/stepmania/src/song.h +++ b/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