Fix HARM3 persisting when switching to Harmony on songs with fewer parts#1515
Open
XaiaX wants to merge 1 commit into
Open
Fix HARM3 persisting when switching to Harmony on songs with fewer parts#1515XaiaX wants to merge 1 commit into
XaiaX wants to merge 1 commit into
Conversation
A player who launched a song as HARM3 and later played as VOCALS could open a two-part song, switch the instrument to HARMONY, and confirm as HARM3 — a part that doesn't exist, producing a song with no notes. The out-of-range check in ChangePlayer compared profile.HarmonyIndex, whose getter returns 0 whenever CurrentInstrument != Harmony (that guard is load-bearing: VocalsPlayer indexes Parts[HarmonyIndex] for VOCALS players too). With the player on VOCALS the check could never fire, so the stale stored index survived until the instrument switch exposed it. Use YargProfile.ResolveHarmonyIndex (added in the corresponding YARG.Core change), which validates the stored value regardless of the current instrument and recomputes the effective index per song from the player's last explicit selection: clamped to the highest available part on songs with fewer parts, restored in full on songs with enough — the same retention DifficultyFallback gives Expert+. Also resolved in CreateInstrumentMenu right after an instrument switch, covering the path from the report. Requires a YARG.Core version containing ResolveHarmonyIndex.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Depends on: YARG.Core PR #395.
Summary
Fixes a bug where a player could launch a song as HARM3 even when the chart only has two harmony parts.
Steps to reproduce
Root cause
YargProfile.HarmonyIndexreturns0whenCurrentInstrument != Harmony— this is correct and load-bearing:VocalsPlayerindexes intoParts[HarmonyIndex]for both VOCALS and HARMONY players, and VOCALS always has exactly one part at index 0. If the getter returned the raw stored value for VOCALS players, a stale index would cause anArgumentOutOfRangeExceptionat song load.However, this same guard masks the out-of-range check in
DifficultySelectMenu.ChangePlayer:When the player's current instrument is VOCALS,
HarmonyIndexreads0,0 >= 2is false, and_harmonyIndexis never corrected. The stale value (e.g.2for HARM3) remains. When the player then switches to HARMONY in the instrument menu,CurrentInstrumentbecomesHarmony, the getter now returns the real_harmonyIndex, and the Difficulty Select main screen displays "3" — and the game launches with it.Fix
Replace the direct
HarmonyIndexcomparison withprofile.ResolveHarmonyIndex(_maxHarmonyIndex)(new method onYargProfile, see Core PR). This recomputes the effective harmony index for the current song from the player's last explicit selection, operating on the backing fields directly so it works regardless ofCurrentInstrument. The same call is also added inCreateInstrumentMenuafter an instrument switch, as a defensive resolve at the point of use.On a song with fewer parts the effective index clamps to
_maxHarmonyIndex - 1(highest available part) rather than0, so a HARM3 player on a two-part song plays HARM2 rather than HARM1. The explicit selection itself is never overwritten by the resolve, so returning to a song with three parts restores HARM3 — the same retention behaviorDifficultyFallbackprovides for Expert+ (it survives songs that lack it as long as the player doesn't explicitly pick something else).Changes
Assets/Script/Menu/DifficultySelect/DifficultySelectMenu.csHarmonyIndexcomparison inChangePlayerwithResolveHarmonyIndex; add same call inCreateInstrumentMenuafter instrument switchWhy not change the getter instead?
Removing the
CurrentInstrumentguard from the getter would breakVocalsPlayer:Parts[profile.HarmonyIndex]is called for VOCALS players wherePartshas only one element. A stale_harmonyIndexof2would immediately throwArgumentOutOfRangeExceptionat song load. The getter guard is correct and necessary;ResolveHarmonyIndexis the right way to access the raw value for validation purposes only.No Core-only fix possible
The resolve logic depends on
_maxHarmonyIndex, which is computed Unity-side fromsong.VocalsCountinDifficultySelectMenu. Core has no concept of which song is currently selected.Testing done
YargProfileHarmonyIndexTestsand verified manually in-game.Beneficial Side Effect