TeamRizu / OutFox

The Bug Reporting Repository for OutFox LTS 0.4, Alpha V and Steam Early Access Builds
https://projectoutfox.com
Apache License 2.0
182 stars 3 forks source link

[Nitpick] Bpm Display for .ssc Songs With BPM Information Not In Header is Incorrect #632

Open bluebandit21 opened 1 year ago

bluebandit21 commented 1 year ago

Game Mode

dance

Give an example of what is wrong

I constructed an example Song with varying different BPM info per-chart.

#VERSION:0.83;
#TITLE:Bloody Tears;
#ARTIST:iR2-X;
#BANNER:banner.png;
#MUSIC:level.mp3;
#OFFSET:-0.0849887;
#SAMPLESTART:24.56834;
#SAMPLELENGTH:20.000000;
#SELECTABLE:YES;
#CREDIT:Nocturne;

//---------------dance-single - ----------------
#NOTEDATA:;
#STEPSTYPE:dance-single;
#DIFFICULTY:Beginner;
#METER:7;
#CREDIT:Nocturne;
#OFFSET:-0.0849887;
#BPMS:0=600
,1=600
,2=600
,4=600
,5=600
...
//---------------dance-single - ----------------
#NOTEDATA:;
#STEPSTYPE:dance-single;
#DIFFICULTY:Easy;
#METER:25;
#CREDIT:Nocturne;
#OFFSET:-0.0849887;
#BPMS:0=165.014
,1=164.993
,2=165.014
,4=164.993
,5=165.014
...

However, Outfox incorrectly displays the BPMs for all charts of this song as having 60 BPM.

Screen Shot 2022-12-16 at 4 48 51 PM Screen Shot 2022-12-16 at 4 48 52 PM

The exact crafted chart I used is attached below: chart.ssc.zip (I chose not to include the art or mp3 to avoid any potential copyright concerns, but they don't matter for purposes of this bug)

Give an example of what it should look like

The correct BPM ranges should be displayed for each selected chart -- with my crafted file, Beginner should show 600-601 and all other difficulties should show 164-165.

bluebandit21 commented 1 year ago

This is really a trivial trivial issue, but thought I'd open it here since I think I already have the exact fix you'll need for it :)

Etterna had the same problem, outlined in https://github.com/etternagame/etterna/issues/615 ; https://github.com/etternagame/etterna/pull/619 and https://github.com/etternagame/etterna/pull/1217 collectively fix it, although the Theme edits would obviously be slightly different :)

If you want to close this as being too trivial for anyone to even care about, definitely feel more than free to ;)

bluebandit21 commented 1 year ago

Actually, sorry, we just found out that the Etterna fix has a slight problem with it (SetBpmFromSteps never accounted for current song rate from the very beginning), will update this issue when that problem is properly fixed :)

bluebandit21 commented 1 year ago

Ok, I think https://github.com/etternagame/etterna/commit/f3b1919bb0f15f63799dba19bde4febaf4c06340 and https://github.com/etternagame/etterna/commit/c8329f13c7f3fe33448affdc08d8e163da595052 should be the last pieces of the puzzle you'll need :)

Jousway commented 1 year ago

I had a;ready added support for this in lua for lua wheels, but didnt add it to the build in wheel cuz I quite dislike it, I also wrote a system a year ago when we added OSU support for multi chart backgrounds and banners on the build in wheel, so I can easily add bpm to that, thanks for the report tho, will prob be fix in next release