fix new Screens get created 3x after the last GameCommand change

remove complicated GetAndEraseScreen logic and export SELECT_NONE rows if they have focus, otherwise don't export them (fixes InsertCoin row)

stepmania/src/OptionRow.cpp

@@ -28,9 +28,6 @@ static const CString LayoutTypeNames[NUM_LAYOUT_TYPES] = {
 XToString( LayoutType, NUM_LAYOUT_TYPES );
 StringToX( LayoutType );
 
-#define FOREACH_OptionsPlayer( pn ) \
-	for( PlayerNumber pn=GetNextHumanPlayer((PlayerNumber)-1); pn!=PLAYER_INVALID && (!m_RowDef.bOneChoiceForAllPlayers || pn==0); pn=GetNextHumanPlayer(pn) )
-
 const CString NEXT_ROW_NAME = "NextRow";
 const CString EXIT_NAME = "Exit";
 
@@ -724,8 +721,8 @@ void OptionRow::Reload()
 				return;
 
 			if( m_RowDef.m_bExportOnChange )
-				FOREACH_OptionsPlayer( p )
+				FOREACH_HumanPlayer( p )
-					ExportOptions( p );
+					ExportOptions( p, false );
 
 			m_pHand->Reload( m_RowDef );
 			ASSERT( !m_RowDef.choices.empty() );
@@ -733,17 +730,17 @@ void OptionRow::Reload()
 			FOREACH_PlayerNumber( p )
 				m_vbSelected[p].resize( m_RowDef.choices.size(), false );
 
-			FOREACH_OptionsPlayer( p )
+			FOREACH_HumanPlayer( p )
 				ImportOptions( p );
 
 			switch( m_RowDef.selectType )
 			{
 			case SELECT_ONE:
-				FOREACH_OptionsPlayer( p )
+				FOREACH_HumanPlayer( p )
 					m_iChoiceInRowWithFocus[p] = GetOneSelection(p);
 				break;
 			case SELECT_MULTIPLE:
-				FOREACH_OptionsPlayer( p )
+				FOREACH_HumanPlayer( p )
 					CLAMP( m_iChoiceInRowWithFocus[p], 0, m_RowDef.choices.size()-1 );
 				break;
 			default:
@@ -751,8 +748,8 @@ void OptionRow::Reload()
 			}
 
 			if( m_RowDef.m_bExportOnChange )
-				FOREACH_OptionsPlayer( p )
+				FOREACH_HumanPlayer( p )
-					ExportOptions( p );
+					ExportOptions( p, false );
 
 			UpdateEnabledDisabled();
 			UpdateText();
@@ -812,7 +809,7 @@ void OptionRow::ImportOptions( PlayerNumber pn )
 	VerifySelected( m_RowDef.selectType, m_vbSelected[pn], m_RowDef.name );
 }
 
-int OptionRow::ExportOptions( PlayerNumber pn )
+int OptionRow::ExportOptions( PlayerNumber pn, bool bRowHasFocus )
 {
 	if( m_pHand == NULL )
 		return 0;
@@ -824,7 +821,18 @@ int OptionRow::ExportOptions( PlayerNumber pn )
 	VerifySelected( m_RowDef.selectType, m_vbSelected[pn], m_RowDef.name );
 	ASSERT( m_vbSelected[pn].size() == m_RowDef.choices.size() );
 	ERASE_ONE_BOOL_AT_FRONT_IF_NEEDED( m_vbSelected[pn] );
+
+	// SELECT_NONE rows get exported if they have focus when the user presses 
+	// Start.
+	int iChoice = GetChoiceInRowWithFocus( pn );
+	if( m_RowDef.selectType == SELECT_NONE && bRowHasFocus )
+		m_vbSelected[pn][iChoice] = true;
+
 	iChangeMask |= m_pHand->ExportOption( m_RowDef, pn, m_vbSelected[pn] );
+	
+	if( m_RowDef.selectType == SELECT_NONE && bRowHasFocus )
+		m_vbSelected[pn][iChoice] = false;
+	
 	INSERT_ONE_BOOL_AT_FRONT_IF_NEEDED( m_vbSelected[pn] );
 	
 	return iChangeMask;

stepmania/src/OptionRow.h

@@ -91,7 +91,7 @@ public:
 	CString GetRowTitle() const;
 
 	void ImportOptions( PlayerNumber pn );
-	int ExportOptions( PlayerNumber pn );
+	int ExportOptions( PlayerNumber pn, bool bRowHasFocus );
 
 	void AfterImportOptions( float fY );
 	void DetachHandler();

stepmania/src/OptionRowHandler.cpp

@@ -229,20 +229,10 @@ public:
 			ListEntries[iFirstSelection].m_sModifiers :
 			def.choices[iFirstSelection];
 	}
-	virtual CString GetAndEraseScreen( int iChoice )
+	virtual bool HasScreen( int iChoice ) const
 	{ 
-		GameCommand &mc = ListEntries[iChoice];
+		const GameCommand &mc = ListEntries[iChoice];
-		if( mc.m_sScreen != "" )
+		return !mc.m_sScreen.empty();
-		{
-			/* Hack: instead of applying screen commands here, store them in
-			* m_sNextScreen and apply them after we tween out.  If we don't set
-			* m_sScreen to "", we'll load it twice (once for each player) and
-			* then again for m_sNextScreen. */
-			CString sNextScreen = mc.m_sScreen;
-			mc.m_sScreen = "";
-			return sNextScreen;
-		}
-		return "";
 	}
 
 	void FillNoteSkins( OptionRowDefinition &defOut, CString sParam )

