From 6cf23c4dc3be463d602f77159d25ecbb317329b3 Mon Sep 17 00:00:00 2001 From: Kyzentun Date: Mon, 14 Jul 2014 15:41:04 -0600 Subject: [PATCH] Changed many parts of GameCommand to improve error reporting. Fixed GameCommand setpref to not set when loading the command. Minor fix to ScreenOptionsExample.ini to explain that SongInCurrentSongGroup is unusable. CommonMetrics, CourseContentsList, HoldJudgent, OptionRowHandler all have minor changes to fix crashes on malformed theme data. --- .../Example_Screens/ScreenOptionsExample.ini | 9 +- src/BitmapText.cpp | 19 +- src/CommonMetrics.cpp | 12 +- src/CourseContentsList.cpp | 1 + src/GameCommand.cpp | 163 +++++++++++------- src/GameCommand.h | 3 +- src/HoldJudgment.cpp | 2 +- src/OptionRowHandler.cpp | 19 +- src/ScreenManager.cpp | 6 + src/ScreenManager.h | 2 + 10 files changed, 147 insertions(+), 89 deletions(-) diff --git a/Docs/Themerdocs/Examples/Example_Screens/ScreenOptionsExample.ini b/Docs/Themerdocs/Examples/Example_Screens/ScreenOptionsExample.ini index b373987f2c..39e4a0032e 100644 --- a/Docs/Themerdocs/Examples/Example_Screens/ScreenOptionsExample.ini +++ b/Docs/Themerdocs/Examples/Example_Screens/ScreenOptionsExample.ini @@ -32,7 +32,7 @@ InputMode="individual" # LineNames sets the names of the rows on the screen. # Names are separated by commas and can be anything that doesn't contain a comma. # Each name will be appended to "Line" to find the metric that defines that line. So a line name of "a" will be defined by the metric "Linea". -LineNames="a,b,c,d,e,f,g,h,i,j,k,l,m,n" +LineNames="a,b,c,d,e,f,g,h,j,k,l,m,n" # Each line is defined by a metric that sets its type and arguments for that type. # This example is going to include a simple example of each type. @@ -71,7 +71,8 @@ Lineg="list,Groups" Lineh="list,Difficulties" # The "SongsInCurrentSongGroup" subtype -Linei="list,SongsInCurrentSongGroup" +# This option row type is not actually usable because it requires language file entries for every song, which is impractical. +#Linei="list,SongsInCurrentSongGroup" # This is the end of the list subtype examples. @@ -82,8 +83,8 @@ Linej="lua,ArbitrarySpeedMods()" # The "steps" row type creates choices for the different charts for the current song. It takes a single argument, "EditSteps" or "EditSourceSteps". Its purpose is currently unknown. Linek="steps,EditSteps" -# The "stepstype" row type is related to the "steps" row type and takes the same argument. Its purpose is also unknown. -Linel="stepstype,EditSteps" +# The "stepstype" row type is related to the "steps" row type. It takes a single argument, "EditStepsType" or "EditSourceStepsType". Its purpose is also unknown. +Linel="stepstype,EditStepsType" # The "conf" row type constructs a row for changing a conf option. conf options are the options in Preferences.ini and the argument is the name of the option to edit. Linem="conf,AllowW1" diff --git a/src/BitmapText.cpp b/src/BitmapText.cpp index 95ca19f83a..3fc7525637 100644 --- a/src/BitmapText.cpp +++ b/src/BitmapText.cpp @@ -126,18 +126,21 @@ void BitmapText::LoadFromNode( const XNode* pNode ) ThemeManager::EvaluateString( sAltText ); RString sFont; - if( !ActorUtil::GetAttrPath(pNode, "Font", sFont) && - !ActorUtil::GetAttrPath(pNode, "File", sFont) ) + // The short circuiting loading condition that was here before wasn't short circuiting correctly on all platforms. + if(!ActorUtil::GetAttrPath(pNode, "Font", sFont)) { - if( !pNode->GetAttrValue("Font", sFont) && - !pNode->GetAttrValue("File", sFont) ) // accept "File" for backward compatibility + if(!ActorUtil::GetAttrPath(pNode, "File", sFont)) { - LuaHelpers::ReportScriptErrorFmt( "%s: BitmapText: Font or File attribute not found", - ActorUtil::GetWhere(pNode).c_str() ); - sFont = "Common Normal"; + if( !pNode->GetAttrValue("Font", sFont) && + !pNode->GetAttrValue("File", sFont) ) // accept "File" for backward compatibility + { + LuaHelpers::ReportScriptErrorFmt( "%s: BitmapText: Font or File attribute not found", + ActorUtil::GetWhere(pNode).c_str() ); + sFont = "Common Normal"; + } } - sFont = THEME->GetPathF( "", sFont ); } + sFont = THEME->GetPathF( "", sFont ); LoadFromFont( sFont ); diff --git a/src/CommonMetrics.cpp b/src/CommonMetrics.cpp index c8752192d8..7ca1fa2569 100644 --- a/src/CommonMetrics.cpp +++ b/src/CommonMetrics.cpp @@ -38,7 +38,11 @@ void ThemeMetricDifficultiesToShow::Read() vector v; split( ThemeMetric::GetValue(), ",", v ); - ASSERT( v.size() > 0 ); + if(v.empty()) + { + LuaHelpers::ReportScriptError("DifficultiesToShow must have at least one entry."); + return; + } FOREACH_CONST( RString, v, i ) { @@ -73,7 +77,11 @@ void ThemeMetricCourseDifficultiesToShow::Read() vector v; split( ThemeMetric::GetValue(), ",", v ); - ASSERT( v.size() > 0 ); + if(v.empty()) + { + LuaHelpers::ReportScriptError("CourseDifficultiesToShow must have at least one entry."); + return; + } FOREACH_CONST( RString, v, i ) { diff --git a/src/CourseContentsList.cpp b/src/CourseContentsList.cpp index 8475b05547..43e76e1526 100644 --- a/src/CourseContentsList.cpp +++ b/src/CourseContentsList.cpp @@ -28,6 +28,7 @@ void CourseContentsList::LoadFromNode( const XNode* pNode ) if( pDisplayNode == NULL ) { LuaHelpers::ReportScriptErrorFmt("%s: CourseContentsList: missing the Display child", ActorUtil::GetWhere(pNode).c_str()); + return; } for( int i=0; iGameAndStringToStyle( GAMESTATE->m_pCurGame, sValue ); - if( style ) - m_pStyle = style; - else - m_bInvalid = true; + CHECK_INVALID_VALUE(m_pStyle, style, NULL, style); } else if( sName == "playmode" ) { PlayMode pm = StringToPlayMode( sValue ); - if( pm != PlayMode_Invalid ) - m_pm = pm; - else - m_bInvalid = true; + CHECK_INVALID_VALUE(m_pm, pm, PlayMode_Invalid, playmode); } else if( sName == "difficulty" ) { Difficulty dc = StringToDifficulty( sValue ); - if( dc != Difficulty_Invalid ) - m_dc = dc; - else - m_bInvalid = true; + CHECK_INVALID_VALUE(m_dc, dc, Difficulty_Invalid, difficulty); } else if( sName == "announcer" ) @@ -236,23 +245,20 @@ void GameCommand::LoadOne( const Command& cmd ) m_LuaFunction.SetFromExpression( sValue ); if(m_LuaFunction.IsNil()) { - LuaHelpers::ReportScriptError("Lua error in game command: \"" + sValue + "\" evaluated to nil"); + MAKE_INVALID("Lua error in game command: \"" + sValue + "\" evaluated to nil"); } } else if( sName == "screen" ) { - m_sScreen = sValue; + CHECK_INVALID_COND(m_sScreen, sValue, (!SCREENMAN->IsScreenNameValid(sValue)), ("Screen \"" + sValue + "\" has invalid class.")); } else if( sName == "song" ) { - m_pSong = SONGMAN->FindSong( sValue ); - if( m_pSong == NULL ) - { - m_sInvalidReason = ssprintf( "Song \"%s\" not found", sValue.c_str() ); - m_bInvalid = true; - } + CHECK_INVALID_COND(m_pSong, SONGMAN->FindSong(sValue), + (SONGMAN->FindSong(sValue) == NULL), + (ssprintf("Song \"%s\" not found", sValue.c_str()))); } else if( sName == "steps" ) @@ -266,34 +272,31 @@ void GameCommand::LoadOne( const Command& cmd ) const Style *pStyle = m_pStyle ? m_pStyle : GAMESTATE->GetCurrentStyle(); if( pSong == NULL || pStyle == NULL ) { - LuaHelpers::ReportScriptError("Must set Song and Style to set Steps."); - m_sInvalidReason = "Song or Style not set"; - m_bInvalid = true; + MAKE_INVALID("Must set Song and Style to set Steps."); } else { Difficulty dc = StringToDifficulty( sSteps ); - if( dc != Difficulty_Edit ) - m_pSteps = SongUtil::GetStepsByDifficulty( pSong, pStyle->m_StepsType, dc ); - else - m_pSteps = SongUtil::GetStepsByDescription( pSong, pStyle->m_StepsType, sSteps ); - if( m_pSteps == NULL ) + Steps* st; + if( dc < Difficulty_Edit ) { - m_sInvalidReason = "steps not found"; - m_bInvalid = true; + st = SongUtil::GetStepsByDifficulty( pSong, pStyle->m_StepsType, dc ); } + else + { + st = SongUtil::GetStepsByDescription( pSong, pStyle->m_StepsType, sSteps ); + } + CHECK_INVALID_COND(m_pSteps, st, (st == NULL), + (ssprintf("Steps \"%s\" not found", sSteps.c_str()))); } } } else if( sName == "course" ) { - m_pCourse = SONGMAN->FindCourse( "", sValue ); - if( m_pCourse == NULL ) - { - m_sInvalidReason = ssprintf( "Course \"%s\" not found", sValue.c_str() ); - m_bInvalid = true; - } + CHECK_INVALID_COND(m_pCourse, SONGMAN->FindCourse("", sValue), + (SONGMAN->FindCourse("", sValue) == NULL), + (ssprintf( "Course \"%s\" not found", sValue.c_str()))); } else if( sName == "trail" ) @@ -307,20 +310,20 @@ void GameCommand::LoadOne( const Command& cmd ) const Style *pStyle = m_pStyle ? m_pStyle : GAMESTATE->GetCurrentStyle(); if( pCourse == NULL || pStyle == NULL ) { - LuaHelpers::ReportScriptError("Must set Course and Style to set Trail."); - m_sInvalidReason = "Course or Style not set"; - m_bInvalid = true; + MAKE_INVALID("Must set Course and Style to set Trail."); } else { const CourseDifficulty cd = StringToDifficulty( sTrail ); - ASSERT_M( cd != Difficulty_Invalid, ssprintf("Invalid difficulty '%s'", sTrail.c_str()) ); - - m_pTrail = pCourse->GetTrail( pStyle->m_StepsType, cd ); - if( m_pTrail == NULL ) + if(cd == Difficulty_Invalid) { - m_sInvalidReason = "trail not found"; - m_bInvalid = true; + MAKE_INVALID(ssprintf("Invalid difficulty '%s'", sTrail.c_str())); + } + else + { + Trail* tr = pCourse->GetTrail(pStyle->m_StepsType, cd); + CHECK_INVALID_COND(m_pTrail, tr, (tr == NULL), + ("Trail \"" + sTrail + "\" not found.")); } } } @@ -328,23 +331,28 @@ void GameCommand::LoadOne( const Command& cmd ) else if( sName == "setenv" ) { - if( cmd.m_vsArgs.size() == 3 ) - m_SetEnv[ cmd.m_vsArgs[1] ] = cmd.m_vsArgs[2]; + if((cmd.m_vsArgs.size() - 1) % 2 != 0) + { + MAKE_INVALID("Arguments to setenv game command must be key,value pairs."); + } + else + { + for(size_t i= 1; i < cmd.m_vsArgs.size(); i+= 2) + { + m_SetEnv[cmd.m_vsArgs[i]]= cmd.m_vsArgs[i+1]; + } + } } else if( sName == "songgroup" ) { - m_sSongGroup = sValue; + CHECK_INVALID_COND(m_sSongGroup, sValue, (!SONGMAN->DoesSongGroupExist(sValue)), ("Song group \"" + sValue + "\" does not exist.")); } else if( sName == "sort" ) { - m_SortOrder = StringToSortOrder( sValue ); - if( m_SortOrder == SortOrder_Invalid ) - { - m_sInvalidReason = ssprintf( "SortOrder \"%s\" is not valid.", sValue.c_str() ); - m_bInvalid = true; - } + SortOrder so= StringToSortOrder(sValue); + CHECK_INVALID_VALUE(m_SortOrder, so, SortOrder_Invalid, sortorder); } else if( sName == "weight" ) @@ -359,7 +367,8 @@ void GameCommand::LoadOne( const Command& cmd ) else if( sName == "goaltype" ) { - m_GoalType = StringToGoalType( sValue ); + GoalType go= StringToGoalType(sValue); + CHECK_INVALID_VALUE(m_GoalType, go, GoalType_Invalid, goaltype); } else if( sName == "profileid" ) @@ -412,15 +421,23 @@ void GameCommand::LoadOne( const Command& cmd ) else if( sName == "setpref" ) { - if( cmd.m_vsArgs.size() == 3 ) + if((cmd.m_vsArgs.size() - 1) % 2 != 0) { - IPreference *pPref = IPreference::GetPreferenceByName( cmd.m_vsArgs[1] ); - if( pPref == NULL ) + MAKE_INVALID("Arguments to setpref game command must be key,value pairs."); + } + else + { + for(size_t i= 1; i < cmd.m_vsArgs.size(); i+= 2) { - m_sInvalidReason = ssprintf("unknown preference \"%s\"", cmd.m_vsArgs[1].c_str() ); - m_bInvalid = true; + if(IPreference::GetPreferenceByName(cmd.m_vsArgs[i]) == NULL) + { + MAKE_INVALID("Unknown preference \"" + cmd.m_vsArgs[i] + "\"."); + } + else + { + m_SetPref[cmd.m_vsArgs[i]]= cmd.m_vsArgs[i+1]; + } } - pPref->FromString(cmd.m_vsArgs[2]); } } @@ -432,13 +449,19 @@ void GameCommand::LoadOne( const Command& cmd ) m_fMusicFadeOutVolume = static_cast(atof( cmd.m_vsArgs[1] )); m_fMusicFadeOutSeconds = static_cast(atof( cmd.m_vsArgs[2] )); } + else + { + MAKE_INVALID("Wrong number of args to fademusic."); + } } else { - RString sWarning = ssprintf( "Command '%s' is not valid.", cmd.GetOriginalCommandString().c_str() ); - LuaHelpers::ReportScriptError(sWarning, "INVALID_GAME_COMMAND"); + MAKE_INVALID(ssprintf( "Command '%s' is not valid.", cmd.GetOriginalCommandString().c_str())); } +#undef CHECK_INVALID_VALUE +#undef CHECK_INVALID_COND +#undef MAKE_INVALID } int GetNumCreditsPaid() @@ -457,8 +480,8 @@ int GetCreditsRequiredToPlayStyle( const Style *style ) { // GameState::GetCoinsNeededToJoin returns 0 if the coin mode isn't // CoinMode_Pay, which means the theme can't make sure that there are - // enough credits available. GameCommand::IsPlayable will cause a crash - // if there aren't enough credits. So we have to check the coin mode here + // enough credits available. + // So we have to check the coin mode here // and return 0 if the player doesn't have to pay. if( GAMESTATE->GetCoinMode() != CoinMode_Pay ) { @@ -692,7 +715,7 @@ void GameCommand::ApplySelf( const vector &vpns ) const } break; default: - FAIL_M(ssprintf("Invalid StyleType: %i", m_pStyle->m_StyleType)); + LuaHelpers::ReportScriptError("Invalid StyleType: " + m_pStyle->m_StyleType); } } if( m_dc != Difficulty_Invalid ) @@ -754,6 +777,14 @@ void GameCommand::ApplySelf( const vector &vpns ) const lua_pop( L, 1 ); LUA->Release(L); } + for(map::const_iterator setting= m_SetPref.begin(); setting != m_SetPref.end(); ++setting) + { + IPreference* pref= IPreference::GetPreferenceByName(setting->first); + if(pref != NULL) + { + pref->FromString(setting->second); + } + } if( !m_sSongGroup.empty() ) GAMESTATE->m_sPreferredSongGroup.Set( m_sSongGroup ); if( m_SortOrder != SortOrder_Invalid ) diff --git a/src/GameCommand.h b/src/GameCommand.h index d0641292ba..cad1ae9c62 100644 --- a/src/GameCommand.h +++ b/src/GameCommand.h @@ -34,7 +34,7 @@ public: m_sAnnouncer(""), m_sPreferredModifiers(""), m_sStageModifiers(""), m_sScreen(""), m_LuaFunction(), m_pSong(NULL), m_pSteps(NULL), m_pCourse(NULL), - m_pTrail(NULL), m_pCharacter(NULL), m_SetEnv(), + m_pTrail(NULL), m_pCharacter(NULL), m_SetEnv(), m_SetPref(), m_sSongGroup(""), m_SortOrder(SortOrder_Invalid), m_sSoundPath(""), m_vsScreensToPrepare(), m_iWeightPounds(-1), m_iGoalCalories(-1), m_GoalType(GoalType_Invalid), @@ -92,6 +92,7 @@ public: Trail* m_pTrail; Character* m_pCharacter; std::map m_SetEnv; + std::map m_SetPref; RString m_sSongGroup; SortOrder m_SortOrder; RString m_sSoundPath; // "" for no sound diff --git a/src/HoldJudgment.cpp b/src/HoldJudgment.cpp index eefa50632f..26656d8120 100644 --- a/src/HoldJudgment.cpp +++ b/src/HoldJudgment.cpp @@ -27,7 +27,7 @@ void HoldJudgment::Load( const RString &sPath ) void HoldJudgment::LoadFromNode( const XNode* pNode ) { RString sFile; - if( ActorUtil::GetAttrPath(pNode, "File", sFile) ) + if(!ActorUtil::GetAttrPath(pNode, "File", sFile)) { LuaHelpers::ReportScriptErrorFmt("%s: HoldJudgment: missing the attribute \"File\"", ActorUtil::GetWhere(pNode).c_str()); } diff --git a/src/OptionRowHandler.cpp b/src/OptionRowHandler.cpp index cefa228f94..9d2893e2b4 100644 --- a/src/OptionRowHandler.cpp +++ b/src/OptionRowHandler.cpp @@ -434,7 +434,7 @@ class OptionRowHandlerListSteps : public OptionRowHandlerList m_Def.m_vsChoices.push_back( "" ); m_aListEntries.push_back( GameCommand() ); } - else if( GAMESTATE->IsCourseMode() && GAMESTATE->m_pCurCourse ) // playing a course + else if(GAMESTATE->GetCurrentStyle() && GAMESTATE->IsCourseMode() && GAMESTATE->m_pCurCourse) // playing a course { m_Def.m_bOneChoiceForAllPlayers = (bool)PREFSMAN->m_bLockCourseDifficulties; m_Def.m_layoutType = StringToLayoutType( STEPS_ROW_LAYOUT_TYPE ); @@ -453,7 +453,7 @@ class OptionRowHandlerListSteps : public OptionRowHandlerList m_aListEntries.push_back( mc ); } } - else if( GAMESTATE->m_pCurSong ) // playing a song + else if(GAMESTATE->GetCurrentStyle() && GAMESTATE->m_pCurSong) // playing a song { m_Def.m_layoutType = StringToLayoutType( STEPS_ROW_LAYOUT_TYPE ); @@ -634,26 +634,31 @@ public: { unsigned i = iter - m_vSteps.begin(); vbSelOut[i] = true; - return; + continue; } // look for matching difficulty + bool matched= false; if( m_pDifficultyToFill ) { FOREACH_CONST( Difficulty, m_vDifficulties, d ) { unsigned i = d - m_vDifficulties.begin(); - if( *d == GAMESTATE->m_PreferredDifficulty[0] ) + if( *d == GAMESTATE->m_PreferredDifficulty[p] ) { vbSelOut[i] = true; + matched= true; vector v; v.push_back( p ); ExportOption( v, vbSelectedOut ); // current steps changed - continue; + break; } } } - // default to 1st - vbSelOut[0] = true; + if(!matched) + { + // default to 1st + vbSelOut[0] = true; + } } } virtual int ExportOption( const vector &vpns, const vector vbSelected[NUM_PLAYERS] ) const diff --git a/src/ScreenManager.cpp b/src/ScreenManager.cpp index 6df06f2a71..eacb2a207c 100644 --- a/src/ScreenManager.cpp +++ b/src/ScreenManager.cpp @@ -367,6 +367,12 @@ bool ScreenManager::AllowOperatorMenuButton() const return true; } +bool ScreenManager::IsScreenNameValid(RString const& name) const +{ + RString ClassName = THEME->GetMetric(name,"Class"); + return g_pmapRegistrees->find(ClassName) != g_pmapRegistrees->end(); +} + bool ScreenManager::IsStackedScreen( const Screen *pScreen ) const { // True if the screen is in the screen stack, but not the first. diff --git a/src/ScreenManager.h b/src/ScreenManager.h index 304cb77d5b..f43b000b66 100644 --- a/src/ScreenManager.h +++ b/src/ScreenManager.h @@ -39,6 +39,8 @@ public: Screen *GetScreen( int iPosition ); bool AllowOperatorMenuButton() const; + bool IsScreenNameValid(RString const& name) const; + // System messages void SystemMessage( const RString &sMessage ); void SystemMessageNoAnimate( const RString &sMessage );