Repository navigation
[BUGFIX] Fix late judgements at lower framerates - #8370
Pooki-LTSC wants to merge 1 commit into
Conversation
nykwono
left a comment
There was a problem hiding this comment.
Did testing for myself and can definitely confirm this PR helping a lot!
|
This needs a lot of testing and feedback, if you're able to, please give it a try and let me know your opinion even if you don't find any bugs! |
There was a problem hiding this comment.
In general, I think this PR is probably going to get denied, mostly because what its doing is quite (not to be rude), naive.
This is a half-compromise; it seems that the clock the game runs is just simply wrong, and a better idea would probably be an interpolation on songPosition entirely, IE:
var audioPos:Float = 0;
var audioStampMs:Float = 0;
var lastPos:Float = 0;
public function update(?songPos:Float, applyOffsets:Bool = true, forceDispatch:Bool = false):Void
{
var useAudioClock = songPos == null;
var currentTime:Float = (FlxG.sound.music != null) ? FlxG.sound.music.time : 0.0;
var currentLength:Float = (FlxG.sound.music != null) ? FlxG.sound.music.length : 0.0;
if (useAudioClock) songPos = currentTime;
songPos += applyOffsets ? combinedOffset : 0;
if (FlxG.sound.music != null && FlxG.sound.music.playing) songPos = Math.min(this.combinedOffset, 0).clamp(songPos, currentLength);
if (useAudioClock) songPos = interpolate(songPos);
this.songPosition = songPos;
}
function interpolate(rawPos:Float):Float
{
var music = FlxG.sound.music;
if (music == null || !music.playing)
{
audioPos = rawPos;
lastPos = rawPos;
return rawPos;
}
var nowMs = Timer.stamp() * 1000;
if (rawPos != audioPos)
{
audioPos = rawPos;
audioStampMs = nowMs;
}
var estimate = audioPos + (nowMs - audioStampMs) * music.pitch;
// 40 is playstate's resync threshold
if (Math.abs(estimate - rawPos) > 40) estimate = rawPos;
lastPos = Math.max(estimate, lastPos);
return lastPos;
}
// inputs would use this IE: `event.position = Conductor.instance.getPositionNow()`
public function getPositionNow():Float
{
var music = FlxG.sound.music;
if (music == null || !music.playing) return this.songPosition;
var nowMs = Timer.stamp() * 1000;
var estimate = audioPos + (nowMs - audioStampMs) * music.pitch;
if (Math.abs(estimate - this.songPosition) > 50) estimate = this.songPosition;
return Math.max(estimate, lastPos);
}
// notes/strumlines use this
public function getVisualPosition():Float
{
// this takes the code from my original comment and puts it into a SmoothTime class
return getPositionNow() + SmoothTime.getHalfFrameMs();
}
// should be called whenever the song changes (IE, seeking, restarting, pausing, resume, etc)
public function resetInterpolation():Void
{
audioPos = 0;
audioStampMs = Timer.stamp() * 1000;
lastPos = 0;
}(I wrote all of that code inside of GitHub so it's kind of rough, but I hope it gets the point across.)
That idea would be a visual position and an exact position - also lets us create a visual offset later.
To be clear on why I think this PR in its state right now wouldn't be good is:
- It makes judgements frame dependent again (means scoring is dependent on frames)
- It treats the symptom and isn't really a solution
- The visual is still wrong (it doesn't actually change the position that is wrong, you are moving the conductor to the notes and not the notes to the conductor)
- Players playing by ear are now playing slightly early because of this change
| */ | ||
| public static function getHalfFrameMs():Float | ||
| { | ||
| return FlxG.elapsed * Constants.MS_PER_SEC / 2; |
There was a problem hiding this comment.
This would break when the game lags, hitching at all would cause judgements to be completely broken (offset by a lot)
A smooth time average would be a much better fit:
var smoothedMs:Float = -1;
public static function getHalfFrameMs():Float
{
return smoothedMs < 0 ? 0 : smoothedMs / 2;
}
override public function update(elapsed:Float):Void
{
// ...
var dtMs = elapsed * Constants.MS_PER_SEC;
if (smoothedMs < 0)
{
smoothedMs = dtMs;
return;
}
var clamped = Math.min(dtMs, smoothedMs * 2);
smoothedMs += (clamped - smoothedMs) * 0.1;
// ...
}| // Use the input's song position when calculating the ms difference. | ||
| // Also compensate for input latency. | ||
| var noteDiff:Int = Std.int(inputPosition - note.noteData.time - inputLatencyMs); | ||
| var noteDiff:Int = Std.int(inputPosition - note.noteData.time - inputLatencyMs - PreciseInputManager.getHalfFrameMs()); |
There was a problem hiding this comment.
This directly correlates frames to judgements, which is a regression.
|
Completely valid, really looking at things this fix is more of a band-aid solution rather than fixing the actual problem. I see what you mean with the clock being wrong being the cause of all this. I think the approach you came up with would be better. I'd be willing to give it another shot and workshop something over the next couple days using the approach you suggested, unless that's something you and the team would rather handle yourselves. Appreciate the feedback regardless, thank you for taking the time to check it out! |
Linked Issues
Closes #5667
Description
At lower framerates, hitting based on the note's location on screen leads to late judgements, causing people playing at lower framerates to have to hit notes early to get a sick. This PR is my attempt at fixing that issue, despite all other variations of this being declined (for fair reason, this is an annoying issue to fix
)
Previous attempts at this: #5734 #6563
Why does this happen?
The screen only updates once per frame, and each frame stays on screen until the next frame is drawn. Even in-between these frames, the game still moves in the background, causing the image you see to be slightly behind the actual song position. This ends up being anywhere from 0 to 1 full frame behind, which is half a frame on average. (remember this, it will be important later!!)
Hit judgement always uses the actual song position rather than what is being shown on your screen. This can cause a hit that visually lines up perfectly with the strumline to be counted as late by up to a frame, or an average of half a frame.
These graphs show small sections of a note moving towards the strumline. The straight line is the real song position that you're judged on, and the steps are what the screen will show you. As you can see the lower the FPS values, the bigger the steps become and the further behind the screen is from what's actually happening.
Erm... How do we fix it though?
When judging a hit, subtract half of the current frame's length:
PreciseInputManager.getHalfFrameMs()takesFlxG.elapsedin ms and halves it, then returns that valuePlayState.goodNoteHitthen subtracts it fromnoteDiff.OffsetMenu.hitNotealso applies this, but I will say the way the offset menu works is a bit strange so this was harder for me to test.Basically what we're doing here is accounting for the half a frame average from earlier. This does NOT touch hit windows, note positions, or how inputs get recorded at all, it's just an additional factor added to the judgement to account for the issue.
In previous PRs I saw discussion of how much a fix like this would actually affect someone playing at lower FPS, so I wanted to test for myself and see the exact difference it makes. To do this I used a tool that hits every note exactly when it visually aligns with the strumline.
Tested on High (Erect), because that song fuckin rocks
As you can see before the fix hits were late by within 0.5ms of the half frame time. This was also tested with VSync and framerate unlocked, with results within 0.5ms of the ones shown above.
Limitations
This is something I think is very important to cover, this fix matches judgement to the notes visually, which is what I believe to be the right way of going about this. If you were to play purely by ear without looking at the notes, low FPS would make you slightly early. Doing it visually wouldn't work with mods that have custom modcharts or change the note positions. It would also be less accurate during any sort of frame drop or inconsistent FPS.
Helpful Info + Other Stuff
https://exceed7.com/native-audio/rhythm-game-crash-course/index.html#some-numbers-for-timing-value
This helped me a lot in understanding the half-frame issue, so I figured I would link it here.
Big ups to @JackXson-Real and @Lasercar for taking a shot at this issue in the past and documenting things about the issue
Screenshots/Videos
Before:
https://www.youtube.com/watch?v=jPBB3oXka_k
After:
https://www.youtube.com/watch?v=ohzVHQdLlis
Sorry these were way too big for me to upload to github lol