Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Replace indexed skill access with skills.First(s is ...) #30034

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+36 −10
Open

Replace indexed skill access with skills.First(s is ...) #30034

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7589000
Replace indexed skill access with `skills.First(s is ...)`
stanriders Sep 28, 2024
b540f0f
Fix comment
stanriders Sep 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion osu.Game.Rulesets.Catch/Difficulty/CatchDifficultyCalculator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Collections.Generic;
using System.Linq;
using osu.Game.Beatmaps;
using osu.Game.Rulesets.Catch.Beatmaps;
using osu.Game.Rulesets.Catch.Difficulty.Preprocessing;
Expand Down Expand Up @@ -40,7 +41,7 @@ protected override DifficultyAttributes CreateDifficultyAttributes(IBeatmap beat

CatchDifficultyAttributes attributes = new CatchDifficultyAttributes
{
StarRating = Math.Sqrt(skills[0].DifficultyValue()) * difficulty_multiplier,
StarRating = Math.Sqrt(skills.First(s => s is Movement).DifficultyValue()) * difficulty_multiplier,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These should really say .Single().

Mods = mods,
ApproachRate = preempt > 1200.0 ? -(preempt - 1800.0) / 120.0 : -(preempt - 1200.0) / 150.0 + 5.0,
MaxCombo = beatmap.GetMaxCombo(),
Expand Down
2 changes: 1 addition & 1 deletion osu.Game.Rulesets.Mania/Difficulty/ManiaDifficultyCalculator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ protected override DifficultyAttributes CreateDifficultyAttributes(IBeatmap beat

ManiaDifficultyAttributes attributes = new ManiaDifficultyAttributes
{
StarRating = skills[0].DifficultyValue() * difficulty_multiplier,
StarRating = skills.First(s => s is Strain).DifficultyValue() * difficulty_multiplier,
Mods = mods,
// In osu-stable mania, rate-adjustment mods don't affect the hit window.
// This is done the way it is to introduce fractional differences in order to match osu-stable for the time being.
Expand Down
21 changes: 14 additions & 7 deletions osu.Game.Rulesets.Osu/Difficulty/OsuDifficultyCalculator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,22 @@ protected override DifficultyAttributes CreateDifficultyAttributes(IBeatmap beat
if (beatmap.HitObjects.Count == 0)
return new OsuDifficultyAttributes { Mods = mods };

double aimRating = Math.Sqrt(skills[0].DifficultyValue()) * difficulty_multiplier;
double aimRatingNoSliders = Math.Sqrt(skills[1].DifficultyValue()) * difficulty_multiplier;
double speedRating = Math.Sqrt(skills[2].DifficultyValue()) * difficulty_multiplier;
double speedNotes = ((Speed)skills[2]).RelevantNoteCount();
var aim = (Aim)skills.First(s => s is Aim);
var aimWithoutSliders = (AimWithoutSliders)skills.First(s => s is AimWithoutSliders);
var speed = (Speed)skills.First(s => s is Speed);

double aimRating = Math.Sqrt(aim.DifficultyValue()) * difficulty_multiplier;
double aimRatingNoSliders = Math.Sqrt(aimWithoutSliders.DifficultyValue()) * difficulty_multiplier;
double speedRating = Math.Sqrt(speed.DifficultyValue()) * difficulty_multiplier;
double speedNotes = speed.RelevantNoteCount();

double flashlightRating = 0.0;

if (mods.Any(h => h is OsuModFlashlight))
flashlightRating = Math.Sqrt(skills[3].DifficultyValue()) * difficulty_multiplier;
{
var flashlight = (Flashlight)skills.First(s => s is Flashlight);
flashlightRating = Math.Sqrt(flashlight.DifficultyValue()) * difficulty_multiplier;
}

double sliderFactor = aimRating > 0 ? aimRatingNoSliders / aimRating : 1;

Expand Down Expand Up @@ -131,8 +138,8 @@ protected override Skill[] CreateSkills(IBeatmap beatmap, Mod[] mods, double clo
{
var skills = new List<Skill>
{
new Aim(mods, true),
new Aim(mods, false),
new Aim(mods),
new AimWithoutSliders(mods),
new Speed(mods)
};

Expand Down
2 changes: 1 addition & 1 deletion osu.Game.Rulesets.Osu/Difficulty/Skills/Aim.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ namespace osu.Game.Rulesets.Osu.Difficulty.Skills
/// </summary>
public class Aim : OsuStrainSkill
{
public Aim(Mod[] mods, bool withSliders)
public Aim(Mod[] mods, bool withSliders = true)
: base(mods)
{
this.withSliders = withSliders;
Expand Down
18 changes: 18 additions & 0 deletions osu.Game.Rulesets.Osu/Difficulty/Skills/AimWithoutSliders.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
// See the LICENCE file in the repository root for full licence text.

using osu.Game.Rulesets.Mods;

namespace osu.Game.Rulesets.Osu.Difficulty.Skills
{
/// <summary>
/// Represents the Aim skill that does not include sliders length in it's calculation
/// </summary>
public class AimWithoutSliders : Aim
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And this is... kinda weird? Why bother?

Why not expose WithSliders as public readonly on the skill, and then find it where you need it based on that flag alone? As in

skills.OfType<Aim>().SingleOrDefault(aim => aim.WithSliders)

or something.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's mostly for the tools, to be able to distinguish between two aim skills

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that to say they can't distinguish by based on a property for some reason?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right now to display the skill name it just takes the type name. I could add an exception for aim, but I feel like it's an ugly way cos for example if someone works on aim revamping they'll need to remove it, or if someone add another multipass skill it'll need another exception and so on, and that'd need to happen not only in tools but in diffcalc itself. Creating a separate class seems like a more elegant solution to me

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. In general I'd expect that to be handled by a Name property or similar, but I suppose this is fine for now.

{
public AimWithoutSliders(Mod[] mods)
: base(mods, false)
{
}
}
}
Loading