-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Allow beatmap conversion from non-std rulesets #32535
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
base: master
Are you sure you want to change the base?
Allow beatmap conversion from non-std rulesets #32535
Conversation
Mania players need this too. |
Supersedes ppy#23875, fix ppy#9263 Gives the ruleset full control which maps to show both with and without converts enabled - e.g. a competitive/harder version ruleset such as for mania could make mania maps show without enabling converts, as well as control which ones to support only if conversions are enabled in the settings. There is a CanConvert function in BeatmapConverter which I considered using as well, however it looks like this beatmap converter isn't directly accessible in the carousel and adding it may be quite severe on performance impact. Also consider for example generally mania->taiko converts might be good to have, but a quality control function inside CanConvert could make the conversion fail if it's unplayable, so two different interfaces make more sense in that case. This way, maps may also show if they fail in conversion, but playing them will show a notification that the maps aren't playable. For the future an async task that just goes through maps to try and see if they are convertible for different modes and caches that might be useful. I did not change how the default rulesets behave, this is just for custom rulesets to be able to play more maps and start off with a larger pool of maps, which I think is an important part of custom ruleset adoption. I adjusted the scrolling example ruleset (pippidon) to include this as well.
753a015
to
4f4d56b
Compare
depends on ppy/osu#32535
/// If set, overrides which maps can be played and should be shown, instead of only showing maps from our own | ||
/// ruleset and std with map conversion enabled. | ||
/// </summary> | ||
public IRulesetConvertSupport? RulesetConvertSupport { get; set; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason for a separate interface instead of using IRulesetFilterCriteria
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made this a separate interface since the IRulesetFilterCriteria, which currently extends the functionality of the search bar, didn't really feel it'd fit / match what this does very well. (as well as being a breaking change for rulesets / requiring changes in implementation)
This new interface also is just a single function to implement, which makes this easier to implement than IRulesetFilterCriteria.
I think keeping it separate is more logical for rulesets as well as making it easier for rulesets to implement this. I have been using this in 2 separate rulesets now and I think it's working just like you'd expect it to and the introduced API is working pretty well.
Supersedes #23875, fix #9263
Summary:
This gives rulesets full control which maps to show both with and without map conversion enabled.
For example a 2-player version ruleset of taiko could make taiko maps show without enabling converts, as well as control which ones to support only if conversions are enabled in the settings.
Design decisions:
There is a
CanConvert
function inBeatmapConverter
which I considered using as well. However this class wasn't accessible in the carousel and the implementation may be quite severe on performance impact, since they validate the full map, instead of going from metadata. Consider for example generally converted maps from mania to taiko might be able to usually convert well, but a quality control function inside CanConvert could make the conversion fail if it's unplayable, which will then be shown when trying to start the map.I did not change how the default rulesets behave, this is just for custom rulesets to be able to play more maps and start off with a larger pool of maps, which I think is an important part of custom ruleset adoption. I adjusted the scrolling example ruleset (pippidon) to include this as well.
During playtesting I personally found adding a ctb -> mania convertor quite fun, but I would not make that part of this PR since the scope would explode when touching the default rulesets. (scoring, pp, web, how to handle changes in a ranked converter, etc.)