Skip to content
This repository was archived by the owner on Mar 20, 2025. It is now read-only.

Add mods and ruleset parameters to GetDifficultyAttributesAsync#2

Open
matte-ek wants to merge 7 commits into
minisbett:masterfrom
matte-ek:add-mods-ruleset-to-difficulty-attributes
Open

Add mods and ruleset parameters to GetDifficultyAttributesAsync#2
matte-ek wants to merge 7 commits into
minisbett:masterfrom
matte-ek:add-mods-ruleset-to-difficulty-attributes

Conversation

@matte-ek

@matte-ek matte-ek commented Aug 21, 2024

Copy link
Copy Markdown
Contributor

As per a discussion in a previous PR.

Adds the ability to pass mods and ruleset to the GetDifficultyAttributesAsync method and the ability to send data in the body during a POST request.

@matte-ek matte-ek marked this pull request as draft August 21, 2024 23:27
@matte-ek

Copy link
Copy Markdown
Contributor Author

Marked as draft for now as I noticed the mods will only allow the mods passed in as an integer, and the ability to pass in a string array should probably be added. The API will not accept HD or HDHR as a string.

@matte-ek

matte-ek commented Aug 23, 2024

Copy link
Copy Markdown
Contributor Author

Sorry for being slow with this, been very busy lately and haven't had any time. Anyway I initially thought of doing some easy overloading like:

public Task<DifficultyAttributes?> GetDifficultyAttributesAsync(int id, string? ruleset = null, int? mods = null) => GetDifficultyAttributesInternalAsync(id, ruleset, mods);

public Task<DifficultyAttributes?> GetDifficultyAttributesAsync(int id, string? ruleset = null, string[]? mods = null) => GetDifficultyAttributesInternalAsync(id, ruleset, mods);

private async Task<DifficultyAttributes?> GetDifficultyAttributesInternalAsync(int id, string? ruleset = null, object? mods = null)
{ ... }

However this will have problems with ambiguity if you were trying to do something like GetDifficultyAttributesAsync(<id>). I'm not sure what direction you want to go with this, but I propose either

  • Just rename the method like GetDifficultyAttributesWithModsArrayAsync to separate them
  • Ignore bitwise flags option, just keep the string[] option, would make sense in the current state since we don't have a Mods enum anyway.
  • Add a Mods enum type, which could have an extension like Mods.FromString("HDHR") and/or Mods.FromString(["HD", "HR"]) which you would use instead.

Do you have any other better ideas?

EDIT: Or we could just have another overload without mods at all, not sure why I didn't think of that initially, lack of sleep catching up with me I guess :)

@minisbett

Copy link
Copy Markdown
Owner

What is the object overload for? I'd make one overload for passing the int, one for passing the array and one for passing a mod string.

@minisbett

Copy link
Copy Markdown
Owner

Oh my bad, I just saw that its the private method. Maybe instead just have it's own implementation in each method and then in the end we'll see how we can abstract it

@matte-ek matte-ek force-pushed the add-mods-ruleset-to-difficulty-attributes branch from b962e4b to 067fa71 Compare August 24, 2024 15:56
@matte-ek matte-ek marked this pull request as ready for review August 24, 2024 16:04
@matte-ek

Copy link
Copy Markdown
Contributor Author

I'm pretty happy over how things are right now. Please let me know if you want anything to be changed.

Everything seems to work fine from my quick testing

var score = await OsuApiClient.GetDifficultyAttributesAsync(2833172); // diff-rating: 6.22964001
var scoreMania = await OsuApiClient.GetDifficultyAttributesAsync(2833172, "mania"); // diff-rating: 3.01440001
var scoreHr = await OsuApiClient.GetDifficultyAttributesAsync(2833172, ["HR"]); // diff-rating: 6.45979977
var scoreHrBitset = await OsuApiClient.GetDifficultyAttributesAsync(2833172, 16); // diff-rating: 6.45979977
var scoreDtMania = await OsuApiClient.GetDifficultyAttributesAsync(2833172, ["DT"], "mania"); // diff-rating: 4.05667019

067fa71 is somewhat outside the scope of this PR but is a bug I noticed while retrieving scores, so added it to this PR.

Comment thread OsuSharp/Endpoints/Beatmaps.cs Outdated
Comment thread OsuSharp/Endpoints/Beatmaps.cs Outdated
Comment thread OsuSharp/Endpoints/Beatmaps.cs Outdated
Comment thread OsuSharp/Endpoints/Beatmaps.cs Outdated
Comment thread OsuSharp/Endpoints/Beatmaps.cs Outdated
Comment thread OsuSharp/Endpoints/Beatmaps.cs Outdated
Comment thread OsuSharp/Endpoints/Beatmaps.cs Outdated
Comment thread OsuSharp/Endpoints/Beatmaps.cs Outdated
Comment thread OsuSharp/Endpoints/Beatmaps.cs Outdated
Comment thread OsuSharp/OsuApiClient.cs Outdated
@minisbett

Copy link
Copy Markdown
Owner

Just noticed that the change to use the Ruleset enum would not work because the post request creates the json via JsonContent, and not via Newtonsoft.Json. This should be change such that it uses NSJ for it, and instead a StringContent.

Could you add another overload for specifying strings such as HDDT? That would be done by doing .Chunk(2) on the string and then calling the string array overload on it.

@matte-ek

Copy link
Copy Markdown
Contributor Author

Am I being totally stupid, or has StringEnumConverter::WriteJson ever worked? JsonSerializationException will always be thrown.

@minisbett

minisbett commented Aug 26, 2024

Copy link
Copy Markdown
Owner

Am I being totally stupid, or has StringEnumConverter::WriteJson ever worked? JsonSerializationException will always be thrown.

Never tried it lol. Will have to look into that my bad, saw the commit. lol.

Comment thread OsuSharp/Converters/StringEnumConverter.cs Outdated
Comment thread OsuSharp/Endpoints/Beatmaps.cs Outdated
Comment thread OsuSharp/Endpoints/Beatmaps.cs Outdated
Comment thread OsuSharp/OsuApiClient.cs Outdated
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants