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.

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

Add argon/classic osu!mania combo counter #26254

Merged
merged 23 commits into from
Aug 15, 2024
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
78cb6b6
Abstractify `LegacyComboCounter` to re-use for mania
frenzibyte Dec 30, 2023
ece532b
Add legacy mania combo counter lookups
frenzibyte Dec 30, 2023
8be3f4f
Add legacy mania combo counter implementation
frenzibyte Dec 30, 2023
408287e
Add very basic argon mania combo counter implementation
frenzibyte Dec 30, 2023
95961dc
Add various visual test coverage
frenzibyte Dec 30, 2023
01219fa
Disable "closest" anchor in mania combo counter for convenience
frenzibyte Dec 30, 2023
02f5ea2
Fix failing tests
frenzibyte Dec 30, 2023
e645651
Merge branch 'master' into mania-combo-counter
peppy Aug 8, 2024
80c8140
Update in line with new changes
peppy Aug 8, 2024
7666e8b
Remove `SupportsClosestAnchor` for the time being
peppy Aug 9, 2024
3f20f05
Remove unnecessary `UsesFixedAnchor` specifications
peppy Aug 9, 2024
161734a
Simplify argon mania combo counter implementation by sharing with bas…
peppy Aug 9, 2024
f6ada68
Fix migration failure due to change in class name
peppy Aug 9, 2024
d2eb6cc
Standardise skin transformer code structure
peppy Aug 12, 2024
dd9705b
Fix file access test failure by forcing retries
peppy Aug 14, 2024
e465049
Merge branch 'master' into mania-combo-counter
peppy Aug 14, 2024
7c142bc
Fix incorrect anchor handling in `ArgonManiaComboCounter`
peppy Aug 14, 2024
ff1ab2b
Remove position-flipping logic from mania combo counters for now
frenzibyte Aug 15, 2024
3a4546d
Remove `x` symbol from argon mania combo counter
frenzibyte Aug 15, 2024
66adddb
Actually bring back position-flipping logic and disable "closest" anchor
frenzibyte Aug 15, 2024
a421231
Fix beatmap skin on mania breaking HUD apart
frenzibyte Aug 15, 2024
358572e
Update code order to match everything else
frenzibyte Aug 15, 2024
7427237
Try make code look better
frenzibyte Aug 15, 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
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public void TestLegacyHUDComboCounterNotExistent([Values] bool withModifiedSkin)
CreateTest();
}

AddAssert("legacy HUD combo counter not added", () => !Player.ChildrenOfType<LegacyComboCounter>().Any());
AddAssert("legacy HUD combo counter not added", () => !Player.ChildrenOfType<LegacyDefaultComboCounter>().Any());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
using System.Linq;
using osu.Framework.Bindables;
using osu.Framework.Graphics;
using osu.Framework.Graphics.Containers;
using osu.Game.Skinning;
using osuTK;
using osuTK.Graphics;
Expand Down Expand Up @@ -37,7 +36,7 @@ public CatchLegacySkinTransformer(ISkin skin)

// Modifications for global components.
if (containerLookup.Ruleset == null)
return base.GetDrawableComponent(lookup) as Container;
return base.GetDrawableComponent(lookup);

// Skin has configuration.
if (base.GetDrawableComponent(lookup) is UserConfiguredLayoutContainer d)
Expand Down
38 changes: 38 additions & 0 deletions osu.Game.Rulesets.Mania.Tests/Skinning/TestSceneComboCounter.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// 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.Framework.Allocation;
using osu.Framework.Testing;
using osu.Game.Rulesets.Judgements;
using osu.Game.Rulesets.Mania.Skinning.Argon;
using osu.Game.Rulesets.Mania.Skinning.Legacy;
using osu.Game.Rulesets.Objects;
using osu.Game.Rulesets.Scoring;
using osu.Game.Skinning;

