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

Remove requirement of base calls to ensure user skin container layouts are retrieved #29564

Merged
merged 8 commits into from
Aug 23, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public void TestLegacyHUDComboCounterNotExistent([Values] bool withModifiedSkin)
if (withModifiedSkin)
{
AddStep("change component scale", () => Player.ChildrenOfType<LegacyScoreCounter>().First().Scale = new Vector2(2f));
AddStep("update target", () => Player.ChildrenOfType<SkinComponentsContainer>().ForEach(LegacySkin.UpdateDrawableTarget));
AddStep("update target", () => Player.ChildrenOfType<SkinnableContainer>().ForEach(LegacySkin.UpdateDrawableTarget));
AddStep("exit player", () => Player.Exit());
CreateTest();
}
Expand Down
2 changes: 1 addition & 1 deletion osu.Game.Rulesets.Catch/CatchSkinComponentLookup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

namespace osu.Game.Rulesets.Catch
{
public class CatchSkinComponentLookup : GameplaySkinComponentLookup<CatchSkinComponents>
public class CatchSkinComponentLookup : SkinComponentLookup<CatchSkinComponents>
{
public CatchSkinComponentLookup(CatchSkinComponents component)
: base(component)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,23 +30,19 @@ public CatchLegacySkinTransformer(ISkin skin)
{
switch (lookup)
{
case SkinComponentsContainerLookup containerLookup:
case GlobalSkinnableContainerLookup containerLookup:
// 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;

// we don't have enough assets to display these components (this is especially the case on a "beatmap" skin).
if (!IsProvidingLegacyResources)
return null;

// Our own ruleset components default.
switch (containerLookup.Target)
switch (containerLookup.Lookup)
{
case SkinComponentsContainerLookup.TargetArea.MainHUDComponents:
case GlobalSkinnableContainers.MainHUDComponents:
// todo: remove CatchSkinComponents.CatchComboCounter and refactor LegacyCatchComboCounter to be added here instead.
return new DefaultSkinComponentsContainer(container =>
{
Expand Down
2 changes: 1 addition & 1 deletion osu.Game.Rulesets.Mania/ManiaSkinComponentLookup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

namespace osu.Game.Rulesets.Mania
{
public class ManiaSkinComponentLookup : GameplaySkinComponentLookup<ManiaSkinComponents>
public class ManiaSkinComponentLookup : SkinComponentLookup<ManiaSkinComponents>
{
/// <summary>
/// Creates a new <see cref="ManiaSkinComponentLookup"/>.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,18 +28,14 @@ public ManiaArgonSkinTransformer(ISkin skin, IBeatmap beatmap)
{
switch (lookup)
{
case SkinComponentsContainerLookup containerLookup:
case GlobalSkinnableContainerLookup containerLookup:
// 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;

switch (containerLookup.Target)
switch (containerLookup.Lookup)
{
case SkinComponentsContainerLookup.TargetArea.MainHUDComponents:
case GlobalSkinnableContainers.MainHUDComponents:
return new DefaultSkinComponentsContainer(container =>
{
var combo = container.ChildrenOfType<ArgonManiaComboCounter>().FirstOrDefault();
Expand All @@ -59,7 +55,7 @@ public ManiaArgonSkinTransformer(ISkin skin, IBeatmap beatmap)

return null;

case GameplaySkinComponentLookup<HitResult> resultComponent:
case SkinComponentLookup<HitResult> resultComponent:
// This should eventually be moved to a skin setting, when supported.
if (Skin is ArgonProSkin && resultComponent.Component >= HitResult.Great)
return Drawable.Empty();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,22 +80,18 @@ public override Drawable GetDrawableComponent(ISkinComponentLookup lookup)
{
switch (lookup)
{
case SkinComponentsContainerLookup containerLookup:
case GlobalSkinnableContainerLookup containerLookup:
// Modifications for global components.
if (containerLookup.Ruleset == null)
return base.GetDrawableComponent(lookup);

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

// we don't have enough assets to display these components (this is especially the case on a "beatmap" skin).
if (!IsProvidingLegacyResources)
return null;

switch (containerLookup.Target)
switch (containerLookup.Lookup)
{
case SkinComponentsContainerLookup.TargetArea.MainHUDComponents:
case GlobalSkinnableContainers.MainHUDComponents:
return new DefaultSkinComponentsContainer(container =>
{
var combo = container.ChildrenOfType<LegacyManiaComboCounter>().FirstOrDefault();
Expand All @@ -114,7 +110,7 @@ public override Drawable GetDrawableComponent(ISkinComponentLookup lookup)

return null;

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

case ManiaSkinComponentLookup maniaComponent:
Expand Down
2 changes: 1 addition & 1 deletion osu.Game.Rulesets.Osu/OsuSkinComponentLookup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

namespace osu.Game.Rulesets.Osu
{
public class OsuSkinComponentLookup : GameplaySkinComponentLookup<OsuSkinComponents>
public class OsuSkinComponentLookup : SkinComponentLookup<OsuSkinComponents>
{
public OsuSkinComponentLookup(OsuSkinComponents component)
: base(component)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public OsuArgonSkinTransformer(ISkin skin)
{
switch (lookup)
{
case GameplaySkinComponentLookup<HitResult> resultComponent:
case SkinComponentLookup<HitResult> resultComponent:
HitResult result = resultComponent.Component;

// This should eventually be moved to a skin setting, when supported.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public OsuTrianglesSkinTransformer(ISkin skin)
{
switch (lookup)
{
case GameplaySkinComponentLookup<HitResult> resultComponent:
case SkinComponentLookup<HitResult> resultComponent:
HitResult result = resultComponent.Component;

switch (result)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,23 +44,19 @@ public OsuLegacySkinTransformer(ISkin skin)
{
switch (lookup)
{
case SkinComponentsContainerLookup containerLookup:
case GlobalSkinnableContainerLookup containerLookup:
// 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;

// we don't have enough assets to display these components (this is especially the case on a "beatmap" skin).
if (!IsProvidingLegacyResources)
return null;

// Our own ruleset components default.
switch (containerLookup.Target)
switch (containerLookup.Lookup)
{
case SkinComponentsContainerLookup.TargetArea.MainHUDComponents:
case GlobalSkinnableContainers.MainHUDComponents:
return new DefaultSkinComponentsContainer(container =>
{
var keyCounter = container.OfType<LegacyKeyCounterDisplay>().FirstOrDefault();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public TaikoArgonSkinTransformer(ISkin skin)
{
switch (lookup)
{
case GameplaySkinComponentLookup<HitResult> resultComponent:
case SkinComponentLookup<HitResult> resultComponent:
// This should eventually be moved to a skin setting, when supported.
if (Skin is ArgonProSkin && resultComponent.Component >= HitResult.Great)
return Drawable.Empty();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public TaikoLegacySkinTransformer(ISkin skin)

public override Drawable? GetDrawableComponent(ISkinComponentLookup lookup)
{
if (lookup is GameplaySkinComponentLookup<HitResult>)
if (lookup is SkinComponentLookup<HitResult>)
{
// if a taiko skin is providing explosion sprites, hide the judgements completely
if (hasExplosion.Value)
Expand Down
2 changes: 1 addition & 1 deletion osu.Game.Rulesets.Taiko/TaikoSkinComponentLookup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

namespace osu.Game.Rulesets.Taiko
{
public class TaikoSkinComponentLookup : GameplaySkinComponentLookup<TaikoSkinComponents>
public class TaikoSkinComponentLookup : SkinComponentLookup<TaikoSkinComponents>
{
public TaikoSkinComponentLookup(TaikoSkinComponents component)
: base(component)
Expand Down
20 changes: 10 additions & 10 deletions osu.Game.Tests/Skins/SkinDeserialisationTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ public void TestDeserialiseModifiedDefault()
var skin = new TestSkin(new SkinInfo(), null, storage);

Assert.That(skin.LayoutInfos, Has.Count.EqualTo(2));
Assert.That(skin.LayoutInfos[SkinComponentsContainerLookup.TargetArea.MainHUDComponents].AllDrawables.ToArray(), Has.Length.EqualTo(9));
Assert.That(skin.LayoutInfos[GlobalSkinnableContainers.MainHUDComponents].AllDrawables.ToArray(), Has.Length.EqualTo(9));
}
}

Expand All @@ -120,8 +120,8 @@ public void TestDeserialiseModifiedArgon()
var skin = new TestSkin(new SkinInfo(), null, storage);

Assert.That(skin.LayoutInfos, Has.Count.EqualTo(2));
Assert.That(skin.LayoutInfos[SkinComponentsContainerLookup.TargetArea.MainHUDComponents].AllDrawables.ToArray(), Has.Length.EqualTo(10));
Assert.That(skin.LayoutInfos[SkinComponentsContainerLookup.TargetArea.MainHUDComponents].AllDrawables.Select(i => i.Type), Contains.Item(typeof(PlayerName)));
Assert.That(skin.LayoutInfos[GlobalSkinnableContainers.MainHUDComponents].AllDrawables.ToArray(), Has.Length.EqualTo(10));
Assert.That(skin.LayoutInfos[GlobalSkinnableContainers.MainHUDComponents].AllDrawables.Select(i => i.Type), Contains.Item(typeof(PlayerName)));
}
}

Expand All @@ -134,10 +134,10 @@ public void TestDeserialiseModifiedClassic()
var skin = new TestSkin(new SkinInfo(), null, storage);

Assert.That(skin.LayoutInfos, Has.Count.EqualTo(2));
Assert.That(skin.LayoutInfos[SkinComponentsContainerLookup.TargetArea.MainHUDComponents].AllDrawables.ToArray(), Has.Length.EqualTo(6));
Assert.That(skin.LayoutInfos[SkinComponentsContainerLookup.TargetArea.SongSelect].AllDrawables.ToArray(), Has.Length.EqualTo(1));
Assert.That(skin.LayoutInfos[GlobalSkinnableContainers.MainHUDComponents].AllDrawables.ToArray(), Has.Length.EqualTo(6));
Assert.That(skin.LayoutInfos[GlobalSkinnableContainers.SongSelect].AllDrawables.ToArray(), Has.Length.EqualTo(1));

var skinnableInfo = skin.LayoutInfos[SkinComponentsContainerLookup.TargetArea.SongSelect].AllDrawables.First();
var skinnableInfo = skin.LayoutInfos[GlobalSkinnableContainers.SongSelect].AllDrawables.First();

Assert.That(skinnableInfo.Type, Is.EqualTo(typeof(SkinnableSprite)));
Assert.That(skinnableInfo.Settings.First().Key, Is.EqualTo("sprite_name"));
Expand All @@ -148,10 +148,10 @@ public void TestDeserialiseModifiedClassic()
using (var storage = new ZipArchiveReader(stream))
{
var skin = new TestSkin(new SkinInfo(), null, storage);
Assert.That(skin.LayoutInfos[SkinComponentsContainerLookup.TargetArea.MainHUDComponents].AllDrawables.ToArray(), Has.Length.EqualTo(8));
Assert.That(skin.LayoutInfos[SkinComponentsContainerLookup.TargetArea.MainHUDComponents].AllDrawables.Select(i => i.Type), Contains.Item(typeof(UnstableRateCounter)));
Assert.That(skin.LayoutInfos[SkinComponentsContainerLookup.TargetArea.MainHUDComponents].AllDrawables.Select(i => i.Type), Contains.Item(typeof(ColourHitErrorMeter)));
Assert.That(skin.LayoutInfos[SkinComponentsContainerLookup.TargetArea.MainHUDComponents].AllDrawables.Select(i => i.Type), Contains.Item(typeof(LegacySongProgress)));
Assert.That(skin.LayoutInfos[GlobalSkinnableContainers.MainHUDComponents].AllDrawables.ToArray(), Has.Length.EqualTo(8));
Assert.That(skin.LayoutInfos[GlobalSkinnableContainers.MainHUDComponents].AllDrawables.Select(i => i.Type), Contains.Item(typeof(UnstableRateCounter)));
Assert.That(skin.LayoutInfos[GlobalSkinnableContainers.MainHUDComponents].AllDrawables.Select(i => i.Type), Contains.Item(typeof(ColourHitErrorMeter)));
Assert.That(skin.LayoutInfos[GlobalSkinnableContainers.MainHUDComponents].AllDrawables.Select(i => i.Type), Contains.Item(typeof(LegacySongProgress)));
}
}

Expand Down
10 changes: 5 additions & 5 deletions osu.Game.Tests/Visual/Gameplay/TestSceneBeatmapSkinFallbacks.cs
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ public partial class TestSceneBeatmapSkinFallbacks : OsuPlayerTestScene
public void TestEmptyLegacyBeatmapSkinFallsBack()
{
CreateSkinTest(TrianglesSkin.CreateInfo(), () => new LegacyBeatmapSkin(new BeatmapInfo(), null));
AddUntilStep("wait for hud load", () => Player.ChildrenOfType<SkinComponentsContainer>().All(c => c.ComponentsLoaded));
AddAssert("hud from default skin", () => AssertComponentsFromExpectedSource(SkinComponentsContainerLookup.TargetArea.MainHUDComponents, skinManager.CurrentSkin.Value));
AddUntilStep("wait for hud load", () => Player.ChildrenOfType<SkinnableContainer>().All(c => c.ComponentsLoaded));
AddAssert("hud from default skin", () => AssertComponentsFromExpectedSource(GlobalSkinnableContainers.MainHUDComponents, skinManager.CurrentSkin.Value));
}

protected void CreateSkinTest(SkinInfo gameCurrentSkin, Func<ISkin> getBeatmapSkin)
Expand All @@ -53,17 +53,17 @@ protected void CreateSkinTest(SkinInfo gameCurrentSkin, Func<ISkin> getBeatmapSk
});
}

protected bool AssertComponentsFromExpectedSource(SkinComponentsContainerLookup.TargetArea target, ISkin expectedSource)
protected bool AssertComponentsFromExpectedSource(GlobalSkinnableContainers target, ISkin expectedSource)
{
var targetContainer = Player.ChildrenOfType<SkinComponentsContainer>().First(s => s.Lookup.Target == target);
var targetContainer = Player.ChildrenOfType<SkinnableContainer>().First(s => s.Lookup.Lookup == target);
var actualComponentsContainer = targetContainer.ChildrenOfType<Container>().SingleOrDefault(c => c.Parent == targetContainer);

if (actualComponentsContainer == null)
return false;

var actualInfo = actualComponentsContainer.CreateSerialisedInfo();

var expectedComponentsContainer = expectedSource.GetDrawableComponent(new SkinComponentsContainerLookup(target)) as Container;
var expectedComponentsContainer = expectedSource.GetDrawableComponent(new GlobalSkinnableContainerLookup(target)) as Container;
if (expectedComponentsContainer == null)
return false;

Expand Down
12 changes: 6 additions & 6 deletions osu.Game.Tests/Visual/Gameplay/TestSceneHUDOverlay.cs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public partial class TestSceneHUDOverlay : OsuManualInputManagerTestScene
private readonly IGameplayClock gameplayClock = new GameplayClockContainer(new TrackVirtual(60000), false, false);

// best way to check without exposing.
private Drawable hideTarget => hudOverlay.ChildrenOfType<SkinComponentsContainer>().First();
private Drawable hideTarget => hudOverlay.ChildrenOfType<SkinnableContainer>().First();
private Drawable keyCounterContent => hudOverlay.ChildrenOfType<KeyCounterDisplay>().First().ChildrenOfType<Drawable>().Skip(1).First();

public TestSceneHUDOverlay()
Expand Down Expand Up @@ -242,8 +242,8 @@ public void TestHiddenHUDDoesntBlockComponentUpdates()

createNew();

AddUntilStep("wait for components to be hidden", () => hudOverlay.ChildrenOfType<SkinComponentsContainer>().Single().Alpha == 0);
AddUntilStep("wait for hud load", () => hudOverlay.ChildrenOfType<SkinComponentsContainer>().All(c => c.ComponentsLoaded));
AddUntilStep("wait for components to be hidden", () => hudOverlay.ChildrenOfType<SkinnableContainer>().Single().Alpha == 0);
AddUntilStep("wait for hud load", () => hudOverlay.ChildrenOfType<SkinnableContainer>().All(c => c.ComponentsLoaded));

AddStep("bind on update", () =>
{
Expand All @@ -260,10 +260,10 @@ public void TestHiddenHUDDoesntBlockSkinnableComponentsLoad()

createNew();

AddUntilStep("wait for components to be hidden", () => hudOverlay.ChildrenOfType<SkinComponentsContainer>().Single().Alpha == 0);
AddUntilStep("wait for components to be hidden", () => hudOverlay.ChildrenOfType<SkinnableContainer>().Single().Alpha == 0);

AddStep("reload components", () => hudOverlay.ChildrenOfType<SkinComponentsContainer>().Single().Reload());
AddUntilStep("skinnable components loaded", () => hudOverlay.ChildrenOfType<SkinComponentsContainer>().Single().ComponentsLoaded);
AddStep("reload components", () => hudOverlay.ChildrenOfType<SkinnableContainer>().Single().Reload());
AddUntilStep("skinnable components loaded", () => hudOverlay.ChildrenOfType<SkinnableContainer>().Single().ComponentsLoaded);
}

private void createNew(Action<HUDOverlay>? action = null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ private void loadPlayer(Func<Ruleset> createRuleset)
AddStep("set ruleset", () => currentRuleset = createRuleset());
AddStep("load player", LoadPlayer);
AddUntilStep("player loaded", () => Player.IsLoaded && Player.Alpha == 1);
AddUntilStep("wait for hud", () => Player.HUDOverlay.ChildrenOfType<SkinComponentsContainer>().All(s => s.ComponentsLoaded));
AddUntilStep("wait for hud", () => Player.HUDOverlay.ChildrenOfType<SkinnableContainer>().All(s => s.ComponentsLoaded));

AddStep("seek to gameplay", () => Player.GameplayClockContainer.Seek(0));
AddUntilStep("wait for seek to finish", () => Player.DrawableRuleset.FrameStableClock.CurrentTime, () => Is.EqualTo(0).Within(500));
Expand Down
Loading
Loading