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

Convert the global offset from seconds to beats before using it in TimingData calculations #302

Closed
wants to merge 1 commit into from

Conversation

sukibaby
Copy link
Contributor

@sukibaby sukibaby commented Jun 21, 2024

I've listed @phantom10111 as a co-author on this commit for their work on PR #300. Thank you!

The theory here is that converting the global offset to beats, instead of mixing beats and seconds, should keep the sync tighter when rate modding a song.

I'd like this to be tested by someone who has a better sense of timing accuracy than I do, but I think it's a better solution than going back to the old code (which seems to only work at the song level) or keeping the current code (Which seems to only work at the engine level).


The if-else statements inside TimingData::GetElapsedTimeFromBeat(float fBeat) and TimingData::GetBeatAndBPSFromElapsedTime(GetBeatArgs& args) only execute the new code if m_fMusicRate is anything other than 1. They are necessary because stuff will break theme side if it's not done like this. Without the if-else statements, this will happen.

The theory here is that converting the global offset to beats, instead of mixing beats and seconds, should keep the sync tighter when rate modding a song.

The if-else statements inside TimingData::GetElapsedTimeFromBeat(float fBeat) and TimingData::GetBeatAndBPSFromElapsedTime(GetBeatArgs& args) only execute the new code if m_fMusicRate is anything other than 1. They are necessary because stuff will break theme side if it's not done like this. Without the if-else statements, the NPS display and density graph will be incorrect (at least in Simply Love).

phantom10111 is listed as a co-author for their work on itgmania#300

Co-Authored-By: phantom10111 <3482004+phantom10111@users.noreply.github.com>
@teejusb
Copy link
Collaborator

teejusb commented Jun 24, 2024

I reallllllyy dont think we should be converting to beats for global offset stuff. If anything, we should be using only seconds since global offset is in seconds. Also if you only look at BPM and not stops i thinnnkk you're gonna get incorrect values on a lot of charts where the BPM is set to like 100000 and then counteracted with stops. This solution does not seem robust so I'd like to think about this some more.

@sukibaby sukibaby closed this Jun 27, 2024
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.

None yet

2 participants