Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement guitar note shuffle #193

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Implement guitar note shuffle #193

wants to merge 8 commits into from

Conversation

TheNathannator
Copy link
Collaborator

Works great with the songs I've tried it on so far (for the most part just Soulless 5 lol), and note distribution is pretty good (albeit chaotic). Probably should have some further testing to review how exactly the shuffle characteristics go, but I feel confident in the current algorithm as an initial implementation.

Copy link
Collaborator Author

@TheNathannator TheNathannator left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some self-comments I have:

Comment on lines +185 to +190
public override void RemoveChildNote(VocalNote note)
{
// TODO
throw new NotImplementedException();
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wasn't sure what to do here and didn't feel like going through the AddChildNote override at the moment, so left it as a TODO.

Comment on lines -118 to +119
public void ApplyModifiers<TNote>(InstrumentDifficulty<TNote> track) where TNote : Note<TNote>
public void ApplyModifiers<TNote>(SongEntry song, InstrumentDifficulty<TNote> track) where TNote : Note<TNote>
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having to pass in the SongEntry feels a little hacky, but I'm not sure how else to get the song hash in for note shuffle.

@RileyTheFox
Copy link
Collaborator

RileyTheFox commented Jul 14, 2024

Think the algorithm needs more tuning, or just removing the weighted consideration for the time being because its far too lenient. Loaded up Knights of Cydonia and the chart is different but not really shuffled at all. Even the slow chords in the intro are keeping the same frets which is rather boring.

Tried a couple more charts, here's some thoughts:

image

I think situations such as a hopo following a strum on an identical fret should be avoided unless absolutely necessary. For taps this is fine as it is commonly seen in some charts.

image

I think double hopos should also be avoided, again fine for tap notes though.

@TheNathannator
Copy link
Collaborator Author

Might be good to separate the weighted shuffling into its own modifier entirely, and maybe up the NPS needed to achieve the maximum weight.

I'm conflicted on whether or not to allow these "invalid" HOPO scenarios. On one hand they seem like they could be interesting in some scenarios, but on the other hand they're a little hard to read (though you could say the same for the tap scenario). Wouldn't be difficult to disallow them though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants