fix "transform mods that last for a whole song in a course get applied twice"

fix "impossible to get 100% on a course that uses AddMines" by calculating RadarValues after applying mods
Trail::GetRadarValues needs to do caching.  Performance is bad.

stepmania/src/Attack.cpp

@@ -3,9 +3,12 @@
 #include "GameState.h"
 #include "RageUtil.h"
 #include "song.h"
+#include "Foreach.h"
 
 void Attack::GetAttackBeats( const Song *song, PlayerNumber pn, float &fStartBeat, float &fEndBeat ) const
 {
+	ASSERT( song );
+
 	if( fStartSecond >= 0 )
 	{
 		CHECKPOINT;
@@ -19,7 +22,7 @@ void Attack::GetAttackBeats( const Song *song, PlayerNumber pn, float &fStartBea
 		ASSERT( GAMESTATE->m_pCurSong );
 		
 		/* We're setting this effect on the fly.  If it's an arrow-changing effect
-		 * (transform or note skin), apply it in the future, past what's currently on
+		 * (transform or note skin), apply it in the future, after what's currently on
 		 * screen, so new arrows will scroll on screen with this effect. */
 		GAMESTATE->GetUndisplayedBeats( pn, fSecsRemaining, fStartBeat, fEndBeat );
 	}
@@ -39,6 +42,23 @@ bool Attack::operator== ( const Attack &rhs ) const
 		EQUAL(bGlobal);
 }
 
+bool Attack::ContainsTransformOrTurn() const
+{
+	PlayerOptions po;
+	po.FromString( sModifier );
+	return po.ContainsTransformOrTurn();
+}
+
+bool AttackArray::ContainsTransformOrTurn() const
+{
+	FOREACH_CONST( Attack, *this, a )
+	{
+		if( a->ContainsTransformOrTurn() )
+			return true;
+	}
+	return false;
+}
+
 /*
  * (c) 2003-2004 Chris Danford
  * All rights reserved.

stepmania/src/Attack.h

@@ -19,8 +19,13 @@ struct Attack
 	void MakeBlank() { sModifier=""; }
 	Attack() { fStartSecond = -1; bOn = false; bGlobal = false; }
 	bool operator== ( const Attack &rhs ) const;
+	bool ContainsTransformOrTurn() const;
+};
+
+struct AttackArray : public vector<Attack>
+{
+	bool ContainsTransformOrTurn() const;
 };
-typedef vector<Attack> AttackArray;
 
 #endif
 

stepmania/src/NoteDataUtil.cpp

@@ -6,6 +6,7 @@
 #include "song.h"
 #include "GameState.h"
 #include "RadarValues.h"
+#include "Foreach.h"
 
 NoteType NoteDataUtil::GetSmallestNoteTypeForMeasure( const NoteData &n, int iMeasureIndex )
 {
@@ -1551,6 +1552,22 @@ void NoteDataUtil::ConvertAdditionsToRegular( NoteData &in )
 				in.SetTapNote(t,r,TAP_TAP);
 }
 
+void NoteDataUtil::TransformNoteData( NoteData &nd, const AttackArray &aa, StepsType st, Song* pSong )
+{
+	FOREACH_CONST( Attack, aa, a )
+	{
+		PlayerOptions po;
+		po.FromString( a->sModifier );
+		if( po.ContainsTransformOrTurn() )
+		{
+			float fStartBeat, fEndBeat;
+			a->GetAttackBeats( pSong, PLAYER_INVALID, fStartBeat, fEndBeat );
+
+			NoteDataUtil::TransformNoteData( nd, po, st, fStartBeat, fEndBeat );
+		}
+	}
+}
+
 void NoteDataUtil::TransformNoteData( NoteData &nd, const PlayerOptions &po, StepsType st, float fStartBeat, float fEndBeat )
 {
 	if( po.m_bTurns[PlayerOptions::TURN_MIRROR] )			NoteDataUtil::Turn( nd, st, NoteDataUtil::mirror, fStartBeat, fEndBeat );

stepmania/src/NoteDataUtil.h

@@ -84,6 +84,7 @@ namespace NoteDataUtil
 	// True if no notes in row that aren't true in the mask
 	bool RowPassesValidMask( NoteData &in, int row, const bool bValidMask[] );
 
+	void TransformNoteData( NoteData &nd, const AttackArray &aa, StepsType st, Song* pSong );
 	void TransformNoteData( NoteData &nd, const PlayerOptions &po, StepsType st, float fStartBeat = 0, float fEndBeat = 99999 );
 	void AddTapAttacks( NoteData &nd, Song* pSong );
 

stepmania/src/PlayerOptions.cpp

@@ -179,7 +179,6 @@ CString PlayerOptions::GetString() const
  * you don't want this. */
 void PlayerOptions::FromString( CString sOptions )
 {
-	ASSERT( GAMESTATE->m_pPosition );
 	ASSERT( NOTESKIN );
 //	Init();
 	sOptions.MakeLower();
@@ -299,11 +298,13 @@ void PlayerOptions::FromString( CString sOptions )
 		else if( sBit == "space" )		{ m_fSkew = level; m_fPerspectiveTilt = +level;		m_SpeedfSkew = m_SpeedfPerspectiveTilt = speed; }
 		else if( sBit == "hallway" )	{ m_fSkew = 0; m_fPerspectiveTilt = -level;			m_SpeedfSkew = m_SpeedfPerspectiveTilt = speed; }
 		else if( sBit == "distant" )	{ m_fSkew = 0; m_fPerspectiveTilt = +level;			m_SpeedfSkew = m_SpeedfPerspectiveTilt = speed; }
-		else if( GAMESTATE->m_pPosition->IsValidModeForAnyStyle(sBit) )
+		else if( GAMESTATE->m_pPosition && GAMESTATE->m_pPosition->IsValidModeForAnyStyle(sBit) )
+			// The only time we'll be in this function and NOTESKIN isn't created is 
+			// when calculating RadarValues for courses with transform mods.
 			m_sPositioning = sBit;
 		else if( sBit == "nopositioning" )
 			m_sPositioning = "";
-		else if( NOTESKIN->DoesNoteSkinExist(sBit) )
+		else if( NOTESKIN && NOTESKIN->DoesNoteSkinExist(sBit) )
 			m_sNoteSkin = sBit;
 		else if( sBit == "noteskin" && !on ) /* "no noteskin" */
 			m_sNoteSkin = "default";
@@ -620,6 +621,21 @@ CString PlayerOptions::GetThemedString() const
 	return join( ", ", asThemedMods );
 }
 
+bool PlayerOptions::ContainsTransformOrTurn() const
+{
+	for( int i=0; i<NUM_TRANSFORMS; i++ )
+	{
+		if( m_bTransforms[i] )
+			return true;
+	}
+	for( int i=0; i<NUM_TURNS; i++ )
+	{
+		if( m_bTurns[i] )
+			return true;
+	}
+	return false;
+}
+
 /*
  * (c) 2001-2004 Chris Danford, Glenn Maynard
  * All rights reserved.

stepmania/src/PlayerOptions.h

@@ -20,6 +20,7 @@ struct PlayerOptions
 	CString GetThemedString() const;
 	void FromString( CString sOptions );
 	void ChooseRandomMofifiers();
+	bool ContainsTransformOrTurn() const;
 
 	bool operator==( const PlayerOptions &other ) const;
 	bool operator!=( const PlayerOptions &other ) const { return !operator==(other); }

stepmania/src/ScoreKeeperMAX2.cpp

@@ -28,42 +28,37 @@ ScoreKeeperMAX2::ScoreKeeperMAX2( const vector<Song*>& apSongs, const vector<Ste
 	{
 		Song* pSong = apSongs[i];
 		Steps* pSteps = apSteps[i];
-		NoteData notedata;
-		pSteps->GetNoteData( &notedata );
+		const AttackArray &aa = asModifiers[i];
+		NoteData ndTemp;
+		pSteps->GetNoteData( &ndTemp );
 
 		/* We might have been given lots of songs; don't keep them in memory uncompressed. */
 		pSteps->Compress();
 
 		const Style* pStyle = GAMESTATE->GetCurrentStyle();
-		NoteData playerNoteData;
-		pStyle->GetTransformedNoteDataForStyle( pn_, &notedata, &playerNoteData );
+		NoteData nd;
+		pStyle->GetTransformedNoteDataForStyle( pn_, &ndTemp, &nd );
 
-		/* Apply transforms to find out how the notes will really look. 
+		/* Compute RadarValues before applying any user-selected mods.  Apply
+		 * Course mods and count them in the "pre" RadarValues because they're
+		 * forced and not chosen by the user.
+		 */
+		NoteDataUtil::TransformNoteData( nd, aa, pSteps->m_StepsType, pSong );
+		RadarValues rvPre;
+		NoteDataUtil::GetRadarValues( nd, pSong->m_fMusicLengthSeconds, rvPre );
+
+		/* Apply user transforms to find out how the notes will really look. 
 		 *
 		 * XXX: This is brittle: if we end up combining mods for a song differently
 		 * than ScreenGameplay, we'll end up with the wrong data.  We should probably
 		 * have eg. GAMESTATE->GetOptionsForCourse(po,so,pn) to get options based on
 		 * the last call to StoreSelectedOptions and the modifiers list, but that'd
 		 * mean moving the queues in ScreenGameplay to GameState ... */
-		NoteData playerNoteDataPostModifiers(playerNoteData);
-		NoteDataUtil::TransformNoteData( playerNoteDataPostModifiers, GAMESTATE->m_PlayerOptions[pn_], GAMESTATE->GetCurrentStyle()->m_StepsType );
-
-		for( unsigned j=0; j < asModifiers[i].size(); j++ )
-		{
-			const Attack &mod = asModifiers[i][j];
-			PlayerOptions po;
-			po.FromString( mod.sModifier );
-
-			float fStartBeat, fEndBeat;
-			mod.GetAttackBeats( pSong, m_PlayerNumber, fStartBeat, fEndBeat );
-
-			NoteDataUtil::TransformNoteData( playerNoteDataPostModifiers, po, GAMESTATE->GetCurrentStyle()->m_StepsType, fStartBeat, fEndBeat );
-		}
-
-		RadarValues radarValuesPostModifiers;
-		NoteDataUtil::GetRadarValues( playerNoteDataPostModifiers, pSong->m_fMusicLengthSeconds, radarValuesPostModifiers );
+		NoteDataUtil::TransformNoteData( nd, GAMESTATE->m_PlayerOptions[pn_], pSteps->m_StepsType );
+		RadarValues rvPost;
+		NoteDataUtil::GetRadarValues( nd, pSong->m_fMusicLengthSeconds, rvPost );
 		 
-		iTotalPossibleDancePoints += this->GetPossibleDancePoints( pSteps->GetRadarValues(), radarValuesPostModifiers );
+		iTotalPossibleDancePoints += this->GetPossibleDancePoints( rvPre, rvPost );
 	}
 	g_CurStageStats.iPossibleDancePoints[pn_] = iTotalPossibleDancePoints;
 

stepmania/src/ScreenGameplay.cpp

@@ -231,7 +231,7 @@ void ScreenGameplay::Init()
 		g_CurStageStats.iMeter[p] = m_vpStepsQueue[p][0]->GetMeter();
 
 		/* Record combo rollover. */
-		g_CurStageStats.UpdateComboList( (PlayerNumber)p, 0, true );
+		g_CurStageStats.UpdateComboList( p, 0, true );
 	}
 
 	if( GAMESTATE->IsExtraStage() )
@@ -251,7 +251,7 @@ void ScreenGameplay::Init()
 		{
 		case PrefsManager::SCORING_MAX2:
 		case PrefsManager::SCORING_5TH:
-			m_pPrimaryScoreKeeper[p] = new ScoreKeeperMAX2( m_apSongsQueue, m_vpStepsQueue[p], m_asModifiersQueue[p], (PlayerNumber)p );
+			m_pPrimaryScoreKeeper[p] = new ScoreKeeperMAX2( m_apSongsQueue, m_vpStepsQueue[p], m_asModifiersQueue[p], p );
 			break;
 		default: ASSERT(0);
 		}
@@ -259,7 +259,7 @@ void ScreenGameplay::Init()
 		switch( GAMESTATE->m_PlayMode )
 		{
 		case PLAY_MODE_RAVE:
-			m_pSecondaryScoreKeeper[p] = new ScoreKeeperRave( (PlayerNumber)p );
+			m_pSecondaryScoreKeeper[p] = new ScoreKeeperRave( p );
 			break;
 		}
 	}
