Skip to content

Playback speed not being used fix #4182

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

Conversation

Lasercar
Copy link
Contributor

@Lasercar Lasercar commented Feb 19, 2025

Does this PR close any issues? If so, link them below.

Fixes #4176

Briefly describe the issue(s) fixed.

The change to the conductor to fix the resync didn't account for if the music's pitch was different. This PR fixes that.

Maybe the check isn't needed and the music time can be used instead? IDK.

Include any relevant screenshots or videos.

2025-02-19.20-56-22.1.mp4
Bonus: Ugh at 2 times playback speed
2025-02-19.21-05-39.1.mp4

@github-actions github-actions bot added status: pending triage Awaiting review. pr: haxe PR modifies game code. size: small A small pull request with 10 or fewer changes. labels Feb 19, 2025
@Hundrec Hundrec added the type: minor bug Involves a minor bug or issue. label Feb 19, 2025
@JackXson-Real
Copy link
Contributor

This fixes the issue of the song being in swing mode, but unfortunately its still broken.

Base.Profile.2025.02.19.-.20.12.19.02.mp4

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.

Works well for me!

@JackXson-Real
Copy link
Contributor

Im going to do some addition testing to see if I can figure out the cause of this issue for me. It seems to work fine for @Hundrec

@JackXson-Real
Copy link
Contributor

JackXson-Real commented Feb 20, 2025

This fixes the issue of the song being in swing mode, but unfortunately its still broken.

Base.Profile.2025.02.19.-.20.12.19.02.mp4

Ok I found what causes the issue. Having visual offsets set to anything other than 0 causes this skipping to happen. I bet if PR #3732 and this were in the same build, I wouldnt have the issue.

Edit: Just tested the above mentioned PR and it seems to fix my issue. Once both of these issues are merged together, there should be no issues. I will approve this PR.

@Lasercar
Copy link
Contributor Author

Lasercar commented Feb 20, 2025

This fixes the issue of the song being in swing mode, but unfortunately its still broken.
Base.Profile.2025.02.19.-.20.12.19.02.mp4

Ok I found what causes the issue. Having visual offsets set to anything other than 0 causes this skipping to happen. I bet if PR #3732 and this were in the same build, I wouldnt have the issue.

Edit: Just tested the above mentioned PR and it seems to fix my issue. Once both of these issues are merged together, there should be no issues. I will approve this PR.

Yeah, the issue you encountered was just fixed by this #4171

It was a funny issue (an abnormal one, even), and I was still in the middle of trying to fix the bug so I just forgot to update my fork. By the time I noticed when I was getting a video for it, I just thought it'd be funnier if I just left it as so. It's not like it currently conflicts with anything, right?

@Lasercar Lasercar changed the base branch from develop to main February 22, 2025 01:14
@Lasercar Lasercar changed the base branch from main to develop February 22, 2025 01:14
@EliteMasterEric EliteMasterEric self-assigned this Feb 22, 2025
@EliteMasterEric EliteMasterEric added status: reviewing internally Under consideration and testing. and removed status: pending triage Awaiting review. labels Feb 22, 2025
@Lasercar
Copy link
Contributor Author

Lasercar commented Mar 16, 2025

Hundrec's busy testing this Conductor.instance.update(FlxG.sound.music.time + elapsed * 1000, false);

It seems to fix both the song not ending issue that the change introduced, this one and the stuttering issues, though it needs more testing.

If it can be fully confirmed to fix all three, I'll change this PR appropriately.

Edit: It does, but it breaks GF's neck, so maybe not.

@Lasercar
Copy link
Contributor Author

Lasercar commented Mar 16, 2025

Superseded by #4334

(if you don't mind or can fix the weird issue hundrec found)

@Lasercar Lasercar closed this Mar 16, 2025
@Hundrec Hundrec added status: rejected Issue did not pass review or PR cannot be approved. and removed type: minor bug Involves a minor bug or issue. status: reviewing internally Under consideration and testing. size: small A small pull request with 10 or fewer changes. pr: haxe PR modifies game code. labels Mar 16, 2025
@Lasercar Lasercar reopened this Mar 16, 2025
@github-actions github-actions bot added size: small A small pull request with 10 or fewer changes. pr: haxe PR modifies game code. labels Mar 16, 2025
@Hundrec Hundrec added type: minor bug Involves a minor bug or issue. status: reviewing internally Under consideration and testing. labels Mar 16, 2025
@Lasercar Lasercar mentioned this pull request Mar 17, 2025
@Hundrec Hundrec 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 Apr 5, 2025
@Hundrec
Copy link
Collaborator

Hundrec commented Apr 5, 2025

Merge conflicts: 6

@Lasercar Lasercar force-pushed the playback-speed-not-being-used-fix branch from 8fef945 to c7834ee Compare April 5, 2025 05:45
@Lasercar
Copy link
Contributor Author

Lasercar commented Apr 5, 2025

Merge conflicts: Fixed!

@Hundrec Hundrec added status: pending triage Awaiting review. and removed status: needs revision Cannot be approved because it is awaiting some work by the contributor. labels Apr 5, 2025
@TechnikTil
Copy link
Contributor

Sorry for causing this-ish😅

@Lasercar Lasercar force-pushed the playback-speed-not-being-used-fix branch from c7834ee to aaefc8b Compare April 29, 2025 09:44
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.

So it turns out GF or Nene has a seizure with this change...

chud.mp4

@Lasercar
Copy link
Contributor Author

Lasercar commented Apr 30, 2025

So it turns out GF or Nene has a seizure with this change...
chud.mp4

yeah uh, I tried investigating how to fix this for the conductor-fixed but I couldn't figure it out. I was able to discover that whatever calls the onStepHit for boppers seems to starts doing it multiple times when you do the pause/unpause mash, though.

Feel free to add help-wanted to this and/or conductor-fixed

@Hundrec Hundrec added the topic: help wanted Ideal for work by outside contributors. label May 1, 2025
@Hundrec Hundrec added status: needs revision Cannot be approved because it is awaiting some work by the contributor. and removed status: pending triage Awaiting review. labels May 11, 2025
@EliteMasterEric EliteMasterEric added size: tiny A tiny pull request with 4 or fewer changes. and removed size: small A small pull request with 10 or fewer changes. labels May 14, 2025
@Lasercar
Copy link
Contributor Author

#5107

@Lasercar Lasercar closed this May 17, 2025
@Hundrec Hundrec added status: duplicate Issue or PR is redundant to another. and removed type: minor bug Involves a minor bug or issue. topic: help wanted Ideal for work by outside contributors. status: needs revision Cannot be approved because it is awaiting some work by the contributor. pr: haxe PR modifies game code. labels May 17, 2025
Copy link

This pull request is a duplicate. Please direct all discussion to the original pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: tiny A tiny pull request with 4 or fewer changes. status: duplicate Issue or PR is redundant to another.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug Report: Playback speed does some weird (cool) stuff in playtest mode
5 participants