namespace osu.Game.Rulesets.Mania.Tests.Skinning
{
public partial class TestSceneComboCounter : ManiaSkinnableTestScene
{
[Cached]
private ScoreProcessor scoreProcessor = new ScoreProcessor(new ManiaRuleset());

[SetUpSteps]
public void SetUpSteps()
{
AddStep("setup", () => SetContents(s =>
{
if (s is ArgonSkin)
return new ArgonManiaComboCounter();

if (s is LegacySkin)
return new LegacyManiaComboCounter();

return new LegacyManiaComboCounter();
}));

AddRepeatStep("perform hit", () => scoreProcessor.ApplyResult(new JudgementResult(new HitObject(), new Judgement()) { Type = HitResult.Great }), 20);
AddStep("perform miss", () => scoreProcessor.ApplyResult(new JudgementResult(new HitObject(), new Judgement()) { Type = HitResult.Miss }));
}
}
}
13 changes: 13 additions & 0 deletions osu.Game.Rulesets.Mania.Tests/Skinning/TestScenePlayfield.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,22 @@

using System.Collections.Generic;
using NUnit.Framework;
using osu.Framework.Allocation;
using osu.Game.Beatmaps;
using osu.Game.Rulesets.Judgements;
using osu.Game.Rulesets.Mania.Beatmaps;
using osu.Game.Rulesets.Mania.UI;
using osu.Game.Rulesets.Objects;
using osu.Game.Rulesets.Scoring;
using osuTK;

namespace osu.Game.Rulesets.Mania.Tests.Skinning
{
public partial class TestScenePlayfield : ManiaSkinnableTestScene
{
[Cached]
private ScoreProcessor scoreProcessor = new ScoreProcessor(new ManiaRuleset());

private List<StageDefinition> stageDefinitions = new List<StageDefinition>();

[Test]
Expand All @@ -29,6 +36,9 @@ public void TestSingleStage()
Child = new ManiaPlayfield(stageDefinitions)
});
});

AddRepeatStep("perform hit", () => scoreProcessor.ApplyResult(new JudgementResult(new HitObject(), new Judgement()) { Type = HitResult.Perfect }), 20);
AddStep("perform miss", () => scoreProcessor.ApplyResult(new JudgementResult(new HitObject(), new Judgement()) { Type = HitResult.Miss }));
}

[TestCase(2)]
Expand All @@ -54,6 +64,9 @@ public void TestDualStages(int columnCount)
}
});
});

AddRepeatStep("perform hit", () => scoreProcessor.ApplyResult(new JudgementResult(new HitObject(), new Judgement()) { Type = HitResult.Perfect }), 20);
AddStep("perform miss", () => scoreProcessor.ApplyResult(new JudgementResult(new HitObject(), new Judgement()) { Type = HitResult.Miss }));
}

protected override IBeatmap CreateBeatmapForSkinProvider()
Expand Down
36 changes: 36 additions & 0 deletions osu.Game.Rulesets.Mania.Tests/TestSceneManiaPlayerLegacySkin.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// 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 NUnit.Framework;
using osu.Game.Beatmaps;
using osu.Game.Rulesets.Mania.Mods;
using osu.Game.Rulesets.Mods;
using osu.Game.Tests.Beatmaps;
using osu.Game.Tests.Visual;

namespace osu.Game.Rulesets.Mania.Tests
{
public partial class TestSceneManiaPlayerLegacySkin : LegacySkinPlayerTestScene
{
protected override Ruleset CreatePlayerRuleset() => new ManiaRuleset();

// play with a converted beatmap to allow dual stages mod to work.
protected override IBeatmap CreateBeatmap(RulesetInfo ruleset) => new TestBeatmap(new RulesetInfo());

protected override bool HasCustomSteps => true;

[Test]
public void TestSingleStage()
{
AddStep("Load single stage", LoadPlayer);
AddUntilStep("player loaded", () => Player.IsLoaded && Player.Alpha == 1);
}

[Test]
public void TestDualStage()
{
AddStep("Load dual stage", () => LoadPlayer(new Mod[] { new ManiaModDualStages() }));
AddUntilStep("player loaded", () => Player.IsLoaded && Player.Alpha == 1);
}
}
}
42 changes: 42 additions & 0 deletions osu.Game.Rulesets.Mania/Skinning/Argon/ArgonManiaComboCounter.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// 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.Framework.Allocation;
using osu.Framework.Bindables;
using osu.Framework.Graphics;
using osu.Game.Rulesets.UI.Scrolling;
using osu.Game.Screens.Play.HUD;