@@ -412,7 +412,7 @@ void ScreenGameplay::Init()
 				ASSERT(0);
 			}
 
-			m_pLifeMeter[p]->Load( (PlayerNumber)p );
+			m_pLifeMeter[p]->Load( p );
 			m_pLifeMeter[p]->SetName( ssprintf("LifeP%d",p+1) );
 			SET_XY( *m_pLifeMeter[p] );
 			this->AddChild( m_pLifeMeter[p] );		
@@ -469,7 +469,7 @@ void ScreenGameplay::Init()
 			ASSERT(0);
 		}
 
-		m_pPrimaryScoreDisplay[p]->Init( (PlayerNumber)p );
+		m_pPrimaryScoreDisplay[p]->Init( p );
 		m_pPrimaryScoreDisplay[p]->SetName( ssprintf("ScoreP%d",p+1) );
 		SET_XY( *m_pPrimaryScoreDisplay[p] );
 		if( GAMESTATE->m_PlayMode != PLAY_MODE_RAVE || SHOW_SCORE_IN_RAVE ) /* XXX: ugly */
@@ -488,7 +488,7 @@ void ScreenGameplay::Init()
 
 		if( m_pSecondaryScoreDisplay[p] )
 		{
-			m_pSecondaryScoreDisplay[p]->Init( (PlayerNumber)p );
+			m_pSecondaryScoreDisplay[p]->Init( p );
 			m_pSecondaryScoreDisplay[p]->SetName( ssprintf("SecondaryScoreP%d",p+1) );
 			SET_XY( *m_pSecondaryScoreDisplay[p] );
 			this->AddChild( m_pSecondaryScoreDisplay[p] );
@@ -576,15 +576,13 @@ void ScreenGameplay::Init()
 	m_textSongOptions.SetText( GAMESTATE->m_SongOptions.GetString() );
 	this->AddChild( &m_textSongOptions );
 
+	FOREACH_EnabledPlayer( pn )
 	{
-		FOREACH_EnabledPlayer( pn )
-		{
-			m_ActiveAttackList[pn].LoadFromFont( THEME->GetPathF(m_sName,"ActiveAttackList") );
-			m_ActiveAttackList[pn].Init( pn );
-			m_ActiveAttackList[pn].SetName( ssprintf("ActiveAttackListP%d",pn+1) );
-			SET_XY( m_ActiveAttackList[pn] );
-			this->AddChild( &m_ActiveAttackList[pn] );
-		}
+		m_ActiveAttackList[pn].LoadFromFont( THEME->GetPathF(m_sName,"ActiveAttackList") );
+		m_ActiveAttackList[pn].Init( pn );
+		m_ActiveAttackList[pn].SetName( ssprintf("ActiveAttackListP%d",pn+1) );
+		SET_XY( m_ActiveAttackList[pn] );
+		this->AddChild( &m_ActiveAttackList[pn] );
 	}
 
 
@@ -629,7 +627,7 @@ void ScreenGameplay::Init()
 //		{
 //		case PLAY_MODE_BATTLE:
 //			m_pInventory[p] = new Inventory;
-//			m_pInventory[p]->Load( (PlayerNumber)p );
+//			m_pInventory[p]->Load( p );
 //			this->AddChild( m_pInventory[p] );
 //			break;
 //		}
@@ -770,14 +768,31 @@ bool ScreenGameplay::IsLastSong()
 	return GAMESTATE->GetCourseSongIndex()+1 == (int)m_apSongsQueue.size(); // GetCourseSongIndex() is 0-based but size() is not
 }
 
-void ScreenGameplay::SetupSong( int p, int iSongIndex )
+void ScreenGameplay::SetupSong( PlayerNumber p, int iSongIndex )
 {
 	/* This is the first beat that can be changed without it being visible.  Until
 	 * we draw for the first time, any beat can be changed. */
 	GAMESTATE->m_fLastDrawnBeat[p] = -100;
 	GAMESTATE->m_pCurSteps[p] = m_vpStepsQueue[p][iSongIndex];
 
-	// Put course options into effect.
+	/* Load new NoteData into Player.  Do this before 
+	 * RebuildPlayerOptionsFromActiveAttacks or else transform mods will get
+	 * propogated to GAMESTATE->m_PlayerOptions too early and be double-applied
+	 * to the NoteData:
+	 * once in Player::Load, then again in Player::ApplyActiveAttacks.  This 
+	 * is very bad for transforms like AddMines.
+	 */
+	NoteData pOriginalNoteData;
+	GAMESTATE->m_pCurSteps[p]->GetNoteData( &pOriginalNoteData );
+	
+	const Style* pStyle = GAMESTATE->GetCurrentStyle();
+	NoteData pNewNoteData;
+	pStyle->GetTransformedNoteDataForStyle( p, &pOriginalNoteData, &pNewNoteData );
+	m_Player[p].Load( p, &pNewNoteData, m_pLifeMeter[p], m_pCombinedLifeMeter, m_pPrimaryScoreDisplay[p], m_pSecondaryScoreDisplay[p], m_pInventory[p], m_pPrimaryScoreKeeper[p], m_pSecondaryScoreKeeper[p] );
+
+
+	// Put course options into effect.  Do this after Player::Load so
+	// that mods aren't double-applied.
 	GAMESTATE->m_ModsToApply[p].clear();
 	for( unsigned i=0; i<m_asModifiersQueue[p][iSongIndex].size(); ++i )
 	{
@@ -785,24 +800,16 @@ void ScreenGameplay::SetupSong( int p, int iSongIndex )
 		if( a.fStartSecond == 0 )
 			a.fStartSecond = -1;	// now
 		
-		GAMESTATE->LaunchAttack( (PlayerNumber)p, a );
+		GAMESTATE->LaunchAttack( p, a );
 		GAMESTATE->m_SongOptions.FromString( a.sModifier );
 	}
 
 	/* Update attack bOn flags. */
 	GAMESTATE->Update(0);
-	GAMESTATE->RebuildPlayerOptionsFromActiveAttacks( (PlayerNumber)p );
+	GAMESTATE->RebuildPlayerOptionsFromActiveAttacks( p );
 
 	/* Hack: Course modifiers that are set to start immediately shouldn't tween on. */
 	GAMESTATE->m_CurrentPlayerOptions[p] = GAMESTATE->m_PlayerOptions[p];
-
-	NoteData pOriginalNoteData;
-	GAMESTATE->m_pCurSteps[p]->GetNoteData( &pOriginalNoteData );
-	
-	const Style* pStyle = GAMESTATE->GetCurrentStyle();
-	NoteData pNewNoteData;
-	pStyle->GetTransformedNoteDataForStyle( (PlayerNumber)p, &pOriginalNoteData, &pNewNoteData );
-	m_Player[p].Load( (PlayerNumber)p, &pNewNoteData, m_pLifeMeter[p], m_pCombinedLifeMeter, m_pPrimaryScoreDisplay[p], m_pSecondaryScoreDisplay[p], m_pInventory[p], m_pPrimaryScoreKeeper[p], m_pSecondaryScoreKeeper[p] );
 }
 
 static int GetMaxSongsPlayed()
