Fix incorrect bpm display for ssc songs with varying bpms per-chart #1217
Conversation
This file contains 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
…e range Previously the function correctly determined the correct BPM range for the particular given chart and then ... did absolutely nothing with it. This mistake in the function dates all the way back to the "initial commit" of the current git repository, which is kind of neat.
…n song Previously, Til Death called the Lua binding for BPMDisplay::SetBpmFromSong as BPMDisplay::SetBpmFromSteps was broken. This meant that songs that had different BPM ranges per-chart (as is possible in .ssc files) only showed the BPM range of the very first chart for all of them. This small change fixes that and lets Til Death display the correct ranges for every individual chart :)
…song Previously, Rebirth called the Lua binding for BPMDisplay::SetBpmFromSong as BPMDisplay::SetBpmFromSteps was broken. This meant that songs that had different BPM ranges per-chart (as is possible in .ssc files) only showed the BPM range of the very first chart for all of them.
…han song Previously, bare-frames called the Lua binding for BPMDisplay::SetBpmFromSong as BPMDisplay::SetBpmFromSteps was broken. This meant that songs that had different BPM ranges per-chart (as is possible in .ssc files) only showed the BPM range of the very first chart for all of them.
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.
This fixes the remaining problem within #615 wherein the displayed BPM range for songs that had different BPM ranges per-chart was incorrect.
#619 fixed most of the problems, but as Poco noted in #619 (comment) the Lua binding for BPMDisplay
SetFromStepsstill did not work even after that change.That was because
BPMDisplay::SetBpmFromStepsdid all the work to determine the correct BPM range and then just ... didn't callSetBPMRangeto actually update it.This PR fixes that function and then makes the simple changes required to Rebirth, Til Death, and bare-frames to get them to use it.
Closes: #615