Skip to content

[BUGFIX] Fix Hit Window Being Offset By FPS #5734

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

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

Conversation

JackXson-Real
Copy link
Contributor

@JackXson-Real JackXson-Real commented Aug 12, 2025

Associated Assets PR

FunkinCrew/funkin.assets#226

Linked Issues

Fixes #5667

Description

Fixes the issue with the hit window being offset slightly on lower FPS values. Previously, playing on 500FPS would grant you the most accuracy and have the hit window be closest to the center of the strumline, and playing on lower FPS like 60 would offset the hit window down a bit, making it so that you have to hit the notes much earlier than you would think, making the hit windows inconsistent between FPS. This is an issue that should not be in a rhythm game due to accuracy being key. This PR aims to fix this issue by scaling the offset of the hit window based on your set FPS cap in preferences using an equation. Different FPS values should play nearly identical to each other on this PR with the exception of more input delay on lower FPS, but that can't be helped. Special thanks to @Lasercar for fixing the known issues. Check out his comment to see more videos showcasing the fix on other FPS values. I'm only posting 60fps videos because I'm lazy and it's the most noticeable.

Known Issues

- Sustain trail is affected by the offset causing it to appear above the hold note cover
- Downscroll is currently weirdly affected by the offsets

Screenshots/Videos

60FPS - Before Fix

8mb.video-XlF-Z271MvZj.mp4

60FPS - After Fix

8mb.video-aFo-a3wYhyiV.mp4

@github-actions github-actions bot added status: pending triage Awaiting review. pr: haxe PR modifies game code. size: huge A huge pull request with more than 500 changes. and removed status: pending triage Awaiting review. labels Aug 12, 2025
@JackXson-Real JackXson-Real changed the base branch from main to develop August 12, 2025 16:53
@Hundrec Hundrec marked this pull request as draft August 12, 2025 16:54
@Hundrec Hundrec added size: medium A medium pull request with 100 or fewer changes. topic: help wanted Ideal for work by outside contributors. status: needs revision Cannot be approved because it is awaiting some work by the contributor. type: minor bug Involves a minor bug or issue. and removed size: huge A huge pull request with more than 500 changes. labels Aug 12, 2025
@JackXson-Real JackXson-Real force-pushed the fix-hit-window-fps-offset branch from de75ffa to 7c6efba Compare August 12, 2025 17:40
@github-actions github-actions bot added size: tiny A tiny pull request with 4 or fewer changes. and removed size: medium A medium pull request with 100 or fewer changes. labels Aug 12, 2025
@JackXson-Real JackXson-Real reopened this Aug 12, 2025
@github-actions github-actions bot added size: medium A medium pull request with 100 or fewer changes. and removed size: tiny A tiny pull request with 4 or fewer changes. labels Aug 12, 2025
@Lasercar
Copy link
Contributor

Lasercar commented Aug 13, 2025

Amazing! Will approve once my changes have been merged. These changes should be more than enough until a proper fix can be found for this issue (unless this turns out to be the only way to fix it....).

Tests after Lasercar's changes

30 FPS, Funkin note style:

2025-08-13.17-48-15.mp4

500 FPS, Funkin note style:

2025-08-13.17-51-41.mp4

500 FPS, pixel note style:

2025-08-13.17-57-11.mp4

@JackXson-Real JackXson-Real marked this pull request as ready for review August 14, 2025 16:53
@Hundrec Hundrec changed the title (HELP WANTED) [BUGFIX] Fix Hit Window Being Offset By FPS [BUGFIX] Fix Hit Window Being Offset By FPS Aug 14, 2025
@Hundrec Hundrec removed topic: help wanted Ideal for work by outside contributors. status: needs revision Cannot be approved because it is awaiting some work by the contributor. labels Aug 14, 2025
@Hundrec Hundrec added the status: pending triage Awaiting review. label Aug 14, 2025
Copy link
Member

@Kade-github Kade-github left a comment

Choose a reason for hiding this comment

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

this function is a little unreadable,
image

my only request is to try cleaning this up with readable variables and docs because:

  • Im going to forget what this does
  • Im going to look at this code
  • And im going to close vscode and cry

nice changes tho, just a weird implementation (I feel like this should be something to do with a deltatime value instead of magic numbers and the fps value itself(?), but if it works it works.)

@JackXson-Real
Copy link
Contributor Author

this function is a little unreadable, image

my only request is to try cleaning this up with readable variables and docs because:

  • Im going to forget what this does
  • Im going to look at this code
  • And im going to close vscode and cry

nice changes tho, just a weird implementation (I feel like this should be something to do with a deltatime value instead of magic numbers and the fps value itself(?), but if it works it works.)

Will work on the readability. I'm not entirely sure what deltatime values really are so I cannot do anything like that, but if you guys are able to do something similar internally using those, that would probably also work. This fix can potentially be inaccurate due to the offset being based on your FPS cap, and not the actual FPS itself.

Copy link
Member

@Kade-github Kade-github left a comment

Choose a reason for hiding this comment

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

solid improvement, nice job

will merge once the known issues are fixed

@Hundrec Hundrec added status: reviewing internally Under consideration and testing. and removed status: pending triage Awaiting review. labels Aug 14, 2025
@Kade-github
Copy link
Member

also gotta make sure that @EliteMasterEric and @Hundrec / @AbnormalPoof are good with it as well

@AbnormalPoof
Copy link
Member

image

@Lasercar
Copy link
Contributor

Lasercar commented Aug 16, 2025

solid improvement, nice job

will merge once the known issues are fixed

May I ask what the known issues are?

I was going to approve this myself but there's still more issues?

@Hundrec
Copy link
Member

Hundrec commented Aug 16, 2025

This worked well in my testing, but Eric had some comments about something regarding data
Maybe the implementation just needs to be tweaked for better code

@Lasercar
Copy link
Contributor

It'd be nice to make it use the current FPS, but there currently isn't a way to accurately get it: #4374

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: reviewing internally Under consideration and testing. type: minor bug Involves a minor bug or issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug Report: Lower FPS Values Affect The Hit Window And Judgements/Score
5 participants