@@ -1371,7 +1378,7 @@ void ScreenGameplay::Update( float fDeltaTime )
                 FOREACH_CpuPlayer(p)
 				{
 					SOUND->PlayOnceFromDir( THEME->GetPathS(m_sName,"oni die") );
-                    ShowOniGameOver((PlayerNumber)p);
+                    ShowOniGameOver( p );
                     m_Player[p].Init();		// remove all notes and scoring
                     m_Player[p].FadeToFail();	// tell the NoteField to fade to white
 				}
@@ -1980,7 +1987,7 @@ void ScreenGameplay::HandleScreenMessage( const ScreenMessage SM )
 				/* Mark failure.  This hasn't been done yet if m_bTwoPlayerRecovery is set. */
 				if( GAMESTATE->m_SongOptions.m_FailType != SongOptions::FAIL_OFF &&
 					(m_pLifeMeter[p] && m_pLifeMeter[p]->IsFailing()) || 
-					(m_pCombinedLifeMeter && m_pCombinedLifeMeter->IsFailing((PlayerNumber)p)) )
+					(m_pCombinedLifeMeter && m_pCombinedLifeMeter->IsFailing(p)) )
 					g_CurStageStats.bFailed[p] = true;
 
 				if( !g_CurStageStats.bFailed[p] )