namespace osu.Game.Rulesets.Mania.Skinning.Argon
{
public partial class ArgonManiaComboCounter : ArgonComboCounter
frenzibyte marked this conversation as resolved.
Show resolved Hide resolved
{
[Resolved]
private IScrollingInfo scrollingInfo { get; set; } = null!;

private IBindable<ScrollingDirection> direction = null!;

protected override void LoadComplete()
{
base.LoadComplete();

direction = scrollingInfo.Direction.GetBoundCopy();
direction.BindValueChanged(_ => updateAnchor());

// two schedules are required so that updateAnchor is executed in the next frame,
// which is when the combo counter receives its Y position by the default layout in ArgonManiaSkinTransformer.
Comment on lines +33 to +34
Copy link
Member

Choose a reason for hiding this comment

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

really dunno but willing to overlook for now because it works.

Schedule(() => Schedule(updateAnchor));
}

private void updateAnchor()
{
Anchor &= ~(Anchor.y0 | Anchor.y2);
Anchor |= direction.Value == ScrollingDirection.Up ? Anchor.y2 : Anchor.y0;
Copy link
Member Author

Choose a reason for hiding this comment

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

The legacy part handles the case where the anchor is set to center, but this one doesn't?

Copy link
Member

Choose a reason for hiding this comment

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

I didn't change this 🙈

Copy link
Member Author

Choose a reason for hiding this comment

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

Whoops 😅

Copy link
Member

Choose a reason for hiding this comment

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

I've updated this to match but there still seems to be something weird going on here.

  • Set scroll direction to "Down"
  • Put the combo counter near the bottom of the screen
  • Change scroll direction

The combo won't move the first time changing direction.

Copy link
Member Author

@frenzibyte frenzibyte Aug 15, 2024

Choose a reason for hiding this comment

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

Alleviated the issue in 66adddb. This is the reason why "Closest" anchor was disabled in this PR (in response to #26254 (comment)).

To explain briefly, I am using the sign of the Y coordinate as an indicator for whether the combo counter is in the correct position or not. When the scroll direction is "Down", the anchor is set to top and the Y position is made sure to be positive. When the scroll direction is "Up", the anchor is set to bottom and the Y position is made sure to be negative.

When the "Closest" anchor is selected, if the user moves their combo counter from top to bottom or vice versa, the anchor will change, and defies everything that the logic above does, therefore breaking apart.


// since we flip the vertical anchor when changing scroll direction,
// we can use the sign of the Y value as an indicator to make the combo counter displayed correctly.
if ((Y < 0 && direction.Value == ScrollingDirection.Down) || (Y > 0 && direction.Value == ScrollingDirection.Up))
Y = -Y;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@
// See the LICENCE file in the repository root for full licence text.

using System;
using System.Linq;
using osu.Framework.Bindables;
using osu.Framework.Graphics;
using osu.Framework.Testing;
using osu.Game.Beatmaps;
using osu.Game.Rulesets.Mania.Beatmaps;
using osu.Game.Rulesets.Scoring;
Expand All @@ -26,6 +28,34 @@ public ManiaArgonSkinTransformer(ISkin skin, IBeatmap beatmap)
{
switch (lookup)
{
case SkinComponentsContainerLookup containerLookup:
if (containerLookup.Target != SkinComponentsContainerLookup.TargetArea.MainHUDComponents)
return base.GetDrawableComponent(lookup);

// Only handle per ruleset defaults here.
if (containerLookup.Ruleset == null)
return base.GetDrawableComponent(lookup);

// Skin has configuration.
if (base.GetDrawableComponent(lookup) is UserConfiguredLayoutContainer d)
return d;
frenzibyte marked this conversation as resolved.
Show resolved Hide resolved

return new DefaultSkinComponentsContainer(container =>
{
var combo = container.ChildrenOfType<ArgonManiaComboCounter>().FirstOrDefault();

if (combo != null)
{
combo.ShowLabel.Value = false;
combo.Anchor = Anchor.TopCentre;
combo.Origin = Anchor.Centre;
combo.Y = 200;
}
})
{
new ArgonManiaComboCounter(),
};

case GameplaySkinComponentLookup<HitResult> resultComponent:
// This should eventually be moved to a skin setting, when supported.
if (Skin is ArgonProSkin && resultComponent.Component >= HitResult.Great)
Expand Down
91 changes: 91 additions & 0 deletions osu.Game.Rulesets.Mania/Skinning/Legacy/LegacyManiaComboCounter.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
// 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.Framework.Allocation;
using osu.Framework.Bindables;
using osu.Framework.Graphics;
using osu.Game.Rulesets.UI.Scrolling;
using osu.Game.Skinning;
using osuTK;
using osuTK.Graphics;

namespace osu.Game.Rulesets.Mania.Skinning.Legacy
{
public partial class LegacyManiaComboCounter : LegacyComboCounter
{
[BackgroundDependencyLoader]
private void load(ISkinSource skin)
{
DisplayedCountText.Anchor = Anchor.Centre;
DisplayedCountText.Origin = Anchor.Centre;

PopOutCountText.Anchor = Anchor.Centre;
PopOutCountText.Origin = Anchor.Centre;
PopOutCountText.Colour = skin.GetManiaSkinConfig<Color4>(LegacyManiaSkinConfigurationLookups.ComboBreakColour)?.Value ?? Color4.Red;
}

[Resolved]
private IScrollingInfo scrollingInfo { get; set; } = null!;

private IBindable<ScrollingDirection> direction = null!;

protected override void LoadComplete()
{
base.LoadComplete();

direction = scrollingInfo.Direction.GetBoundCopy();
direction.BindValueChanged(_ => updateAnchor());

// two schedules are required so that updateAnchor is executed in the next frame,
// which is when the combo counter receives its Y position by the default layout in LegacyManiaSkinTransformer.
Schedule(() => Schedule(updateAnchor));
}

private void updateAnchor()
{
// if the anchor isn't a vertical center, set top or bottom anchor based on scroll direction
if (!Anchor.HasFlag(Anchor.y1))
{
Anchor &= ~(Anchor.y0 | Anchor.y2);
Anchor |= direction.Value == ScrollingDirection.Up ? Anchor.y2 : Anchor.y0;
}

// since we flip the vertical anchor when changing scroll direction,
// we can use the sign of the Y value as an indicator to make the combo counter displayed correctly.
if ((Y < 0 && direction.Value == ScrollingDirection.Down) || (Y > 0 && direction.Value == ScrollingDirection.Up))
Y = -Y;
}

protected override void OnCountIncrement()
{
base.OnCountIncrement();

PopOutCountText.Hide();
DisplayedCountText.ScaleTo(new Vector2(1f, 1.4f))
.ScaleTo(new Vector2(1f), 300, Easing.Out)
.FadeIn(120);
}

protected override void OnCountChange()
{
base.OnCountChange();

PopOutCountText.Hide();
DisplayedCountText.ScaleTo(1f);
}

protected override void OnCountRolling()
{
if (DisplayedCount > 0)
{
PopOutCountText.Text = FormatCount(DisplayedCount);
PopOutCountText.FadeTo(0.8f).FadeOut(200)
.ScaleTo(1f).ScaleTo(4f, 200);

DisplayedCountText.FadeTo(0.5f, 300);
}

base.OnCountRolling();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@

using System;
using System.Collections.Generic;
using System.Linq;
using osu.Framework.Audio.Sample;
using osu.Framework.Bindables;
using osu.Framework.Graphics;
using osu.Framework.Testing;
using osu.Game.Audio;
using osu.Game.Beatmaps;
using osu.Game.Rulesets.Mania.Beatmaps;
Expand Down Expand Up @@ -78,6 +80,33 @@ public override Drawable GetDrawableComponent(ISkinComponentLookup lookup)
{
switch (lookup)
{
case SkinComponentsContainerLookup containerLookup:
if (containerLookup.Target != SkinComponentsContainerLookup.TargetArea.MainHUDComponents)
return base.GetDrawableComponent(lookup);

// Modifications for global components.
if (containerLookup.Ruleset == null)
return base.GetDrawableComponent(lookup);

// Skin has configuration.
if (base.GetDrawableComponent(lookup) is UserConfiguredLayoutContainer d)
return d;

return new DefaultSkinComponentsContainer(container =>
{
var combo = container.ChildrenOfType<LegacyManiaComboCounter>().FirstOrDefault();

if (combo != null)
{
combo.Anchor = Anchor.TopCentre;
combo.Origin = Anchor.Centre;
combo.Y = this.GetManiaSkinConfig<float>(LegacyManiaSkinConfigurationLookups.ComboPosition)?.Value ?? 0;
}
})
{
new LegacyManiaComboCounter(),
};

case GameplaySkinComponentLookup<HitResult> resultComponent:
return getResult(resultComponent.Component);

Expand Down
Loading
Loading