FunkinCrew
[BUGFIX] Fix Audio/Visual Offset causing skips on song start
Starting the song previously utilized combinedOffset to offset where in the song the instrumental starts in. This is no longer the case, instead using instrumentalOffset for starting.
The Conductor and Countdown classes have been altered to reflect this behavior.
function startSong also no longer uses resyncVocals(). It was kind of pointless being there, and needing to play the instrumental before calling it makes the audio double up sometimes and it sounds awful
Does this PR close any issues? If so, link them below.
[TBA, but quick searches about offset don't show much]
Briefly describe the issue(s) fixed.
Audio used to skip forward when negative values of Audio/Visual Offset were present. This is now fixed, along with ugly audio doubling behavior on start.
Include any relevant screenshots or videos.
BEFORE:
https://github.com/user-attachments/assets/74f905e3-6318-4549-85c9-b3990d94f32e
AFTER:
https://github.com/user-attachments/assets/67a3f160-e111-4f41-8b93-4a6e80c10850
(recorded on -210ms audio/visual offset. yes my headphones are that bad, yes they are bluetooth)
Currently a weird awkward pause after the countdown before the song plays. Currently investigating
Not sure if this is an issue on my end specifically, but after switching to Linux I've found myself using positive visual offset. It seems to have an awkward pause at the end of the countdown like it did before-- can someone else see if this happens to them too? NOTE: Also check if this is a linux-exclusive issue?????? Seems like it happens on any input offset...
~~Also, squashing these commits. Kinda messy~~
Having an unreasonably difficult time trying to squash these. Why on earth is GH Desktop so buggy on Linux??
~~(probably because it isn't officially supported...)~~
https://github.com/FunkinCrew/Funkin/issues/3039 should be linked to this
don't know entirely if this fixes everything recorded in the issue, but thank GOD someone else noticed something was wrong, thank you i am free
Just did some testing, and it works flawlessly! Can't wait for this one to be merged. It's been a pain in the ass dealing with this issue.
Just tested this out on Eggnog Erect like in the example, the audio skip is resolved but I can't hit Boyfriend's first down note! It just doesn't respond to inputs at all.
Yeah, I encountered this issue consistently when I tested it a few weeks ago After some discussion with @JackXson-Real, we decided this PR fixed the issue as promised, though it leaves notes at the beginning of the song impossible to hit.
Just tested this out on Eggnog Erect like in the example, the audio skip is resolved but I can't hit Boyfriend's first down note! It just doesn't respond to inputs at all.
This PR fixes the issue with the audio skipping exclusively. The issue with high input offset on Eggnog Erect exists on the develop branch as well, so it was not an issue introduced by this PR. I believe this should be merged as is and then hopefully someone can make a PR that fixes that issue.
Oh! This actually got looked at. I kind of went inactive a while after I made these PRs... It's [being unable to hit starting notes] probably already fixed internally in the update coming in probably less than a day, but I'm gonna see what I can do anyway
Just tested this out on Eggnog Erect like in the example, the audio skip is resolved but I can't hit Boyfriend's first down note! It just doesn't respond to inputs at all.
Yeah, I encountered this issue consistently when I tested it a few weeks ago After some discussion with @JackXson-Real, we decided this PR fixed the issue as promised, though it leaves notes at the beginning of the song impossible to hit.
Couldn't replicate the described problem - tried higher, lower, exactly 100ms offsets. Disabled all note-related mods to be sure, 0 input offset, made sure I was on the right branch. I could hit the Eggnog Erect note just fine -- maybe some commit upstream fixed it while I merged????
It seems to be working fine on my machine, anyhow, so if the issue persists, do respond with your offset configs I have a feeling that it might be unrelated to this PR and instead related to Input Offsets, which I've fixed in a separate PR
Just tested this out on Eggnog Erect like in the example, the audio skip is resolved but I can't hit Boyfriend's first down note! It just doesn't respond to inputs at all.
Yeah, I encountered this issue consistently when I tested it a few weeks ago After some discussion with @JackXson-Real, we decided this PR fixed the issue as promised, though it leaves notes at the beginning of the song impossible to hit.
Couldn't replicate the described problem - tried higher, lower, exactly 100ms offsets. Disabled all note-related mods to be sure, 0 input offset, made sure I was on the right branch. I could hit the Eggnog Erect note just fine -- maybe some commit upstream fixed it while I merged????
It seems to be working fine on my machine, anyhow, so if the issue persists, do respond with your offset configs I have a feeling that it might be unrelated to this PR and instead related to Input Offsets, which I've fixed in a separate PR
We got the bug on 200ms offset.
We got the bug on 200ms offset.
Still can't replicate it... I can hit the note like normal [Trying to attach a video, but can't atm]
Are you sure you didn't set input offset instead while testing? (& any mods/other PRs that might interfere?) Because I tried that and had outcomes that match the description (seemingly unable to hit the note)
From my testing, all combos of -200ms offsets (one, the other, or both) didn't allow for the first note to be hit I'll go test this again later with all the combos
I tested the fixes provided here as well as in #3708 along with managing to replicate the issue. Here's what -200 visual offset and 200 input offset looks like right now, both in 0.5.3 and 0.6.0 (clip is from 0.5.3 but I tested it on 0.6.0 as well and it looked the exact same -- note just flies by and hit windows seem unplayable):
https://github.com/user-attachments/assets/fe8e0dca-8a38-42dd-9486-8204007a1203
And here's what it looks like after applying both pull requests (I assume that's how you say it lol, I'm not really a good coder I just replaced the stuff in the code that was changed and compiled it) with the same settings:
https://github.com/user-attachments/assets/83c5bb6e-b35f-4484-8f89-34c4938cd39b
So I can confirm the issue is still present and the fixes do work perfectly for me, at least for the 0.5.3 version
Edit: I seem to have misunderstood it just a little, I can test it right now with just visual offset as well though
Edit 2: Here's just -200 visual offset on 0.5.3 -- it just straight up skips over the note so you can't even hit it with a regular hit window:
https://github.com/user-attachments/assets/7e110e38-758b-41cd-b81d-7590d8d3863c
I tested the fixes provided here as well as in #3708 along with managing to replicate the issue. Here's what -200 visual offset and 200 input offset looks like right now, both in 0.5.3 and 0.6.0 (clip is from 0.5.3 but I tested it on 0.6.0 as well and it looked the exact same -- note just flies by and hit windows seem unplayable): ...
:yippe: So it is just the input offset causing it! This PR doesn't touch that [not its job] but the other PR (https://github.com/FunkinCrew/Funkin/pull/3708) fixes it. The reason this happens is because the input offset is shifting the rating windows but not the hit windows. It's wack lol Please review again 🥺
Rebased and cleaned up the PR's commit history, Hoping for a re-review considering it seems like PRs are currently being checked outright now :pray: (p.s. Is it okay/appropriate to use github's "Re-request review" feature here?)
For this one, aside from general testing to make sure this doesn't break anything, how does this work when playing Senpai BF mix with the Pico alt instrumental? Currently that one has an unresolved bug where it skips forward slightly and this might fix that.
For this one, aside from general testing to make sure this doesn't break anything, how does this work when playing Senpai BF mix with the Pico alt instrumental? Currently that one has an unresolved bug where it skips forward slightly and this might fix that.
It seems to play the exact same as it would when playing the Pico version, so it is synced properly. Are you talking about the half second intro that Senpai Pico has in the OST version, but is not there in-game? Or did you mean to say Roses?
@EliteMasterEric Here is a video showing the behavior of Roses BF with alt inst. It doesn't really seem to fix that issue, but just makes it happen slightly different (it plays the full intro instead, but vocals are still delayed to work with the song)
https://github.com/user-attachments/assets/f99523d9-a343-4f89-a4e4-9325c5b6f97e
@EliteMasterEric Here is a video showing the behavior of Roses BF with alt inst. It doesn't really seem to fix that issue, but just makes it happen slightly different (it plays the full intro instead, but vocals are still delayed to work with the song)
Oh! Haven't seen this issue. Going to see if I can do anything about this, but what is the intended behavior? Should it skip the instrumental's intro or should it wait like my PR did? :3c
@EliteMasterEric Here is a video showing the behavior of Roses BF with alt inst. It doesn't really seem to fix that issue, but just makes it happen slightly different (it plays the full intro instead, but vocals are still delayed to work with the song)
Oh! Haven't seen this issue. Going to see if I can do anything about this, but what is the intended behavior? Should it skip the instrumental's intro or should it wait like my PR did? :3c
Me and Hundrec believe it works better if the full instrumental plays. The main issue with it currently is the initial audio skip at the beginning, and restarting the song makes senpai’s first set of notes not appear.
Me and Hundrec believe it works better if the full instrumental plays. The main issue with it currently is the initial audio skip at the beginning, and restarting the song makes senpai’s first set of notes not appear.
Found it, lol
At least I think so, and adding the offsets here should fix it! Going to test...
Okay I'm lost again :sob: goose chase this is
OK, found the actual cause! Since this check is based on the song's position (the instrumental) and disregards the vocal offset, it erroneously concludes that the vocals are "too far in the past" to load.
HOWEVER!! This makes things complicated, and makes me ask: why are altVocals offsets specified per-character? Why are they separate from the altInstrumental offset?? And, most importantly, how do we account for that here? :sob:
(tl;dr need clarification on what "altInstrumental" and "altVocals" fields in the chart metadata is supposed to do) (p.s. why do these offsets require a text editor to work? why are they inaccessible in the chart editor :sob:)
Setting offsets in the instrumental and offsets separately is used in the case where the vocals aren't properly synced to the instrumental, so you don't need to wait for the musician to re-export
Also, for the record, the correct behavior is that when Roses is played with Pico's instrumental, the full intro should play and the chart and vocals should start at the appropriate time as configured by the vocal offset. Some versions of the game start Roses Pico inst 4 seconds into the song, which a bug.
Also, for the record, the correct behavior is that when Roses is played with Pico's instrumental, the full intro should play and the chart and vocals should start at the appropriate time as configured by the vocal offset. Some versions of the game start Roses Pico inst 4 seconds into the song, which a bug.
After some debugging... it appears https://github.com/FunkinCrew/Funkin/pull/4875/ fixes the notes disappearing! I'll probably post some explanation there or on the issue tracking it (https://github.com/FunkinCrew/Funkin/issues/4839) so I don't clutter this PR's comments further :]c
EDIT: Done! :+1:
After some debugging... it appears # 4875 fixes the notes disappearing! I'll probably post some explanation there or on the issue tracking it (#4839) so I don't clutter this PR's comments further :]c
@Hundrec Should this PR be considered as fixing the same issue? As it seems to fix the song skipping ahead as well When I checked there wasn't a separate issue specifically mentioning it (except maybe https://github.com/FunkinCrew/Funkin/issues/4404, but that's marked as solved for 0.6.3!)