search

flesnuk / oppai5

osu! pp and difficulty calculator, pure Go implementation of https://github.com/Francesco149/oppai-ng
8 stars 5 forks source link

Performance points start going up after getting certain amount of misses #5

Closed Wieku closed 3 years ago

Wieku commented 3 years ago

Video: https://streamable.com/xrbzwr

Map: https://osu.ppy.sh/beatmapsets/351630#osu/774965

Logs (all hits with their pp values are recorded here): danser.log

It's not the only case: image

Wieku commented 3 years ago

After looking again at the footage it seems to break when misses on hitcircles outweigh 300s/100s/50s

flesnuk commented 3 years ago

Just looked at the code and after adding some prints, seems like this line is the culprit (called here)... https://github.com/flesnuk/oppai5/blob/master/accuracy.go#L71

Since nobjects is -1, n300,n100,n50 is 0 and misses more than 1, then nobjects will have the value of -1, which breaks: https://github.com/flesnuk/oppai5/blob/master/accuracy.go#L78-L81

and finally the value returned will be 1.0 (100% accuracy).

Will submit fix for this later today. Thanks for opening and pointing out this issue. I think in the koohai has the same bug if I'm not mistaken.. https://github.com/Francesco149/koohii/blob/master/src/main/java/com/github/francesco149/koohii/Koohii.java#L1477-L1478

flesnuk commented 3 years ago

Please, if this commit makes other PP calcultations off (when there is not a lot of misses I mean), open again and I will check if my changes broke something else.

Wieku commented 3 years ago

Unfortunately the change made pp to always be NaN

Wieku commented 3 years ago

Correction: NaN was a bug in my display routine, but maps starting with sliders give NaN pp before the first hitcircle is scored.

Wieku commented 3 years ago

danser.log

flesnuk commented 3 years ago

I think making it returning 0 should be ok for acc pp, since sliders/spinners doesn't count as 300s. Better than NaN is for sure :)

Wieku commented 3 years ago

Sliders only affect speed and aim pp, so pp won't be 0 even if acc pp is 0, but NaNs as NaNs break everything ;p

flesnuk commented 3 years ago

Yep, NaN should definitely not be returned in any case. Pushed commit with the fix, so closing this for now.

I had this abandoned because I don't have much time, but I know it needs some refactoring and maybe add Go modules. I'm glad its being useful in your project of danser, its pretty cool.

But I will try to debug if you see any major issues with the pp calculation. Thanks.