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

Fix mania combo counter positioning break on centre anchor #29529

Merged
merged 2 commits into from
Aug 22, 2024
Merged

Fix mania combo counter positioning break on centre anchor #29529

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
a3234e2
Add failing test case
frenzibyte Aug 20, 2024
4d74625
Fix mania combo counter positioning break on centre anchor
frenzibyte Aug 20, 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
10 changes: 5 additions & 5 deletions osu.Game.Rulesets.Mania.Tests/Skinning/ManiaSkinnableTestScene.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ namespace osu.Game.Rulesets.Mania.Tests.Skinning
public abstract partial class ManiaSkinnableTestScene : SkinnableTestScene
{
[Cached(Type = typeof(IScrollingInfo))]
private readonly TestScrollingInfo scrollingInfo = new TestScrollingInfo();
protected readonly TestScrollingInfo ScrollingInfo = new TestScrollingInfo();

[Cached]
private readonly StageDefinition stage = new StageDefinition(4);
Expand All @@ -30,7 +30,7 @@ public abstract partial class ManiaSkinnableTestScene : SkinnableTestScene

protected ManiaSkinnableTestScene()
{
scrollingInfo.Direction.Value = ScrollingDirection.Down;
ScrollingInfo.Direction.Value = ScrollingDirection.Down;

Add(new Box
{
Expand All @@ -43,16 +43,16 @@ protected ManiaSkinnableTestScene()
[Test]
public void TestScrollingDown()
{
AddStep("change direction to down", () => scrollingInfo.Direction.Value = ScrollingDirection.Down);
AddStep("change direction to down", () => ScrollingInfo.Direction.Value = ScrollingDirection.Down);
}

[Test]
public void TestScrollingUp()
{
AddStep("change direction to up", () => scrollingInfo.Direction.Value = ScrollingDirection.Up);
AddStep("change direction to up", () => ScrollingInfo.Direction.Value = ScrollingDirection.Up);
}

private class TestScrollingInfo : IScrollingInfo
protected class TestScrollingInfo : IScrollingInfo
{
public readonly Bindable<ScrollingDirection> Direction = new Bindable<ScrollingDirection>();

Expand Down
75 changes: 66 additions & 9 deletions osu.Game.Rulesets.Mania.Tests/Skinning/TestSceneComboCounter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
// Copyright (c) ppy Pty Ltd <[email protected]>. Licensed under the MIT Licence.
// See the LICENCE file in the repository root for full licence text.

using NUnit.Framework;
using osu.Framework.Allocation;
using osu.Framework.Graphics;
using osu.Framework.Graphics.Containers;
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.Rulesets.UI.Scrolling;
using osu.Game.Skinning;

namespace osu.Game.Rulesets.Mania.Tests.Skinning
Expand All @@ -17,22 +21,75 @@ public partial class TestSceneComboCounter : ManiaSkinnableTestScene
[Cached]
private ScoreProcessor scoreProcessor = new ScoreProcessor(new ManiaRuleset());

[SetUpSteps]
public void SetUpSteps()
[Test]
public void TestDisplay()
{
AddStep("setup", () => SetContents(s =>
setup(Anchor.Centre);
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 }));
}

[Test]
public void TestAnchorOrigin()
{
AddStep("set direction down", () => ScrollingInfo.Direction.Value = ScrollingDirection.Down);
setup(Anchor.TopCentre, 20);
AddStep("set direction up", () => ScrollingInfo.Direction.Value = ScrollingDirection.Up);
check(Anchor.BottomCentre, -20);

AddStep("set direction up", () => ScrollingInfo.Direction.Value = ScrollingDirection.Up);
setup(Anchor.BottomCentre, -20);
AddStep("set direction down", () => ScrollingInfo.Direction.Value = ScrollingDirection.Down);
check(Anchor.TopCentre, 20);

AddStep("set direction down", () => ScrollingInfo.Direction.Value = ScrollingDirection.Down);
setup(Anchor.Centre, 20);
AddStep("set direction up", () => ScrollingInfo.Direction.Value = ScrollingDirection.Up);
check(Anchor.Centre, 20);

AddStep("set direction up", () => ScrollingInfo.Direction.Value = ScrollingDirection.Up);
setup(Anchor.Centre, -20);
AddStep("set direction down", () => ScrollingInfo.Direction.Value = ScrollingDirection.Down);
check(Anchor.Centre, -20);
}

private void setup(Anchor anchor, float y = 0)
{
AddStep($"setup {anchor} {y}", () => SetContents(s =>
{
var container = new Container
{
RelativeSizeAxes = Axes.Both,
};

if (s is ArgonSkin)
return new ArgonManiaComboCounter();
container.Add(new ArgonManiaComboCounter());
else if (s is LegacySkin)
container.Add(new LegacyManiaComboCounter());
else
container.Add(new LegacyManiaComboCounter());

if (s is LegacySkin)
return new LegacyManiaComboCounter();
container.Child.Anchor = anchor;
container.Child.Origin = Anchor.Centre;
container.Child.Y = y;

return new LegacyManiaComboCounter();
return container;
}));
}

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 }));
private void check(Anchor anchor, float y)
{
AddAssert($"check {anchor} {y}", () =>
{
foreach (var combo in this.ChildrenOfType<ISerialisableDrawable>())
{
var drawableCombo = (Drawable)combo;
if (drawableCombo.Anchor != anchor || drawableCombo.Y != y)
return false;
}

return true;
});
}
}
}
10 changes: 5 additions & 5 deletions osu.Game.Rulesets.Mania/Skinning/Argon/ArgonManiaComboCounter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,11 @@ protected override void LoadComplete()
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;
}
if (Anchor.HasFlag(Anchor.y1))
return;

Anchor &= ~(Anchor.y0 | Anchor.y2);
Anchor |= direction.Value == ScrollingDirection.Up ? Anchor.y2 : Anchor.y0;

// change the sign of the Y coordinate in line with the scrolling direction.
// i.e. if the user changes direction from down to up, the anchor is changed from top to bottom, and the Y is flipped from positive to negative here.
Expand Down
18 changes: 9 additions & 9 deletions osu.Game.Rulesets.Mania/Skinning/Legacy/LegacyManiaComboCounter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) ppy Pty Ltd <[email protected]>. Licensed under the MIT Licence.
// See the LICENCE file in the repository root for full licence text.

using System;
using osu.Framework.Allocation;
using osu.Framework.Bindables;
using osu.Framework.Graphics;
Expand Down Expand Up @@ -44,16 +45,15 @@ protected override void LoadComplete()
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;
}
if (Anchor.HasFlag(Anchor.y1))
return;

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;
// change the sign of the Y coordinate in line with the scrolling direction.
// i.e. if the user changes direction from down to up, the anchor is changed from top to bottom, and the Y is flipped from positive to negative here.
Y = Math.Abs(Y) * (direction.Value == ScrollingDirection.Up ? -1 : 1);
}

protected override void OnCountIncrement()
Expand Down
Loading