encapsulate hold results in HoldNoteResult (HoldNoteScore + life)

instead of moving HoldNote::iStartRow forward while a hold note is
held, set HoldNoteResult::iLastHeldRow
fixes hold heads cycling through all note colors while held (wasn't very
visible due to the hold ghost) and GetSuccessfulHands bugs

stepmania/src/NoteDataWithScoring.cpp

@@ -29,7 +29,6 @@ void NoteDataWithScoring::Init()
 		m_TapNoteScores[t].clear();
 
 	m_HoldNoteScores.clear();
-	m_fHoldNoteLife.clear();
 }
 
 int NoteDataWithScoring::GetNumTapNotesWithScore( TapNoteScore tns, const float fStartBeat, float fEndBeat ) const
@@ -169,7 +168,18 @@ int NoteDataWithScoring::GetSuccessfulHands( float fStartBeat, float fEndBeat )
 		for( int j=0; j<GetNumHoldNotes(); j++ )
 		{
 			const HoldNote &hn = GetHoldNote(j);
-			if( hn.iStartRow+1 <= i && i <= hn.iEndRow && GetHoldNoteScore(hn) != HNS_OK )
+			HoldNoteResult hnr = GetHoldNoteResult( hn );
+
+			/* Check if the row we're checking is in range. */
+			if( !hn.RowIsInRange(i) )
+				continue;
+
+			/* If a hold is released *after* a hands containing it, the hands is
+			 * still good.  So, ignore the judgement and only examine iLastHeldRow
+			 * to be sure that the hold was still held at the point of this row.
+			 * (Note that if the hold head tap was missed, then iLastHeldRow == i
+			 * and this won't fail--but the tap check above will have already failed.) */
+			if( hnr.iLastHeldRow < i )
 				Missed = true;
 		}
 
@@ -363,26 +373,50 @@ void NoteDataWithScoring::SetTapNoteOffset(unsigned track, unsigned row, float o
  * changes when hold notes are being stepped on, but end rows never change. */
 HoldNoteScore NoteDataWithScoring::GetHoldNoteScore( const HoldNote &hn ) const
 {
-	map<RowTrack, HoldNoteScore>::const_iterator it = m_HoldNoteScores.find( RowTrack(hn) );
+	return GetHoldNoteResult(hn).hns;
-	if( it == m_HoldNoteScores.end() )
-		return HNS_NONE;
-	return it->second;
 }
 
 void NoteDataWithScoring::SetHoldNoteScore( const HoldNote &hn, HoldNoteScore hns )
 {
-	m_HoldNoteScores[RowTrack(hn)] = hns;
+	HoldNoteResult *hnr = CreateHoldNoteResult( hn );
+	hnr->hns = hns;
 }
 
 void NoteDataWithScoring::SetHoldNoteLife( const HoldNote &hn, float f )
 {
-	m_fHoldNoteLife[RowTrack(hn)] = f;
+	HoldNoteResult *hnr = CreateHoldNoteResult( hn );
+	hnr->fLife = f;
 }
 
 float NoteDataWithScoring::GetHoldNoteLife( const HoldNote &hn ) const
 {
-	map<RowTrack, float>::const_iterator it = m_fHoldNoteLife.find( RowTrack(hn) );
+	return GetHoldNoteResult(hn).fLife;
-	if( it == m_fHoldNoteLife.end() )
+}
-		return 1.0f;
+
+const HoldNoteResult NoteDataWithScoring::GetHoldNoteResult( const HoldNote &hn ) const
+{
+	map<RowTrack, HoldNoteResult>::const_iterator it = m_HoldNoteScores.find( RowTrack(hn) );
+	if( it == m_HoldNoteScores.end() )
+		return HoldNoteResult(hn);
+
 	return it->second;
 }
+
+HoldNoteResult *NoteDataWithScoring::CreateHoldNoteResult( const HoldNote &hn )
+{
+	map<RowTrack, HoldNoteResult>::iterator it = m_HoldNoteScores.find( RowTrack(hn) );
+	if( it == m_HoldNoteScores.end() )
+	{
+		HoldNoteResult *ret = &m_HoldNoteScores[hn];
+		ret->iLastHeldRow = hn.iStartRow;
+		return ret;
+	}
+	return &it->second;
+}
+
+HoldNoteResult::HoldNoteResult( const HoldNote &hn )
+{
+	hns = HNS_NONE;
+	fLife = 1.0f;
+	iLastHeldRow = hn.iStartRow;
+}

stepmania/src/NoteDataWithScoring.h

@@ -22,21 +22,42 @@ public:
 	RowTrack( const HoldNote &hn ): pair<int,int>( hn.iEndRow, hn.iTrack ) { }
 };
 
+struct HoldNoteResult
+{
+	HoldNoteScore hns;
+
+	/* 1.0 means this HoldNote has full life.
+	 * 0.0 means this HoldNote is dead
+	 * When this value hits 0.0 for the first time, m_HoldScore becomes HSS_NG.
+	 * If the life is > 0.0 when the HoldNote ends, then m_HoldScore becomes HSS_OK. */
+	float fLife;
+
+	/* Last index where fLife was greater than 0.  If the tap was missed, this will
+	 * be the first index of the hold. */
+	int iLastHeldRow;
+
+	HoldNoteResult()
+	{
+		hns = HNS_NONE;
+		fLife = 1.0f;
+		iLastHeldRow = 0;
+	}
+	HoldNoteResult( const HoldNote &hn );
+
+	float GetLastHeldBeat() const { return NoteRowToBeat(iLastHeldRow); }
+};
+
 class NoteDataWithScoring : public NoteData
 {
 	// maintain this extra data in addition to the NoteData
 	vector<TapNoteScore> m_TapNoteScores[MAX_NOTE_TRACKS];
-	map<RowTrack, HoldNoteScore> m_HoldNoteScores;
+	map<RowTrack, HoldNoteResult> m_HoldNoteScores;
 
 	/* Offset, in seconds, for each tap grade.  Negative numbers mean the note
 	 * was hit early; positive numbers mean it was hit late.  These values are
 	 * only meaningful for graded taps (m_TapNoteScores >= TNS_BOO). */
 	vector<float> m_TapNoteOffset[MAX_NOTE_TRACKS];
 
-	/* 1.0 means this HoldNote has full life.
-	 * 0.0 means this HoldNote is dead
-	 * When this value hits 0.0 for the first time, m_HoldScore becomes HSS_NG.
-	 * If the life is > 0.0 when the HoldNote ends, then m_HoldScore becomes HSS_OK. */
 	map<RowTrack, float> m_fHoldNoteLife;
 
 public:
@@ -58,6 +79,8 @@ public:
 	void SetHoldNoteScore( const HoldNote &hn, HoldNoteScore hns );
 	float GetHoldNoteLife( const HoldNote &hn ) const;
 	void SetHoldNoteLife( const HoldNote &hn, float f );
+	const HoldNoteResult GetHoldNoteResult( const HoldNote &hn ) const;
+	HoldNoteResult *CreateHoldNoteResult( const HoldNote &hn );
 
 	bool IsRowCompletelyJudged(unsigned row) const;
 	TapNoteScore MinTapNoteScore(unsigned row) const;

stepmania/src/NoteDisplay.cpp

@@ -25,6 +25,7 @@
 #include "NoteTypes.h"
 #include "NoteFieldPositioning.h"
 #include "ActorUtil.h"
+#include "NoteDataWithScoring.h"
 
 enum part
 {
@@ -767,13 +768,13 @@ void NoteDisplay::DrawHoldHead( const HoldNote& hn, bool bIsBeingHeld, float fYH
 	}
 }
 
-void NoteDisplay::DrawHold( const HoldNote& hn, bool bIsBeingHeld, bool bIsActive, float fLife, float fPercentFadeToFail, bool bDrawGlowOnly, float fReverseOffsetPixels )
+void NoteDisplay::DrawHold( const HoldNote& hn, bool bIsBeingHeld, bool bIsActive, const HoldNoteResult &Result, float fPercentFadeToFail, bool bDrawGlowOnly, float fReverseOffsetPixels )
 {
 	// bDrawGlowOnly is a little hacky.  We need to draw the diffuse part and the glow part one pass at a time to minimize state changes
 
 	int	iCol			= hn.iTrack;
 	bool bReverse = GAMESTATE->m_CurrentPlayerOptions[m_PlayerNumber].GetReversePercentForColumn(iCol) > 0.5;
-	float fStartYOffset	= ArrowGetYOffset( m_PlayerNumber, iCol, hn.GetStartBeat() );
+	float fStartYOffset	= ArrowGetYOffset( m_PlayerNumber, iCol, Result.GetLastHeldBeat() );
 	
 	// HACK: If active, don't allow the top of the hold to go above the receptor
 	if( bIsActive )
@@ -791,7 +792,7 @@ void NoteDisplay::DrawHold( const HoldNote& hn, bool bIsBeingHeld, bool bIsActiv
 	/* Hack: Z effects need a finer grain step. */
 	const int	fYStep = WavyPartsNeedZBuffer? 4: 16; //bWavy ? 16 : 128;	// use small steps only if wavy
 
-	const float fColorScale		= 1*fLife + (1-fLife)*cache->m_fHoldNGGrayPercent;
+	const float fColorScale		= 1*Result.fLife + (1-Result.fLife)*cache->m_fHoldNGGrayPercent;
 
 	bool bFlipHeadAndTail = bReverse && cache->m_bFlipHeadAndTailWhenReverse;
 
@@ -822,7 +823,7 @@ void NoteDisplay::DrawHold( const HoldNote& hn, bool bIsBeingHeld, bool bIsActiv
 
 	// now, draw the glow pass
 	if( !bDrawGlowOnly )
-		DrawHold( hn, bIsBeingHeld, bIsActive, fLife, fPercentFadeToFail, true, fReverseOffsetPixels );
+		DrawHold( hn, bIsBeingHeld, bIsActive, Result, fPercentFadeToFail, true, fReverseOffsetPixels );
 }
 
 void NoteDisplay::DrawActor( Actor* pActor, int iCol, float fBeat, float fPercentFadeToFail, float fLife, float fReverseOffsetPixels, bool bUseLighting )

stepmania/src/NoteDisplay.h

@@ -18,7 +18,7 @@
 #include "NoteTypes.h"
 #include "PlayerNumber.h"
 
-
+struct HoldNoteResult;
 struct NoteMetricCache_t;
 
 class NoteDisplay
@@ -33,7 +33,7 @@ public:
 
 	void DrawActor( Actor* pActor, int iCol, float fBeat, float fPercentFadeToFail, float fLife, float fReverseOffsetPixels, bool bUseLighting );
 	void DrawTap( int iCol, float fBeat, bool bOnSameRowAsHoldStart, bool bIsAddition, bool bIsMine, float fPercentFadeToFail, float fLife, float fReverseOffsetPixels );
-	void DrawHold( const HoldNote& hn, bool bIsBeingHeld, bool bIsActive, float fLife, float fPercentFadeToFail, bool bDrawGlowOnly, float fReverseOffsetPixels );
+	void DrawHold( const HoldNote& hn, bool bIsBeingHeld, bool bIsActive, const HoldNoteResult &Result, float fPercentFadeToFail, bool bDrawGlowOnly, float fReverseOffsetPixels );
 
 protected:
 	void SetActiveFrame( float fNoteBeat, Actor &actorToSet, float fAnimationLengthInBeats, bool bVivid, bool bNoteColor );

stepmania/src/NoteField.cpp

@@ -543,8 +543,8 @@ void NoteField::DrawPrimitives()
 			if( hn.iTrack != c )	// this HoldNote doesn't belong to this column
 				continue;
 
-			const HoldNoteScore hns = GetHoldNoteScore( hn );
+			const HoldNoteResult Result = GetHoldNoteResult( hn );
-			if( hns == HNS_OK )	// if this HoldNote was completed
+			if( Result.hns == HNS_OK )	// if this HoldNote was completed
 				continue;	// don't draw anything
 
 			// If no part of this HoldNote is on the screen, skip it
@@ -565,7 +565,6 @@ void NoteField::DrawPrimitives()
 
 			const bool bIsActive = m_ActiveHoldNotes[hn];
 			const bool bIsHoldingNote = m_HeldHoldNotes[hn];
-			const float fLife = GetHoldNoteLife( hn );
 			if( bIsActive )
 				SearchForSongBeat()->m_GhostArrowRow.SetHoldIsActive( hn.iTrack );
 			
@@ -577,7 +576,7 @@ void NoteField::DrawPrimitives()
 				bIsInSelectionRange = hn.ContainedByRange( BeatToNoteRow( m_fBeginMarker ), BeatToNoteRow( m_fEndMarker ) );
 
 			NoteDisplayCols *nd = CurDisplay->second;
-			nd->display[c].DrawHold( hn, bIsHoldingNote, bIsActive, fLife, bIsInSelectionRange ? fSelectedRangeGlow : m_fPercentFadeToFail, false, m_fYReverseOffsetPixels );
+			nd->display[c].DrawHold( hn, bIsHoldingNote, bIsActive, Result, bIsInSelectionRange ? fSelectedRangeGlow : m_fPercentFadeToFail, false, m_fYReverseOffsetPixels );
 		}
 		
 

stepmania/src/Player.cpp

@@ -356,12 +356,15 @@ void PlayerMinus::Update( float fDeltaTime )
 			m_pNoteField->m_HeldHoldNotes[hn] = bIsHoldingButton && bSteppedOnTapNote;
 			m_pNoteField->m_ActiveHoldNotes[hn] = bSteppedOnTapNote;
 
-			if( bSteppedOnTapNote )		// this note is not judged and we stepped on its head
+			if( bSteppedOnTapNote )
 			{
-				// Move the start of this Hold
+				/* This hold note is not judged and we stepped on its head.  Update
-				const int n = m_pNoteField->GetMatchingHoldNote( hn );
+				 * iLastHeldRow. */
-				HoldNote &fhn = m_pNoteField->GetHoldNote( n );
+				HoldNoteResult *hnr = m_pNoteField->CreateHoldNoteResult( hn );
-				fhn.iStartRow = min( iSongRow, fhn.iEndRow );
+				hnr->iLastHeldRow = min( iSongRow, hn.iEndRow );
+
+				hnr = this->CreateHoldNoteResult( hn );
+				hnr->iLastHeldRow = min( iSongRow, hn.iEndRow );
 			}