search

osuAkatsuki / bancho.py

An osu! server for the generic public, optimized for maintainability in modern python
https://akatsuki.gg
MIT License
216 stars 132 forks source link

Mania converts always return 0.00pp, making overwrites impossible #108

Closed ghost closed 1 year ago

ghost commented 3 years ago

ghost commented 3 years ago

issue resolved in https://github.com/cmyui/gulag/commit/dccbc39d02fbdefa6e96329ee2d11f9f6fed361c? please close if verified resolved, or I will close if I verify myself

infernalfire72 commented 3 years ago

not resolved, it does not replace the score since it checks for newScorePP > oldScorePP and since Maniera does not have cv support, 0 > 0 == false, therefore will never replace the first score you have set

infernalfire72 commented 3 years ago

How to resolve this (without full cv calc): generally check for score on mania submission. This will not have much drawback imo since pp is generally dependent on score in mania Another idea is having multiple layers of "new best score assertion"

cmyui commented 2 years ago

Sorry i didn't get around to this for so long - so basically it looks like the issue is convert pp on mania not working and always returning 0.00? is this also the case for other gamemodes converted pp? I can do some investigation myself as well.

cmyui commented 2 years ago

Another idea is having multiple layers of "new best score assertion"

Yea we should likely have multiple layers; on very difficult maps since pp is truncated, it's possible for people to repetitively get 0.00pp even if there is a working difficulty calculator, so there should be a fallback (likely to score, and perhaps further steps beyond that)

juup1ter commented 2 years ago

Sorry i didn't get around to this for so long - so basically it looks like the issue is convert pp on mania not working and always returning 0.00? is this also the case for other gamemodes converted pp? I can do some investigation myself as well.

doesn't seem to be an issue on taiko or catch the beat -- purely a mania issue

tsunyoku commented 2 years ago

i think going forward the best bet is probably not using peace performance (or maniera if we still use that, i don’t remember). we use it on akatsuki and it has caused us multiple issues on other modes and it doesn’t look like it’s being maintained anymore either. i think we should look into using https://github.com/MaxOhn/rosu-pp as it is still maintained, has up to date reworks that peace doesn’t have. it also has python bindings available via pip

cmyui commented 2 years ago

does rosu-pp support converted maps?

alowave223 commented 2 years ago

yes, it is

tsunyoku commented 2 years ago

does rosu-pp support converted maps?

yep!

cmyui commented 2 years ago

it'd definitely be a good move forwards then.

eventually, i'd like to make our pp calculator use consistent - e.g. rather than using multiple different libraries/programs for pp calculation, to combine it into a single, consistent interface. the code could be simplified GREATLY from this, it's one of my pet peeves with osu server codebases as a whole at the moment.

tsunyoku commented 2 years ago

rosu-pp is a good gateway for this as its really easy to edit (and compile), but would lose the ability of using the pip package directly. in an ideal world we wouldn’t have to have the end user do any compiling for changes but too much of a perfect world. however rosu-pp is pretty well setup for making changes which is why the new WIP akatsuki system is using it since it allows us to use a single calculator across all modes (including rx)

Pure-Peace commented 2 years ago

rosu-pp is a good gateway for this as its really easy to edit (and compile), but would lose the ability of using the pip package directly. in an ideal world we wouldn’t have to have the end user do any compiling for changes but too much of a perfect world. however rosu-pp is pretty well setup for making changes which is why the new WIP akatsuki system is using it since it allows us to use a single calculator across all modes (including rx)

The latest rosu-pp-py seems to already support installation via pip: pip install rosu-pp-py

We can try switching to rosu-pp-py, or keep using peace-performance-python, now it will also build with the latest rosu-pp code instead of peace-performance.

The latest version of peace-performance-python is now 2.0.0.

tsunyoku commented 2 years ago

bancho.py has already switched to rosu-pp!

rosu-pp is a good gateway for this as its really easy to edit (and compile), but would lose the ability of using the pip package directly. in an ideal world we wouldn’t have to have the end user do any compiling for changes but too much of a perfect world. however rosu-pp is pretty well setup for making changes which is why the new WIP akatsuki system is using it since it allows us to use a single calculator across all modes (including rx)

The latest rosu-pp-py seems to already support installation via pip: pip install rosu-pp-py

We can try switching to rosu-pp-py, or keep using peace-performance-python, now it will also build with the latest rosu-pp code instead of peace-performance.

The latest version of peace-performance-python is now 2.0.0.

Pure-Peace commented 2 years ago

bancho.py has already switched to rosu-pp!

rosu-pp is a good gateway for this as its really easy to edit (and compile), but would lose the ability of using the pip package directly. in an ideal world we wouldn’t have to have the end user do any compiling for changes but too much of a perfect world. however rosu-pp is pretty well setup for making changes which is why the new WIP akatsuki system is using it since it allows us to use a single calculator across all modes (including rx)

The latest rosu-pp-py seems to already support installation via pip: pip install rosu-pp-py We can try switching to rosu-pp-py, or keep using peace-performance-python, now it will also build with the latest rosu-pp code instead of peace-performance. The latest version of peace-performance-python is now 2.0.0.

Currently this repository still uses peace-performance-python=1.1.2 requirements.txt and some users reported the same issue to me.

So a temporary solution is necessary

7mochi commented 1 year ago

I think we can close this because it has been fixed in #315

alowave223 commented 1 year ago

Fixed in #315