stepmania/src/OptionRowHandler.h

@@ -28,7 +28,7 @@ public:
 	/* Returns an OPT mask. */
 	virtual int ExportOption( const OptionRowDefinition &def, PlayerNumber pn, const vector<bool> &vbSelected ) const = 0;
 	virtual CString GetIconText( const OptionRowDefinition &def, int iFirstSelection ) const { return ""; }
-	virtual CString GetAndEraseScreen( int iChoice ) { return ""; }
+	virtual bool HasScreen( int iChoice ) const { return false; }
 };
 
 

stepmania/src/ScreenOptions.h

@@ -18,6 +18,8 @@ enum InputMode
 	INPUTMODE_SHARE_CURSOR		// both players control the same cursor
 };
 
+#define FOREACH_OptionsPlayer( pn ) \
+	for( PlayerNumber pn=GetNextHumanPlayer((PlayerNumber)-1); pn!=PLAYER_INVALID && (m_InputMode==INPUTMODE_INDIVIDUAL || pn==0); pn=GetNextHumanPlayer(pn) )
 
 class ScreenOptions : public ScreenWithMenuElements
 {

stepmania/src/ScreenOptionsMaster.cpp

@@ -115,30 +115,29 @@ void ScreenOptionsMaster::ImportOptions( int r, PlayerNumber pn )
 void ScreenOptionsMaster::ExportOptions( int r, PlayerNumber pn )
 {
 	OptionRow &row = *m_Rows[r];
-	m_iChangeMask |= row.ExportOptions( pn );
+	bool bRowHasFocus = m_iCurrentRow[pn] == r;
+	m_iChangeMask |= row.ExportOptions( pn, bRowHasFocus );
 }
 
 void ScreenOptionsMaster::BeginFadingOut()
 {
 	/* If the selection is on a LIST, and the selected LIST option sets the screen,
 	* honor it. */
-	m_sExportedNextScreen = "";
+	m_bExportWillSetANewScreen = false;
 
 	int iCurRow = this->GetCurrentRow();
 	ASSERT( iCurRow >= 0 && iCurRow < (int)m_Rows.size() );
 	OptionRow &row = *m_Rows[iCurRow];
 
-	if( iCurRow < (int)OptionRowHandlers.size() )
+	if( row.GetRowType() != OptionRow::ROW_EXIT )
 	{
-		const int iChoice = row.GetChoiceInRowWithFocus(GAMESTATE->m_MasterPlayerNumber);
+		const int iChoice = row.GetChoiceInRowWithFocus( GAMESTATE->m_MasterPlayerNumber );
 		OptionRowHandler *pHand = OptionRowHandlers[iCurRow];
-		CString sScreen = pHand->GetAndEraseScreen( iChoice );
+		m_bExportWillSetANewScreen = pHand->HasScreen( iChoice );
-		if( !sScreen.empty() )
-			m_sExportedNextScreen = sScreen;
 	}
 
 	// If options set a NextScreen or one is specified in metrics, then fade out
-	if( m_sExportedNextScreen != "" || NEXT_SCREEN != "" )
+	if( m_bExportWillSetANewScreen || NEXT_SCREEN != "" )
 		ScreenOptions::BeginFadingOut();
 }
 
@@ -146,8 +145,8 @@ void ScreenOptionsMaster::GoToNextScreen()
 {
 	if( GAMESTATE->m_bEditing )
 		SCREENMAN->PopTopScreen( SM_None );
-	else if( m_sExportedNextScreen != "" )
+	else if( m_bExportWillSetANewScreen )
-		SCREENMAN->SetNewScreen( m_sExportedNextScreen );
+		;	// Do nothing.  Let Export set the screen.
 	else if( NEXT_SCREEN != "" )
 		SCREENMAN->SetNewScreen( NEXT_SCREEN );
 }
@@ -169,10 +168,11 @@ void ScreenOptionsMaster::GoToPrevScreen()
 
 void ScreenOptionsMaster::RefreshIcons( int r, PlayerNumber pn )
 {
-	if( m_Rows[r]->GetRowType() == OptionRow::ROW_EXIT )
+	OptionRow &row = *m_Rows[r];
+	
+	if( row.GetRowType() == OptionRow::ROW_EXIT )
 		return;	// skip
 
-	OptionRow &row = *m_Rows[r];
 	const OptionRowDefinition &def = row.GetRowDef();
 
 	// find first selection and whether multiple are selected
@@ -216,7 +216,7 @@ void ScreenOptionsMaster::HandleScreenMessage( const ScreenMessage SM )
 			{
 				CHECKPOINT_M( ssprintf("%i/%i", r, int(OptionRowHandlers.size())) );
 
-				FOREACH_HumanPlayer( p )
+				FOREACH_OptionsPlayer( p )
 					ExportOptions( r, p );
 			}
 
@@ -246,7 +246,7 @@ void ScreenOptionsMaster::HandleScreenMessage( const ScreenMessage SM )
 			if( m_iChangeMask & OPT_RESET_GAME )
 			{
 				ResetGame();
-				m_sExportedNextScreen = "";
+				m_bExportWillSetANewScreen = false;
 			}
 
 			if( m_iChangeMask & OPT_APPLY_SOUND )

stepmania/src/ScreenOptionsMaster.h

@@ -14,7 +14,7 @@ public:
 
 protected:
 	int m_iChangeMask;
-	CString m_sExportedNextScreen;	// from an OptionRowHandler
+	bool m_bExportWillSetANewScreen;	// from an OptionRowHandler
 
 	vector<OptionRowHandler*> OptionRowHandlers;