stepmania/src/ScreenGameplay.h

@@ -68,7 +68,7 @@ protected:
 	void TweenOffScreen();
 
 	bool IsLastSong();
-	void SetupSong( int p, int iSongIndex );
+	void SetupSong( PlayerNumber p, int iSongIndex );
 	void LoadNextSong();
 	void LoadCourseSongNumber( int SongNumber );
 	float StartPlayingSong(float MinTimeToNotes, float MinTimeToMusic);

stepmania/src/Trail.cpp

@@ -3,6 +3,9 @@
 #include "Foreach.h"
 #include "Steps.h"
 #include "song.h"
+#include "PlayerOptions.h"
+#include "NoteData.h"
+#include "NoteDataUtil.h"
 
 void TrailEntry::GetAttackArray( AttackArray &out ) const
 {
@@ -35,6 +38,17 @@ bool TrailEntry::operator== ( const TrailEntry &rhs ) const
 		EQUAL(dc);
 }
 
+bool TrailEntry::ContainsTransformOrTurn() const
+{
+	PlayerOptions po;
+	po.FromString( Modifiers );
+	if( po.ContainsTransformOrTurn() )
+		return true;
+	if( Attacks.ContainsTransformOrTurn() )
+		return true;
+	return false;
+}
+
 RadarValues Trail::GetRadarValues() const
 {
 	RadarValues rv;
@@ -43,7 +57,25 @@ RadarValues Trail::GetRadarValues() const
 	{
 		const Steps *pSteps = e->pSteps;
 		ASSERT( pSteps );
-		rv += pSteps->GetRadarValues();
+		if( e->ContainsTransformOrTurn() )
+		{
+			NoteData nd;
+			pSteps->GetNoteData( &nd );
+			RadarValues rv_orig;
+			NoteDataUtil::GetRadarValues( nd, e->pSong->m_fMusicLengthSeconds, rv_orig );
+			PlayerOptions po;
+			po.FromString( e->Modifiers );
+			if( po.ContainsTransformOrTurn() )
+				NoteDataUtil::TransformNoteData( nd, po, pSteps->m_StepsType );
+			NoteDataUtil::TransformNoteData( nd, e->Attacks, pSteps->m_StepsType, e->pSong );
+			RadarValues rv2;
+			NoteDataUtil::GetRadarValues( nd, e->pSong->m_fMusicLengthSeconds, rv2 );
+			rv += rv2;
+		}
+		else
+		{
+			rv += pSteps->GetRadarValues();			
+		}
 	}
 
 	return rv;

stepmania/src/Trail.h

@@ -35,6 +35,7 @@ struct TrailEntry
 	Difficulty	dc;
 	bool operator== ( const TrailEntry &rhs ) const;
 	bool operator!= ( const TrailEntry &rhs ) const { return !(*this==rhs); }
+	bool ContainsTransformOrTurn() const;
 };
 
 class Trail