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

[BUGFIX] Fix Audio/Visual Offset causing skips on song start #3732

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

xenkap
Copy link

@xenkap xenkap commented Oct 19, 2024

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:

2024-10-19.17-06-43.mp4

AFTER:

2024-10-19.17-04-39.mp4

(recorded on -210ms audio/visual offset. yes my headphones are that bad, yes they are bluetooth)

Don't use combined offset here-- using it will cause the instrumental to skip forwards due to your offset. Just use instrumental offset, and don't play it when the song starts-- let resyncVocals do that
Conductor's minimum songPosition when music is playing is now combinedOffset.

resyncVocals is also altered a smidge due to the fact that we don't want to apply offsets when resyncing at the start + we don't want to play the music and THEN resync because it makes the sound double up when lag happens on start and it sounds ugly
Throw resyncVocals out of startSong because it complicates matters
@github-actions github-actions bot added size: medium A medium pull request with 100 or fewer changes. pr: haxe PR modifies game code. and removed size: medium A medium pull request with 100 or fewer changes. labels Oct 19, 2024
@xenkap xenkap changed the title Audio visual offset start [BUGFIX] Fix Audio/Visual Offset causing skipping on song start Oct 19, 2024
@xenkap
Copy link
Author

xenkap commented Oct 19, 2024

Currently a weird awkward pause after the countdown before the song plays. Currently investigating

@xenkap xenkap changed the title [BUGFIX] Fix Audio/Visual Offset causing skipping on song start [BUGFIX] Fix Audio/Visual Offset causing skips on song start Oct 19, 2024
@xenkap
Copy link
Author

xenkap commented Oct 19, 2024

okay all good :yippe:

@github-actions github-actions bot added the size: medium A medium pull request with 100 or fewer changes. label Oct 19, 2024
@xenkap
Copy link
Author

xenkap commented Oct 19, 2024

added footage :D

xenkap and others added 2 commits October 30, 2024 20:10
thank you https://github.com/cyn0x8 for reminding me FlxMath.bound exists

Co-authored-by: cyn <cyn0x8+git@gmail.com>
oopsie 👉 👈
@xenkap
Copy link
Author

xenkap commented Nov 6, 2024

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...)

@EliteMasterEric EliteMasterEric added the status: pending triage Awaiting review. label Jan 17, 2025
@NotHyper-474
Copy link
Contributor

#3039 should be linked to this

@Squiddu
Copy link

Squiddu commented Jan 22, 2025

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

@Hundrec Hundrec added the type: minor bug Involves a minor bug or issue. label Jan 22, 2025
@JackXson-Real
Copy link

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.

Copy link
Collaborator

@Hundrec Hundrec left a comment

Choose a reason for hiding this comment

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

Tested this and it does resolve the skipping on start!
Offsets are still far from playable for me, but this makes it a little better :)

Copy link
Member

@EliteMasterEric EliteMasterEric left a comment

Choose a reason for hiding this comment

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

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.

@EliteMasterEric EliteMasterEric added status: needs revision Cannot be approved because it is awaiting some work by the contributor. and removed status: reviewing internally Under consideration and testing. labels Mar 11, 2025
@Hundrec
Copy link
Collaborator

Hundrec commented Mar 11, 2025

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.

@JackXson-Real
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: haxe PR modifies game code. size: medium A medium pull request with 100 or fewer changes. status: needs revision Cannot be approved because it is awaiting some work by the contributor. type: minor bug Involves a minor bug or issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants