From c4f7dee82bec2e7f7d7339ffc2fe2b22cc5e3f02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 11 Dec 2025 05:40:48 +0100 Subject: [PATCH] Fix skin editor sometimes dropping anchor/origin specification on paste (#35957) * Add failing test for copy->paste not being idempotent * Ensure all elements on default skins use fixed anchors `UsesFixedAnchor` defaults to false, i.e. closest anchors. Combined with manual anchor / origin specs on some drawables, this would get default skins into impossible states wherein a drawable would use "closest anchor" but also explicitly specify anchor / origin that aren't closest, which horribly fails on attempting to copy and paste. Frankly shocked this has gone unnoticed for this long, and I regret not vetoing this "feature" more every time I see its tentacles spread to produce breakage of levels yet unseen. Does this commit contain major levels of suck? For sure. Do I have any better ideas that wouldn't consist of a multi-day rewrite or deletion of this "feature"? No. * Fix skin editor always applying closest anchor / origin on paste regardless of whether the component uses fixed anchor Self-explanatory. Should close https://github.com/ppy/osu/issues/29111 along with previous commit. --- .../Legacy/CatchLegacySkinTransformer.cs | 3 +++ .../Argon/ManiaArgonSkinTransformer.cs | 3 +++ .../Legacy/ManiaLegacySkinTransformer.cs | 3 +++ .../Legacy/OsuLegacySkinTransformer.cs | 3 +++ .../Argon/TaikoArgonSkinTransformer.cs | 3 +++ .../Default/TaikoTrianglesSkinTransformer.cs | 3 +++ .../Legacy/TaikoLegacySkinTransformer.cs | 3 +++ .../Visual/Gameplay/TestSceneSkinEditor.cs | 18 ++++++++++++++++++ osu.Game/Overlays/SkinEditor/SkinEditor.cs | 4 +++- osu.Game/Skinning/ArgonSkin.cs | 6 ++++++ osu.Game/Skinning/LegacySkin.cs | 6 ++++++ osu.Game/Skinning/TrianglesSkin.cs | 6 ++++++ 12 files changed, 60 insertions(+), 1 deletion(-) diff --git a/osu.Game.Rulesets.Catch/Skinning/Legacy/CatchLegacySkinTransformer.cs b/osu.Game.Rulesets.Catch/Skinning/Legacy/CatchLegacySkinTransformer.cs index 4f9048b988..4704e83b76 100644 --- a/osu.Game.Rulesets.Catch/Skinning/Legacy/CatchLegacySkinTransformer.cs +++ b/osu.Game.Rulesets.Catch/Skinning/Legacy/CatchLegacySkinTransformer.cs @@ -72,6 +72,9 @@ namespace osu.Game.Rulesets.Catch.Skinning.Legacy leaderboard.Origin = Anchor.CentreLeft; leaderboard.X = 10; } + + foreach (var d in container.OfType()) + d.UsesFixedAnchor = true; }) { Children = new Drawable[] diff --git a/osu.Game.Rulesets.Mania/Skinning/Argon/ManiaArgonSkinTransformer.cs b/osu.Game.Rulesets.Mania/Skinning/Argon/ManiaArgonSkinTransformer.cs index a71b8aa982..81cc52e925 100644 --- a/osu.Game.Rulesets.Mania/Skinning/Argon/ManiaArgonSkinTransformer.cs +++ b/osu.Game.Rulesets.Mania/Skinning/Argon/ManiaArgonSkinTransformer.cs @@ -57,6 +57,9 @@ namespace osu.Game.Rulesets.Mania.Skinning.Argon if (spectatorList != null) spectatorList.Position = new Vector2(36, -66); + + foreach (var d in container.OfType()) + d.UsesFixedAnchor = true; }) { new DrawableGameplayLeaderboard(), diff --git a/osu.Game.Rulesets.Mania/Skinning/Legacy/ManiaLegacySkinTransformer.cs b/osu.Game.Rulesets.Mania/Skinning/Legacy/ManiaLegacySkinTransformer.cs index addb96d2c3..690705664d 100644 --- a/osu.Game.Rulesets.Mania/Skinning/Legacy/ManiaLegacySkinTransformer.cs +++ b/osu.Game.Rulesets.Mania/Skinning/Legacy/ManiaLegacySkinTransformer.cs @@ -122,6 +122,9 @@ namespace osu.Game.Rulesets.Mania.Skinning.Legacy leaderboard.Origin = Anchor.CentreLeft; leaderboard.X = 10; } + + foreach (var d in container.OfType()) + d.UsesFixedAnchor = true; }) { new LegacyManiaComboCounter(), diff --git a/osu.Game.Rulesets.Osu/Skinning/Legacy/OsuLegacySkinTransformer.cs b/osu.Game.Rulesets.Osu/Skinning/Legacy/OsuLegacySkinTransformer.cs index 7118b6f95e..219e754dcc 100644 --- a/osu.Game.Rulesets.Osu/Skinning/Legacy/OsuLegacySkinTransformer.cs +++ b/osu.Game.Rulesets.Osu/Skinning/Legacy/OsuLegacySkinTransformer.cs @@ -103,6 +103,9 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy leaderboard.Origin = Anchor.BottomLeft; leaderboard.Position = pos; } + + foreach (var d in container.OfType()) + d.UsesFixedAnchor = true; }) { Children = new Drawable[] diff --git a/osu.Game.Rulesets.Taiko/Skinning/Argon/TaikoArgonSkinTransformer.cs b/osu.Game.Rulesets.Taiko/Skinning/Argon/TaikoArgonSkinTransformer.cs index b0a1c5d3f7..820af4ce54 100644 --- a/osu.Game.Rulesets.Taiko/Skinning/Argon/TaikoArgonSkinTransformer.cs +++ b/osu.Game.Rulesets.Taiko/Skinning/Argon/TaikoArgonSkinTransformer.cs @@ -51,6 +51,9 @@ namespace osu.Game.Rulesets.Taiko.Skinning.Argon spectatorList.Anchor = Anchor.BottomLeft; spectatorList.Origin = Anchor.TopLeft; } + + foreach (var d in container.OfType()) + d.UsesFixedAnchor = true; }) { RelativeSizeAxes = Axes.Both, diff --git a/osu.Game.Rulesets.Taiko/Skinning/Default/TaikoTrianglesSkinTransformer.cs b/osu.Game.Rulesets.Taiko/Skinning/Default/TaikoTrianglesSkinTransformer.cs index f627417889..1a73457c67 100644 --- a/osu.Game.Rulesets.Taiko/Skinning/Default/TaikoTrianglesSkinTransformer.cs +++ b/osu.Game.Rulesets.Taiko/Skinning/Default/TaikoTrianglesSkinTransformer.cs @@ -51,6 +51,9 @@ namespace osu.Game.Rulesets.Taiko.Skinning.Default spectatorList.Origin = Anchor.TopLeft; spectatorList.Position = new Vector2(320, -280); } + + foreach (var d in container.OfType()) + d.UsesFixedAnchor = true; }) { RelativeSizeAxes = Axes.Both, diff --git a/osu.Game.Rulesets.Taiko/Skinning/Legacy/TaikoLegacySkinTransformer.cs b/osu.Game.Rulesets.Taiko/Skinning/Legacy/TaikoLegacySkinTransformer.cs index 73d32a7933..a985bd362e 100644 --- a/osu.Game.Rulesets.Taiko/Skinning/Legacy/TaikoLegacySkinTransformer.cs +++ b/osu.Game.Rulesets.Taiko/Skinning/Legacy/TaikoLegacySkinTransformer.cs @@ -79,6 +79,9 @@ namespace osu.Game.Rulesets.Taiko.Skinning.Legacy spectatorList.Origin = Anchor.TopLeft; spectatorList.Position = pos; } + + foreach (var d in container.OfType()) + d.UsesFixedAnchor = true; }) { new LegacyDefaultComboCounter(), diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSkinEditor.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSkinEditor.cs index 97889eea4d..3b8e63c596 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneSkinEditor.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneSkinEditor.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.IO; using System.Linq; using System.Text; +using Newtonsoft.Json; using NUnit.Framework; using osu.Framework.Allocation; using osu.Framework.Extensions; @@ -378,6 +379,23 @@ namespace osu.Game.Tests.Visual.Gameplay () => Is.EqualTo(3)); } + [Test] + public void TestCopyPasteIdempotency() + { + string state = null!; + AddStep("select everything", () => InputManager.Keys(PlatformAction.SelectAll)); + AddStep("dump state", () => + { + state = JsonConvert.SerializeObject(skinEditor.SelectedComponents.Cast().Select(s => s.CreateSerialisedInfo()).ToArray()); + }); + AddStep("copy", () => InputManager.Keys(PlatformAction.Copy)); + AddStep("delete", () => InputManager.Keys(PlatformAction.Delete)); + AddStep("paste", () => InputManager.Keys(PlatformAction.Paste)); + AddAssert("pasted state equals dumped", + () => JsonConvert.SerializeObject(skinEditor.SelectedComponents.Cast().Select(s => s.CreateSerialisedInfo()).ToArray()), + () => Is.EqualTo(state)); + } + private SkinnableContainer globalHUDTarget => Player.ChildrenOfType() .Single(c => c.Lookup.Lookup == GlobalSkinnableContainers.MainHUDComponents && c.Lookup.Ruleset == null); diff --git a/osu.Game/Overlays/SkinEditor/SkinEditor.cs b/osu.Game/Overlays/SkinEditor/SkinEditor.cs index 823456dddd..2903c867f7 100644 --- a/osu.Game/Overlays/SkinEditor/SkinEditor.cs +++ b/osu.Game/Overlays/SkinEditor/SkinEditor.cs @@ -529,7 +529,9 @@ namespace osu.Game.Overlays.SkinEditor } SelectedComponents.Add(component); - SkinSelectionHandler.ApplyClosestAnchorOrigin(drawableComponent); + + if (!component.UsesFixedAnchor) + SkinSelectionHandler.ApplyClosestAnchorOrigin(drawableComponent); return true; } diff --git a/osu.Game/Skinning/ArgonSkin.cs b/osu.Game/Skinning/ArgonSkin.cs index 9e8fe4f617..6d922e1402 100644 --- a/osu.Game/Skinning/ArgonSkin.cs +++ b/osu.Game/Skinning/ArgonSkin.cs @@ -128,6 +128,9 @@ namespace osu.Game.Skinning if (spectatorList != null) spectatorList.Position = pos; + + foreach (var d in container.OfType()) + d.UsesFixedAnchor = true; }) { RelativeSizeAxes = Axes.Both, @@ -238,6 +241,9 @@ namespace osu.Game.Skinning keyCounter.Position = new Vector2(-(hitError.Width + padding), -(padding * 2 + song_progress_offset_height)); } } + + foreach (var d in container.OfType()) + d.UsesFixedAnchor = true; } }) { diff --git a/osu.Game/Skinning/LegacySkin.cs b/osu.Game/Skinning/LegacySkin.cs index 6ff569869a..74b404a578 100644 --- a/osu.Game/Skinning/LegacySkin.cs +++ b/osu.Game/Skinning/LegacySkin.cs @@ -412,6 +412,9 @@ namespace osu.Game.Skinning leaderboard.Origin = Anchor.CentreLeft; leaderboard.X = 10; } + + foreach (var d in container.OfType()) + d.UsesFixedAnchor = true; }) { new LegacyDefaultComboCounter(), @@ -448,6 +451,9 @@ namespace osu.Game.Skinning hitError.Origin = Anchor.CentreLeft; hitError.Rotation = -90; } + + foreach (var d in container.OfType()) + d.UsesFixedAnchor = true; }) { Children = new Drawable[] diff --git a/osu.Game/Skinning/TrianglesSkin.cs b/osu.Game/Skinning/TrianglesSkin.cs index 3881a5e970..ae3df35383 100644 --- a/osu.Game/Skinning/TrianglesSkin.cs +++ b/osu.Game/Skinning/TrianglesSkin.cs @@ -106,6 +106,9 @@ namespace osu.Game.Skinning spectatorList.Origin = Anchor.BottomLeft; spectatorList.Position = new Vector2(screen_edge_padding, -(song_progress_offset_height + screen_edge_padding)); } + + foreach (var d in container.OfType()) + d.UsesFixedAnchor = true; }) { RelativeSizeAxes = Axes.Both, @@ -178,6 +181,9 @@ namespace osu.Game.Skinning keyCounter.Origin = Anchor.BottomRight; keyCounter.Position = new Vector2(-screen_edge_padding, -(song_progress_offset_height + screen_edge_padding)); } + + foreach (var d in container.OfType()) + d.UsesFixedAnchor = true; }) { Children = new Drawable[]