From c452715bf190e7aaa8aef11612cd0f56e35b5561 Mon Sep 17 00:00:00 2001 From: Robin Avery Date: Sun, 6 Jun 2021 01:18:30 -0400 Subject: [PATCH 01/39] Allow skin elements to find closest anchor - Resolves ppy/osu#13252 - Add localisation strings for the context menu instead of using enum --- osu.Game/Localisation/SkinEditorStrings.cs | 74 ++++++ .../Skinning/Editor/SkinSelectionHandler.cs | 215 +++++++++++++++--- 2 files changed, 262 insertions(+), 27 deletions(-) create mode 100644 osu.Game/Localisation/SkinEditorStrings.cs diff --git a/osu.Game/Localisation/SkinEditorStrings.cs b/osu.Game/Localisation/SkinEditorStrings.cs new file mode 100644 index 0000000000..24a077963f --- /dev/null +++ b/osu.Game/Localisation/SkinEditorStrings.cs @@ -0,0 +1,74 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using osu.Framework.Localisation; + +namespace osu.Game.Localisation +{ + public static class SkinEditorStrings + { + private const string prefix = "osu.Game.Localisation.SkinEditor"; + + /// + /// "anchor" + /// + public static LocalisableString Anchor => new TranslatableString(getKey("anchor"), "anchor"); + + /// + /// "origin" + /// + public static LocalisableString Origin => new TranslatableString(getKey("origin"), "origin"); + + /// + /// "top-left" + /// + public static LocalisableString TopLeft => new TranslatableString(getKey("top_left"), "top-left"); + + /// + /// "top-centre" + /// + public static LocalisableString TopCentre => new TranslatableString(getKey("top_centre"), "top-centre"); + + /// + /// "top-right" + /// + public static LocalisableString TopRight => new TranslatableString(getKey("top_right"), "top-right"); + + /// + /// "centre-left" + /// + public static LocalisableString CentreLeft => new TranslatableString(getKey("centre_left"), "centre-left"); + + /// + /// "centre" + /// + public static LocalisableString Centre => new TranslatableString(getKey("centre"), "centre"); + + /// + /// "centre-right" + /// + public static LocalisableString CentreRight => new TranslatableString(getKey("centre_right"), "centre-right"); + + /// + /// "bottom-left" + /// + public static LocalisableString BottomLeft => new TranslatableString(getKey("bottom_left"), "bottom-left"); + + /// + /// "bottom-centre" + /// + public static LocalisableString BottomCentre => new TranslatableString(getKey("bottom_centre"), "bottom-centre"); + + /// + /// "bottom-right" + /// + public static LocalisableString BottomRight => new TranslatableString(getKey("bottom_right"), "bottom-right"); + + /// + /// "closest" + /// + public static LocalisableString Closest => new TranslatableString(getKey("closest"), "closest"); + + private static string getKey(string key) => $"{prefix}:{key}"; + } +} diff --git a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs index 9cca0ba2c7..956f6c79f9 100644 --- a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs +++ b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs @@ -4,22 +4,152 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Runtime.CompilerServices; using osu.Framework.Allocation; +using osu.Framework.Bindables; using osu.Framework.Extensions.EnumExtensions; using osu.Framework.Graphics; using osu.Framework.Graphics.Primitives; using osu.Framework.Graphics.UserInterface; +using osu.Framework.Localisation; using osu.Framework.Utils; using osu.Game.Extensions; using osu.Game.Graphics.UserInterface; using osu.Game.Rulesets.Edit; using osu.Game.Screens.Edit.Compose.Components; using osuTK; +using Humanizer; +using osu.Game.Localisation; namespace osu.Game.Skinning.Editor { public class SkinSelectionHandler : SelectionHandler { + /// + ///

Keeps track of whether a is using the closest anchor point within its parent, + /// or whether the user is overriding its anchor point.

+ ///

Each key is either a direct cast of an Anchor value, or it is equal to . This is done + /// because the "Closest" menu item is not a valid anchor, so something other than an anchor must be used.

+ ///

Each value is a . If the is , the user has + /// overridden the anchor point. + /// If , the closest anchor point is assigned to the Drawable when it is either dragged by the user via , or when "Closest" is assigned from + /// the anchor context menu via .

+ ///
+ /// + ///

A ConditionalWeakTable is preferable to a Dictionary because a Dictionary will keep + /// orphaned references to a Drawable forever, unless manually pruned

+ ///

is used as a thin wrapper around bool because ConditionalWeakTable requires a reference type as both a key and a value.

+ ///
+ private readonly ConditionalWeakTable isDrawableUsingClosestAnchorLookup = new ConditionalWeakTable(); + + /// + /// The hash code of the "Closest" menu item in the anchor point context menu. + /// + /// This does not need to change with locale; it need only be constant and distinct from any values. + private static readonly int hash_of_closest_anchor_menu_item = @"Closest".GetHashCode(); + + /// Used by to populate and . + private static readonly LocalisableString[] unbound_anchor_menu_items = + { + SkinEditorStrings.Closest, + SkinEditorStrings.TopLeft, + SkinEditorStrings.TopCentre, + SkinEditorStrings.TopRight, + SkinEditorStrings.CentreLeft, + SkinEditorStrings.Centre, + SkinEditorStrings.CentreRight, + SkinEditorStrings.BottomLeft, + SkinEditorStrings.BottomCentre, + SkinEditorStrings.BottomRight, + }; + + /// Used by to populate and . + private static readonly int[] anchor_menu_hashes = + new[] + { + Anchor.TopLeft, + Anchor.TopCentre, + Anchor.TopRight, + Anchor.CentreLeft, + Anchor.Centre, + Anchor.CentreRight, + Anchor.BottomLeft, + Anchor.BottomCentre, + Anchor.BottomRight, + } + .Cast() + .Prepend(hash_of_closest_anchor_menu_item) + .ToArray(); + + private Dictionary localisedAnchorMenuItems; + private Dictionary localisedOriginMenuItems; + private ILocalisedBindableString localisedAnchor; + private ILocalisedBindableString localisedOrigin; + + [BackgroundDependencyLoader] + private void load(LocalisationManager localisation) + { + localisedAnchor = localisation.GetLocalisedString(SkinEditorStrings.Anchor); + localisedOrigin = localisation.GetLocalisedString(SkinEditorStrings.Origin); + + var boundAnchorMenuItems = unbound_anchor_menu_items.Select(localisation.GetLocalisedString).ToArray(); + + var anchorPairs = anchor_menu_hashes.Zip(boundAnchorMenuItems, (k, v) => new KeyValuePair(k, v)).ToArray(); + localisedAnchorMenuItems = new Dictionary(anchorPairs); + var originPairs = anchorPairs.Where(pair => pair.Key != hash_of_closest_anchor_menu_item); + localisedOriginMenuItems = new Dictionary(originPairs); + } + + private Anchor getClosestAnchorForDrawable(Drawable drawable) + { + var parent = drawable.Parent; + if (parent == null) + return drawable.Anchor; + + // If there is a better way to get this information, let me know. Was taken from LogoTrackingContainer.ComputeLogoTrackingPosition + // I tried a lot of different things, such as just using Position / ChildSize, but none of them worked properly. + var screenPosition = getOriginPositionFromQuad(drawable.ScreenSpaceDrawQuad, drawable.Origin); + var absolutePosition = parent.ToLocalSpace(screenPosition); + var factor = parent.RelativeToAbsoluteFactor; + + var result = default(Anchor); + + static Anchor getTieredComponent(float component, Anchor tier0, Anchor tier1, Anchor tier2) => + component >= 2 / 3f + ? tier2 + : component >= 1 / 3f + ? tier1 + : tier0; + + result |= getTieredComponent(absolutePosition.X / factor.X, Anchor.x0, Anchor.x1, Anchor.x2); + result |= getTieredComponent(absolutePosition.Y / factor.Y, Anchor.y0, Anchor.y1, Anchor.y2); + + return result; + } + + private Vector2 getOriginPositionFromQuad(in Quad quad, Anchor origin) + { + var result = quad.TopLeft; + + if (origin.HasFlagFast(Anchor.x2)) + result.X += quad.Width; + else if (origin.HasFlagFast(Anchor.x1)) + result.X += quad.Width / 2f; + + if (origin.HasFlagFast(Anchor.y2)) + result.Y += quad.Height; + else if (origin.HasFlagFast(Anchor.y1)) + result.Y += quad.Height / 2f; + + return result; + } + + /// Defaults to , meaning anchors are closest by default. + private BindableBool isDrawableUsingClosestAnchor(Drawable drawable) => isDrawableUsingClosestAnchorLookup.GetValue(drawable, _ => new BindableBool(true)); + + // There may be a more generalised form of this somewhere in the codebase. If so, use that. + private static string getSentenceCaseLocalisedString(ILocalisedBindableString ls) => ls.Value.Transform(To.SentenceCase); + [Resolved] private SkinEditor skinEditor { get; set; } @@ -151,11 +281,24 @@ namespace osu.Game.Skinning.Editor { Drawable drawable = (Drawable)c.Item; drawable.Position += drawable.ScreenSpaceDeltaToParentSpace(moveEvent.ScreenSpaceDelta); + + updateDrawableAnchorIfUsingClosest(drawable); } return true; } + private void updateDrawableAnchorIfUsingClosest(Drawable drawable) + { + if (!isDrawableUsingClosestAnchor(drawable).Value) return; + + var closestAnchor = getClosestAnchorForDrawable(drawable); + + if (closestAnchor == drawable.Anchor) return; + + updateDrawableAnchor(drawable, closestAnchor); + } + protected override void OnSelectionChanged() { base.OnSelectionChanged(); @@ -171,42 +314,35 @@ namespace osu.Game.Skinning.Editor protected override IEnumerable GetContextMenuItemsForSelection(IEnumerable> selection) { - yield return new OsuMenuItem("Anchor") + int checkAnchor(Drawable drawable) => + isDrawableUsingClosestAnchor(drawable).Value + ? hash_of_closest_anchor_menu_item + : (int)drawable.Anchor; + + yield return new OsuMenuItem(getSentenceCaseLocalisedString(localisedAnchor)) { - Items = createAnchorItems(d => d.Anchor, applyAnchor).ToArray() + Items = createAnchorItems(localisedAnchorMenuItems, checkAnchor, applyAnchor).ToArray() }; - yield return new OsuMenuItem("Origin") + yield return new OsuMenuItem(getSentenceCaseLocalisedString(localisedOrigin)) { - Items = createAnchorItems(d => d.Origin, applyOrigin).ToArray() + // Origins can't be "closest" so we just cast to int + Items = createAnchorItems(localisedOriginMenuItems, d => (int)d.Origin, applyOrigin).ToArray() }; foreach (var item in base.GetContextMenuItemsForSelection(selection)) yield return item; - IEnumerable createAnchorItems(Func checkFunction, Action applyFunction) - { - var displayableAnchors = new[] + IEnumerable createAnchorItems(IDictionary items, Func checkFunction, Action applyFunction) => + items.Select(pair => { - Anchor.TopLeft, - Anchor.TopCentre, - Anchor.TopRight, - Anchor.CentreLeft, - Anchor.Centre, - Anchor.CentreRight, - Anchor.BottomLeft, - Anchor.BottomCentre, - Anchor.BottomRight, - }; + var (hash, ls) = pair; - return displayableAnchors.Select(a => - { - return new TernaryStateRadioMenuItem(a.ToString(), MenuItemType.Standard, _ => applyFunction(a)) + return new TernaryStateRadioMenuItem(getSentenceCaseLocalisedString(ls), MenuItemType.Standard, _ => applyFunction(hash)) { - State = { Value = GetStateFromSelection(selection, c => checkFunction((Drawable)c.Item) == a) } + State = { Value = GetStateFromSelection(selection, c => checkFunction((Drawable)c.Item) == hash) } }; }); - } } private static void updateDrawablePosition(Drawable drawable, Vector2 screenSpacePosition) @@ -215,8 +351,10 @@ namespace osu.Game.Skinning.Editor drawable.Parent.ToLocalSpace(screenSpacePosition) - drawable.AnchorPosition; } - private void applyOrigin(Anchor anchor) + private void applyOrigin(int hash) { + var anchor = (Anchor)hash; + foreach (var item in SelectedItems) { var drawable = (Drawable)item; @@ -224,6 +362,8 @@ namespace osu.Game.Skinning.Editor var previousOrigin = drawable.OriginPosition; drawable.Origin = anchor; drawable.Position += drawable.OriginPosition - previousOrigin; + + updateDrawableAnchorIfUsingClosest(drawable); } } @@ -234,18 +374,39 @@ namespace osu.Game.Skinning.Editor private Quad getSelectionQuad() => GetSurroundingQuad(SelectedBlueprints.SelectMany(b => b.Item.ScreenSpaceDrawQuad.GetVertices().ToArray())); - private void applyAnchor(Anchor anchor) + private void applyAnchor(int hash) { foreach (var item in SelectedItems) { var drawable = (Drawable)item; - var previousAnchor = drawable.AnchorPosition; - drawable.Anchor = anchor; - drawable.Position -= drawable.AnchorPosition - previousAnchor; + var anchor = getAnchorFromHashAndDrawableAndRecordWhetherUsingClosestAnchor(hash, drawable); + + updateDrawableAnchor(drawable, anchor); } } + private static void updateDrawableAnchor(Drawable drawable, Anchor anchor) + { + var previousAnchor = drawable.AnchorPosition; + drawable.Anchor = anchor; + drawable.Position -= drawable.AnchorPosition - previousAnchor; + } + + private Anchor getAnchorFromHashAndDrawableAndRecordWhetherUsingClosestAnchor(int hash, Drawable drawable) + { + var isUsingClosestAnchor = isDrawableUsingClosestAnchor(drawable); + + if (hash == hash_of_closest_anchor_menu_item) + { + isUsingClosestAnchor.Value = true; + return getClosestAnchorForDrawable(drawable); + } + + isUsingClosestAnchor.Value = false; + return (Anchor)hash; + } + private static void adjustScaleFromAnchor(ref Vector2 scale, Anchor reference) { // cancel out scale in axes we don't care about (based on which drag handle was used). From 4aee76456f8cdb8bade6d037d1a9bb02af125ee8 Mon Sep 17 00:00:00 2001 From: Robin Avery Date: Sun, 6 Jun 2021 05:34:32 -0400 Subject: [PATCH 02/39] Replace localised strings with static English --- osu.Game/Localisation/SkinEditorStrings.cs | 74 ------------- .../Skinning/Editor/SkinSelectionHandler.cs | 101 +++++++----------- 2 files changed, 36 insertions(+), 139 deletions(-) delete mode 100644 osu.Game/Localisation/SkinEditorStrings.cs diff --git a/osu.Game/Localisation/SkinEditorStrings.cs b/osu.Game/Localisation/SkinEditorStrings.cs deleted file mode 100644 index 24a077963f..0000000000 --- a/osu.Game/Localisation/SkinEditorStrings.cs +++ /dev/null @@ -1,74 +0,0 @@ -// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. -// See the LICENCE file in the repository root for full licence text. - -using osu.Framework.Localisation; - -namespace osu.Game.Localisation -{ - public static class SkinEditorStrings - { - private const string prefix = "osu.Game.Localisation.SkinEditor"; - - /// - /// "anchor" - /// - public static LocalisableString Anchor => new TranslatableString(getKey("anchor"), "anchor"); - - /// - /// "origin" - /// - public static LocalisableString Origin => new TranslatableString(getKey("origin"), "origin"); - - /// - /// "top-left" - /// - public static LocalisableString TopLeft => new TranslatableString(getKey("top_left"), "top-left"); - - /// - /// "top-centre" - /// - public static LocalisableString TopCentre => new TranslatableString(getKey("top_centre"), "top-centre"); - - /// - /// "top-right" - /// - public static LocalisableString TopRight => new TranslatableString(getKey("top_right"), "top-right"); - - /// - /// "centre-left" - /// - public static LocalisableString CentreLeft => new TranslatableString(getKey("centre_left"), "centre-left"); - - /// - /// "centre" - /// - public static LocalisableString Centre => new TranslatableString(getKey("centre"), "centre"); - - /// - /// "centre-right" - /// - public static LocalisableString CentreRight => new TranslatableString(getKey("centre_right"), "centre-right"); - - /// - /// "bottom-left" - /// - public static LocalisableString BottomLeft => new TranslatableString(getKey("bottom_left"), "bottom-left"); - - /// - /// "bottom-centre" - /// - public static LocalisableString BottomCentre => new TranslatableString(getKey("bottom_centre"), "bottom-centre"); - - /// - /// "bottom-right" - /// - public static LocalisableString BottomRight => new TranslatableString(getKey("bottom_right"), "bottom-right"); - - /// - /// "closest" - /// - public static LocalisableString Closest => new TranslatableString(getKey("closest"), "closest"); - - private static string getKey(string key) => $"{prefix}:{key}"; - } -} diff --git a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs index 956f6c79f9..fbe7cc6d91 100644 --- a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs +++ b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs @@ -11,15 +11,12 @@ using osu.Framework.Extensions.EnumExtensions; using osu.Framework.Graphics; using osu.Framework.Graphics.Primitives; using osu.Framework.Graphics.UserInterface; -using osu.Framework.Localisation; using osu.Framework.Utils; using osu.Game.Extensions; using osu.Game.Graphics.UserInterface; using osu.Game.Rulesets.Edit; using osu.Game.Screens.Edit.Compose.Components; using osuTK; -using Humanizer; -using osu.Game.Localisation; namespace osu.Game.Skinning.Editor { @@ -28,7 +25,7 @@ namespace osu.Game.Skinning.Editor /// ///

Keeps track of whether a is using the closest anchor point within its parent, /// or whether the user is overriding its anchor point.

- ///

Each key is either a direct cast of an Anchor value, or it is equal to . This is done + ///

Each key is either a direct cast of an Anchor value, or it is equal to . This is done /// because the "Closest" menu item is not a valid anchor, so something other than an anchor must be used.

///

Each value is a . If the is , the user has /// overridden the anchor point. @@ -42,62 +39,39 @@ namespace osu.Game.Skinning.Editor /// private readonly ConditionalWeakTable isDrawableUsingClosestAnchorLookup = new ConditionalWeakTable(); + private const string closest_text = @"Closest"; + ///

/// The hash code of the "Closest" menu item in the anchor point context menu. /// - /// This does not need to change with locale; it need only be constant and distinct from any values. - private static readonly int hash_of_closest_anchor_menu_item = @"Closest".GetHashCode(); + /// Needs only be constant and distinct from any values. + private static readonly int closest_text_hash = closest_text.GetHashCode(); - /// Used by to populate and . - private static readonly LocalisableString[] unbound_anchor_menu_items = + private static readonly Dictionary anchor_menu_items; + private static readonly Dictionary origin_menu_items; + + static SkinSelectionHandler() { - SkinEditorStrings.Closest, - SkinEditorStrings.TopLeft, - SkinEditorStrings.TopCentre, - SkinEditorStrings.TopRight, - SkinEditorStrings.CentreLeft, - SkinEditorStrings.Centre, - SkinEditorStrings.CentreRight, - SkinEditorStrings.BottomLeft, - SkinEditorStrings.BottomCentre, - SkinEditorStrings.BottomRight, - }; + var anchorMenuSubset = new[] + { + Anchor.TopLeft, + Anchor.TopCentre, + Anchor.TopRight, + Anchor.CentreLeft, + Anchor.Centre, + Anchor.CentreRight, + Anchor.BottomLeft, + Anchor.BottomCentre, + Anchor.BottomRight, + }; - /// Used by to populate and . - private static readonly int[] anchor_menu_hashes = - new[] - { - Anchor.TopLeft, - Anchor.TopCentre, - Anchor.TopRight, - Anchor.CentreLeft, - Anchor.Centre, - Anchor.CentreRight, - Anchor.BottomLeft, - Anchor.BottomCentre, - Anchor.BottomRight, - } - .Cast() - .Prepend(hash_of_closest_anchor_menu_item) - .ToArray(); + var texts = anchorMenuSubset.Select(a => a.ToString()).Prepend(closest_text).ToArray(); + var hashes = anchorMenuSubset.Cast().Prepend(closest_text_hash).ToArray(); - private Dictionary localisedAnchorMenuItems; - private Dictionary localisedOriginMenuItems; - private ILocalisedBindableString localisedAnchor; - private ILocalisedBindableString localisedOrigin; - - [BackgroundDependencyLoader] - private void load(LocalisationManager localisation) - { - localisedAnchor = localisation.GetLocalisedString(SkinEditorStrings.Anchor); - localisedOrigin = localisation.GetLocalisedString(SkinEditorStrings.Origin); - - var boundAnchorMenuItems = unbound_anchor_menu_items.Select(localisation.GetLocalisedString).ToArray(); - - var anchorPairs = anchor_menu_hashes.Zip(boundAnchorMenuItems, (k, v) => new KeyValuePair(k, v)).ToArray(); - localisedAnchorMenuItems = new Dictionary(anchorPairs); - var originPairs = anchorPairs.Where(pair => pair.Key != hash_of_closest_anchor_menu_item); - localisedOriginMenuItems = new Dictionary(originPairs); + var anchorPairs = hashes.Zip(texts, (k, v) => new KeyValuePair(k, v)).ToArray(); + anchor_menu_items = new Dictionary(anchorPairs); + var originPairs = anchorPairs.Where(pair => pair.Key != closest_text_hash); + origin_menu_items = new Dictionary(originPairs); } private Anchor getClosestAnchorForDrawable(Drawable drawable) @@ -147,9 +121,6 @@ namespace osu.Game.Skinning.Editor /// Defaults to , meaning anchors are closest by default. private BindableBool isDrawableUsingClosestAnchor(Drawable drawable) => isDrawableUsingClosestAnchorLookup.GetValue(drawable, _ => new BindableBool(true)); - // There may be a more generalised form of this somewhere in the codebase. If so, use that. - private static string getSentenceCaseLocalisedString(ILocalisedBindableString ls) => ls.Value.Transform(To.SentenceCase); - [Resolved] private SkinEditor skinEditor { get; set; } @@ -316,29 +287,29 @@ namespace osu.Game.Skinning.Editor { int checkAnchor(Drawable drawable) => isDrawableUsingClosestAnchor(drawable).Value - ? hash_of_closest_anchor_menu_item + ? closest_text_hash : (int)drawable.Anchor; - yield return new OsuMenuItem(getSentenceCaseLocalisedString(localisedAnchor)) + yield return new OsuMenuItem(nameof(Anchor)) { - Items = createAnchorItems(localisedAnchorMenuItems, checkAnchor, applyAnchor).ToArray() + Items = createAnchorItems(anchor_menu_items, checkAnchor, applyAnchor).ToArray() }; - yield return new OsuMenuItem(getSentenceCaseLocalisedString(localisedOrigin)) + yield return new OsuMenuItem(nameof(Origin)) { // Origins can't be "closest" so we just cast to int - Items = createAnchorItems(localisedOriginMenuItems, d => (int)d.Origin, applyOrigin).ToArray() + Items = createAnchorItems(origin_menu_items, d => (int)d.Origin, applyOrigin).ToArray() }; foreach (var item in base.GetContextMenuItemsForSelection(selection)) yield return item; - IEnumerable createAnchorItems(IDictionary items, Func checkFunction, Action applyFunction) => + IEnumerable createAnchorItems(IDictionary items, Func checkFunction, Action applyFunction) => items.Select(pair => { - var (hash, ls) = pair; + var (hash, text) = pair; - return new TernaryStateRadioMenuItem(getSentenceCaseLocalisedString(ls), MenuItemType.Standard, _ => applyFunction(hash)) + return new TernaryStateRadioMenuItem(text, MenuItemType.Standard, _ => applyFunction(hash)) { State = { Value = GetStateFromSelection(selection, c => checkFunction((Drawable)c.Item) == hash) } }; @@ -397,7 +368,7 @@ namespace osu.Game.Skinning.Editor { var isUsingClosestAnchor = isDrawableUsingClosestAnchor(drawable); - if (hash == hash_of_closest_anchor_menu_item) + if (hash == closest_text_hash) { isUsingClosestAnchor.Value = true; return getClosestAnchorForDrawable(drawable); From c9f5808bf2cba4966f9fb5a5e1347a9a31e13bf5 Mon Sep 17 00:00:00 2001 From: Robin Avery Date: Sun, 6 Jun 2021 06:58:21 -0400 Subject: [PATCH 03/39] Move lookup logic to DrawableExtensions This is now a global lookup to be shared by serialization and editor. --- osu.Game/Extensions/DrawableExtensions.cs | 25 +++++++++++++++++ .../Skinning/Editor/SkinSelectionHandler.cs | 28 ++----------------- 2 files changed, 28 insertions(+), 25 deletions(-) diff --git a/osu.Game/Extensions/DrawableExtensions.cs b/osu.Game/Extensions/DrawableExtensions.cs index 2ac6e6ff22..febad0d739 100644 --- a/osu.Game/Extensions/DrawableExtensions.cs +++ b/osu.Game/Extensions/DrawableExtensions.cs @@ -2,11 +2,15 @@ // See the LICENCE file in the repository root for full licence text. using System; +using System.Collections.Generic; +using System.Runtime.CompilerServices; +using osu.Framework.Bindables; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Input.Bindings; using osu.Framework.Threading; using osu.Game.Screens.Play.HUD; +using osu.Game.Skinning; using osuTK; namespace osu.Game.Extensions @@ -63,5 +67,26 @@ namespace osu.Game.Extensions container.Add(child.CreateInstance()); } } + + /// + ///

A ConditionalWeakTable is preferable to a Dictionary because a Dictionary will keep + /// orphaned references to an forever, unless manually pruned.

+ ///

is used as a thin wrapper around bool because ConditionalWeakTable requires a reference type as both a key and a value.

+ ///

was chosen over because it is a common ancestor between (which is required for logic) + /// and (which is required for serialization via ).

+ ///

This collection is thread-safe according to the + /// documentation, + /// but the BindableBools are not unless leased.

+ ///
+ private static readonly ConditionalWeakTable is_drawable_using_closest_anchor_lookup = new ConditionalWeakTable(); + + /// + /// Gets or creates a representing whether is using the closest anchor point within its + /// parent. + /// + /// A whose is if the is using the closest anchor point, + /// otherwise . + public static BindableBool IsUsingClosestAnchor(this IDrawable drawable) => + is_drawable_using_closest_anchor_lookup.GetValue(drawable, _ => new BindableBool(true)); } } diff --git a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs index fbe7cc6d91..5619a7ba19 100644 --- a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs +++ b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs @@ -4,9 +4,7 @@ using System; using System.Collections.Generic; using System.Linq; -using System.Runtime.CompilerServices; using osu.Framework.Allocation; -using osu.Framework.Bindables; using osu.Framework.Extensions.EnumExtensions; using osu.Framework.Graphics; using osu.Framework.Graphics.Primitives; @@ -22,23 +20,6 @@ namespace osu.Game.Skinning.Editor { public class SkinSelectionHandler : SelectionHandler { - /// - ///

Keeps track of whether a is using the closest anchor point within its parent, - /// or whether the user is overriding its anchor point.

- ///

Each key is either a direct cast of an Anchor value, or it is equal to . This is done - /// because the "Closest" menu item is not a valid anchor, so something other than an anchor must be used.

- ///

Each value is a . If the is , the user has - /// overridden the anchor point. - /// If , the closest anchor point is assigned to the Drawable when it is either dragged by the user via , or when "Closest" is assigned from - /// the anchor context menu via .

- ///
- /// - ///

A ConditionalWeakTable is preferable to a Dictionary because a Dictionary will keep - /// orphaned references to a Drawable forever, unless manually pruned

- ///

is used as a thin wrapper around bool because ConditionalWeakTable requires a reference type as both a key and a value.

- ///
- private readonly ConditionalWeakTable isDrawableUsingClosestAnchorLookup = new ConditionalWeakTable(); - private const string closest_text = @"Closest"; /// @@ -118,9 +99,6 @@ namespace osu.Game.Skinning.Editor return result; } - /// Defaults to , meaning anchors are closest by default. - private BindableBool isDrawableUsingClosestAnchor(Drawable drawable) => isDrawableUsingClosestAnchorLookup.GetValue(drawable, _ => new BindableBool(true)); - [Resolved] private SkinEditor skinEditor { get; set; } @@ -261,7 +239,7 @@ namespace osu.Game.Skinning.Editor private void updateDrawableAnchorIfUsingClosest(Drawable drawable) { - if (!isDrawableUsingClosestAnchor(drawable).Value) return; + if (!drawable.IsUsingClosestAnchor().Value) return; var closestAnchor = getClosestAnchorForDrawable(drawable); @@ -286,7 +264,7 @@ namespace osu.Game.Skinning.Editor protected override IEnumerable GetContextMenuItemsForSelection(IEnumerable> selection) { int checkAnchor(Drawable drawable) => - isDrawableUsingClosestAnchor(drawable).Value + drawable.IsUsingClosestAnchor().Value ? closest_text_hash : (int)drawable.Anchor; @@ -366,7 +344,7 @@ namespace osu.Game.Skinning.Editor private Anchor getAnchorFromHashAndDrawableAndRecordWhetherUsingClosestAnchor(int hash, Drawable drawable) { - var isUsingClosestAnchor = isDrawableUsingClosestAnchor(drawable); + var isUsingClosestAnchor = drawable.IsUsingClosestAnchor(); if (hash == closest_text_hash) { From 11b1b8c633a3f1f04decb76c5ea49050ad8931ee Mon Sep 17 00:00:00 2001 From: Robin Avery Date: Sun, 6 Jun 2021 07:18:08 -0400 Subject: [PATCH 04/39] Add serialization support via SkinnableInfo --- osu.Game/Extensions/DrawableExtensions.cs | 1 + osu.Game/Screens/Play/HUD/SkinnableInfo.cs | 8 ++++++++ 2 files changed, 9 insertions(+) diff --git a/osu.Game/Extensions/DrawableExtensions.cs b/osu.Game/Extensions/DrawableExtensions.cs index febad0d739..d8c627ea8e 100644 --- a/osu.Game/Extensions/DrawableExtensions.cs +++ b/osu.Game/Extensions/DrawableExtensions.cs @@ -60,6 +60,7 @@ namespace osu.Game.Extensions component.Scale = info.Scale; component.Anchor = info.Anchor; component.Origin = info.Origin; + component.IsUsingClosestAnchor().Value = !info.IsNotUsingClosestAnchor; if (component is Container container) { diff --git a/osu.Game/Screens/Play/HUD/SkinnableInfo.cs b/osu.Game/Screens/Play/HUD/SkinnableInfo.cs index e08044b14c..3e829f6d38 100644 --- a/osu.Game/Screens/Play/HUD/SkinnableInfo.cs +++ b/osu.Game/Screens/Play/HUD/SkinnableInfo.cs @@ -32,6 +32,13 @@ namespace osu.Game.Screens.Play.HUD public Anchor Origin { get; set; } + /// + /// if this 's is + /// automatically determined by proximity, if the user has overridden it. + /// + /// Stored this way because default(bool) is and we want the default behaviour to be "closest". + public bool IsNotUsingClosestAnchor { get; set; } + public List Children { get; } = new List(); [JsonConstructor] @@ -52,6 +59,7 @@ namespace osu.Game.Screens.Play.HUD Scale = component.Scale; Anchor = component.Anchor; Origin = component.Origin; + IsNotUsingClosestAnchor = !component.IsUsingClosestAnchor().Value; if (component is Container container) { From 63346f6b756521bdac240777bcb26716cb2106ae Mon Sep 17 00:00:00 2001 From: Robin Avery Date: Sun, 6 Jun 2021 09:40:58 -0400 Subject: [PATCH 05/39] Refactor getTieredComponent --- .../Skinning/Editor/SkinSelectionHandler.cs | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs index 5619a7ba19..b1d353b0af 100644 --- a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs +++ b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs @@ -69,19 +69,23 @@ namespace osu.Game.Skinning.Editor var result = default(Anchor); - static Anchor getTieredComponent(float component, Anchor tier0, Anchor tier1, Anchor tier2) => - component >= 2 / 3f - ? tier2 - : component >= 1 / 3f - ? tier1 - : tier0; - result |= getTieredComponent(absolutePosition.X / factor.X, Anchor.x0, Anchor.x1, Anchor.x2); result |= getTieredComponent(absolutePosition.Y / factor.Y, Anchor.y0, Anchor.y1, Anchor.y2); return result; } + private static Anchor getTieredComponent(float component, Anchor tier0, Anchor tier1, Anchor tier2) + { + if (component >= 2 / 3f) + return tier2; + + if (component >= 1 / 3f) + return tier1; + + return tier0; + } + private Vector2 getOriginPositionFromQuad(in Quad quad, Anchor origin) { var result = quad.TopLeft; From da1c38d5a907ae2e4122b4d133f38a60949d0435 Mon Sep 17 00:00:00 2001 From: Robin Avery Date: Sun, 6 Jun 2021 10:10:55 -0400 Subject: [PATCH 06/39] Uninvert logic of SkinnableInfo.UsingClosestAnchor Also rename "IsUsingClosestAnchor" to simply "UsingClosestAnchor". --- osu.Game/Extensions/DrawableExtensions.cs | 4 ++-- osu.Game/Screens/Play/HUD/SkinnableInfo.cs | 10 +++++----- osu.Game/Skinning/Editor/SkinSelectionHandler.cs | 6 +++--- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/osu.Game/Extensions/DrawableExtensions.cs b/osu.Game/Extensions/DrawableExtensions.cs index d8c627ea8e..f66fe2b8b2 100644 --- a/osu.Game/Extensions/DrawableExtensions.cs +++ b/osu.Game/Extensions/DrawableExtensions.cs @@ -60,7 +60,7 @@ namespace osu.Game.Extensions component.Scale = info.Scale; component.Anchor = info.Anchor; component.Origin = info.Origin; - component.IsUsingClosestAnchor().Value = !info.IsNotUsingClosestAnchor; + component.UsingClosestAnchor().Value = info.UsingClosestAnchor; if (component is Container container) { @@ -87,7 +87,7 @@ namespace osu.Game.Extensions /// /// A whose is if the is using the closest anchor point, /// otherwise . - public static BindableBool IsUsingClosestAnchor(this IDrawable drawable) => + public static BindableBool UsingClosestAnchor(this IDrawable drawable) => is_drawable_using_closest_anchor_lookup.GetValue(drawable, _ => new BindableBool(true)); } } diff --git a/osu.Game/Screens/Play/HUD/SkinnableInfo.cs b/osu.Game/Screens/Play/HUD/SkinnableInfo.cs index 3e829f6d38..8ca5f1ba2d 100644 --- a/osu.Game/Screens/Play/HUD/SkinnableInfo.cs +++ b/osu.Game/Screens/Play/HUD/SkinnableInfo.cs @@ -33,11 +33,11 @@ namespace osu.Game.Screens.Play.HUD public Anchor Origin { get; set; } /// - /// if this 's is - /// automatically determined by proximity, if the user has overridden it. + ///

if this 's is + /// automatically determined by proximity, if the user has overridden it.

+ ///

Corresponds to at runtime.

///
- /// Stored this way because default(bool) is and we want the default behaviour to be "closest". - public bool IsNotUsingClosestAnchor { get; set; } + public bool UsingClosestAnchor { get; set; } = true; public List Children { get; } = new List(); @@ -59,7 +59,7 @@ namespace osu.Game.Screens.Play.HUD Scale = component.Scale; Anchor = component.Anchor; Origin = component.Origin; - IsNotUsingClosestAnchor = !component.IsUsingClosestAnchor().Value; + UsingClosestAnchor = component.UsingClosestAnchor().Value; if (component is Container container) { diff --git a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs index b1d353b0af..f8d41d94fa 100644 --- a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs +++ b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs @@ -243,7 +243,7 @@ namespace osu.Game.Skinning.Editor private void updateDrawableAnchorIfUsingClosest(Drawable drawable) { - if (!drawable.IsUsingClosestAnchor().Value) return; + if (!drawable.UsingClosestAnchor().Value) return; var closestAnchor = getClosestAnchorForDrawable(drawable); @@ -268,7 +268,7 @@ namespace osu.Game.Skinning.Editor protected override IEnumerable GetContextMenuItemsForSelection(IEnumerable> selection) { int checkAnchor(Drawable drawable) => - drawable.IsUsingClosestAnchor().Value + drawable.UsingClosestAnchor().Value ? closest_text_hash : (int)drawable.Anchor; @@ -348,7 +348,7 @@ namespace osu.Game.Skinning.Editor private Anchor getAnchorFromHashAndDrawableAndRecordWhetherUsingClosestAnchor(int hash, Drawable drawable) { - var isUsingClosestAnchor = drawable.IsUsingClosestAnchor(); + var isUsingClosestAnchor = drawable.UsingClosestAnchor(); if (hash == closest_text_hash) { From 888882ac63234f5372675218ea8449385f06bc33 Mon Sep 17 00:00:00 2001 From: Robin Avery Date: Sun, 6 Jun 2021 13:27:13 -0400 Subject: [PATCH 07/39] Remove first-person comment --- osu.Game/Skinning/Editor/SkinSelectionHandler.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs index f8d41d94fa..e07d4e351b 100644 --- a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs +++ b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs @@ -61,8 +61,6 @@ namespace osu.Game.Skinning.Editor if (parent == null) return drawable.Anchor; - // If there is a better way to get this information, let me know. Was taken from LogoTrackingContainer.ComputeLogoTrackingPosition - // I tried a lot of different things, such as just using Position / ChildSize, but none of them worked properly. var screenPosition = getOriginPositionFromQuad(drawable.ScreenSpaceDrawQuad, drawable.Origin); var absolutePosition = parent.ToLocalSpace(screenPosition); var factor = parent.RelativeToAbsoluteFactor; From 6a456e53f40a6a5f1b00d41957b0ba7b56907828 Mon Sep 17 00:00:00 2001 From: Robin Avery Date: Sun, 6 Jun 2021 13:28:17 -0400 Subject: [PATCH 08/39] Rename overly long method --- osu.Game/Skinning/Editor/SkinSelectionHandler.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs index e07d4e351b..5acc949182 100644 --- a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs +++ b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs @@ -331,7 +331,7 @@ namespace osu.Game.Skinning.Editor { var drawable = (Drawable)item; - var anchor = getAnchorFromHashAndDrawableAndRecordWhetherUsingClosestAnchor(hash, drawable); + var anchor = mapHashToAnchorSettings(hash, drawable); updateDrawableAnchor(drawable, anchor); } @@ -344,7 +344,7 @@ namespace osu.Game.Skinning.Editor drawable.Position -= drawable.AnchorPosition - previousAnchor; } - private Anchor getAnchorFromHashAndDrawableAndRecordWhetherUsingClosestAnchor(int hash, Drawable drawable) + private Anchor mapHashToAnchorSettings(int hash, Drawable drawable) { var isUsingClosestAnchor = drawable.UsingClosestAnchor(); From ce635af83edf43645cba2c6c6fcbe723faebe98a Mon Sep 17 00:00:00 2001 From: Robin Avery Date: Sun, 6 Jun 2021 23:47:47 -0400 Subject: [PATCH 09/39] Add UsingClosestAnchor to ISkinnableDrawable Also implement it as an auto property in its inheritors. The auto properties default to true. --- osu.Game/Screens/Play/HUD/DefaultAccuracyCounter.cs | 2 ++ osu.Game/Screens/Play/HUD/DefaultComboCounter.cs | 2 ++ osu.Game/Screens/Play/HUD/DefaultHealthDisplay.cs | 2 ++ osu.Game/Screens/Play/HUD/DefaultScoreCounter.cs | 2 ++ osu.Game/Screens/Play/HUD/HitErrorMeters/HitErrorMeter.cs | 2 ++ osu.Game/Screens/Play/HUD/LegacyComboCounter.cs | 2 ++ osu.Game/Screens/Play/HUD/SkinnableInfo.cs | 6 +----- osu.Game/Screens/Play/SongProgress.cs | 2 ++ osu.Game/Skinning/ISkinnableDrawable.cs | 6 ++++++ osu.Game/Skinning/LegacyAccuracyCounter.cs | 2 ++ osu.Game/Skinning/LegacyHealthDisplay.cs | 2 ++ osu.Game/Skinning/LegacyScoreCounter.cs | 2 ++ osu.Game/Skinning/SkinnableTargetComponentsContainer.cs | 2 ++ 13 files changed, 29 insertions(+), 5 deletions(-) diff --git a/osu.Game/Screens/Play/HUD/DefaultAccuracyCounter.cs b/osu.Game/Screens/Play/HUD/DefaultAccuracyCounter.cs index 45ba05e036..5c6bb7299a 100644 --- a/osu.Game/Screens/Play/HUD/DefaultAccuracyCounter.cs +++ b/osu.Game/Screens/Play/HUD/DefaultAccuracyCounter.cs @@ -12,6 +12,8 @@ namespace osu.Game.Screens.Play.HUD [Resolved(canBeNull: true)] private HUDOverlay hud { get; set; } + public bool UsingClosestAnchor { get; set; } = true; + [BackgroundDependencyLoader] private void load(OsuColour colours) { diff --git a/osu.Game/Screens/Play/HUD/DefaultComboCounter.cs b/osu.Game/Screens/Play/HUD/DefaultComboCounter.cs index c4575c5ad0..97f33c0e07 100644 --- a/osu.Game/Screens/Play/HUD/DefaultComboCounter.cs +++ b/osu.Game/Screens/Play/HUD/DefaultComboCounter.cs @@ -17,6 +17,8 @@ namespace osu.Game.Screens.Play.HUD [Resolved(canBeNull: true)] private HUDOverlay hud { get; set; } + public bool UsingClosestAnchor { get; set; } = true; + public DefaultComboCounter() { Current.Value = DisplayedCount = 0; diff --git a/osu.Game/Screens/Play/HUD/DefaultHealthDisplay.cs b/osu.Game/Screens/Play/HUD/DefaultHealthDisplay.cs index ed297f0ffc..82f014d1e3 100644 --- a/osu.Game/Screens/Play/HUD/DefaultHealthDisplay.cs +++ b/osu.Game/Screens/Play/HUD/DefaultHealthDisplay.cs @@ -72,6 +72,8 @@ namespace osu.Game.Screens.Play.HUD } } + public bool UsingClosestAnchor { get; set; } = true; + public DefaultHealthDisplay() { Size = new Vector2(1, 5); diff --git a/osu.Game/Screens/Play/HUD/DefaultScoreCounter.cs b/osu.Game/Screens/Play/HUD/DefaultScoreCounter.cs index 16e3642181..56eaab4408 100644 --- a/osu.Game/Screens/Play/HUD/DefaultScoreCounter.cs +++ b/osu.Game/Screens/Play/HUD/DefaultScoreCounter.cs @@ -20,6 +20,8 @@ namespace osu.Game.Screens.Play.HUD [Resolved(canBeNull: true)] private HUDOverlay hud { get; set; } + public bool UsingClosestAnchor { get; set; } = true; + [BackgroundDependencyLoader] private void load(OsuColour colours) { diff --git a/osu.Game/Screens/Play/HUD/HitErrorMeters/HitErrorMeter.cs b/osu.Game/Screens/Play/HUD/HitErrorMeters/HitErrorMeter.cs index b0f9928b13..f263b5accc 100644 --- a/osu.Game/Screens/Play/HUD/HitErrorMeters/HitErrorMeter.cs +++ b/osu.Game/Screens/Play/HUD/HitErrorMeters/HitErrorMeter.cs @@ -22,6 +22,8 @@ namespace osu.Game.Screens.Play.HUD.HitErrorMeters [Resolved] private OsuColour colours { get; set; } + public bool UsingClosestAnchor { get; set; } = true; + [BackgroundDependencyLoader(true)] private void load(DrawableRuleset drawableRuleset) { diff --git a/osu.Game/Screens/Play/HUD/LegacyComboCounter.cs b/osu.Game/Screens/Play/HUD/LegacyComboCounter.cs index 1737634e31..9640bf1d4d 100644 --- a/osu.Game/Screens/Play/HUD/LegacyComboCounter.cs +++ b/osu.Game/Screens/Play/HUD/LegacyComboCounter.cs @@ -59,6 +59,8 @@ namespace osu.Game.Screens.Play.HUD set => counterContainer.Alpha = value ? 1 : 0; } + public bool UsingClosestAnchor { get; set; } = true; + public LegacyComboCounter() { AutoSizeAxes = Axes.Both; diff --git a/osu.Game/Screens/Play/HUD/SkinnableInfo.cs b/osu.Game/Screens/Play/HUD/SkinnableInfo.cs index 8ca5f1ba2d..5c733a9bc1 100644 --- a/osu.Game/Screens/Play/HUD/SkinnableInfo.cs +++ b/osu.Game/Screens/Play/HUD/SkinnableInfo.cs @@ -32,11 +32,7 @@ namespace osu.Game.Screens.Play.HUD public Anchor Origin { get; set; } - /// - ///

if this 's is - /// automatically determined by proximity, if the user has overridden it.

- ///

Corresponds to at runtime.

- ///
+ /// public bool UsingClosestAnchor { get; set; } = true; public List Children { get; } = new List(); diff --git a/osu.Game/Screens/Play/SongProgress.cs b/osu.Game/Screens/Play/SongProgress.cs index cab44c7473..e8687b9ab1 100644 --- a/osu.Game/Screens/Play/SongProgress.cs +++ b/osu.Game/Screens/Play/SongProgress.cs @@ -78,6 +78,8 @@ namespace osu.Game.Screens.Play private IClock referenceClock; + public bool UsingClosestAnchor { get; set; } = true; + public SongProgress() { RelativeSizeAxes = Axes.X; diff --git a/osu.Game/Skinning/ISkinnableDrawable.cs b/osu.Game/Skinning/ISkinnableDrawable.cs index d42b6f71b0..a0c7ae43a2 100644 --- a/osu.Game/Skinning/ISkinnableDrawable.cs +++ b/osu.Game/Skinning/ISkinnableDrawable.cs @@ -14,5 +14,11 @@ namespace osu.Game.Skinning /// Whether this component should be editable by an end user. ///
bool IsEditable => true; + + /// + /// if this 's is automatically determined by proximity, + /// if the user has overridden it. + /// + bool UsingClosestAnchor { get; set; } } } diff --git a/osu.Game/Skinning/LegacyAccuracyCounter.cs b/osu.Game/Skinning/LegacyAccuracyCounter.cs index 16562d9571..6c75ade365 100644 --- a/osu.Game/Skinning/LegacyAccuracyCounter.cs +++ b/osu.Game/Skinning/LegacyAccuracyCounter.cs @@ -12,6 +12,8 @@ namespace osu.Game.Skinning { public class LegacyAccuracyCounter : GameplayAccuracyCounter, ISkinnableDrawable { + public bool UsingClosestAnchor { get; set; } = true; + public LegacyAccuracyCounter() { Anchor = Anchor.TopRight; diff --git a/osu.Game/Skinning/LegacyHealthDisplay.cs b/osu.Game/Skinning/LegacyHealthDisplay.cs index c601adc3a0..d70d672189 100644 --- a/osu.Game/Skinning/LegacyHealthDisplay.cs +++ b/osu.Game/Skinning/LegacyHealthDisplay.cs @@ -30,6 +30,8 @@ namespace osu.Game.Skinning private bool isNewStyle; + public bool UsingClosestAnchor { get; set; } = true; + [BackgroundDependencyLoader] private void load() { diff --git a/osu.Game/Skinning/LegacyScoreCounter.cs b/osu.Game/Skinning/LegacyScoreCounter.cs index 64ea03d59c..944e42ed0e 100644 --- a/osu.Game/Skinning/LegacyScoreCounter.cs +++ b/osu.Game/Skinning/LegacyScoreCounter.cs @@ -13,6 +13,8 @@ namespace osu.Game.Skinning protected override double RollingDuration => 1000; protected override Easing RollingEasing => Easing.Out; + public bool UsingClosestAnchor { get; set; } = true; + public LegacyScoreCounter() : base(6) { diff --git a/osu.Game/Skinning/SkinnableTargetComponentsContainer.cs b/osu.Game/Skinning/SkinnableTargetComponentsContainer.cs index 2107ca7a8b..6acd917991 100644 --- a/osu.Game/Skinning/SkinnableTargetComponentsContainer.cs +++ b/osu.Game/Skinning/SkinnableTargetComponentsContainer.cs @@ -17,6 +17,8 @@ namespace osu.Game.Skinning { public bool IsEditable => false; + public bool UsingClosestAnchor { get; set; } = true; + private readonly Action applyDefaults; /// From f28916e30fafab0ac8148cc795bce2d0ad891cf8 Mon Sep 17 00:00:00 2001 From: Robin Avery Date: Mon, 7 Jun 2021 00:04:53 -0400 Subject: [PATCH 10/39] Remove all UsingClosestAnchor() extension logic It is replaced with ISkinnableDrawable.UsingClosestAnchor. --- osu.Game/Extensions/DrawableExtensions.cs | 28 ++----------------- osu.Game/Screens/Play/HUD/SkinnableInfo.cs | 5 +++- .../Skinning/Editor/SkinSelectionHandler.cs | 26 ++++++----------- 3 files changed, 15 insertions(+), 44 deletions(-) diff --git a/osu.Game/Extensions/DrawableExtensions.cs b/osu.Game/Extensions/DrawableExtensions.cs index f66fe2b8b2..1199db587a 100644 --- a/osu.Game/Extensions/DrawableExtensions.cs +++ b/osu.Game/Extensions/DrawableExtensions.cs @@ -2,9 +2,6 @@ // See the LICENCE file in the repository root for full licence text. using System; -using System.Collections.Generic; -using System.Runtime.CompilerServices; -using osu.Framework.Bindables; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Input.Bindings; @@ -60,7 +57,9 @@ namespace osu.Game.Extensions component.Scale = info.Scale; component.Anchor = info.Anchor; component.Origin = info.Origin; - component.UsingClosestAnchor().Value = info.UsingClosestAnchor; + + if (component is ISkinnableDrawable skinnable) + skinnable.UsingClosestAnchor = info.UsingClosestAnchor; if (component is Container container) { @@ -68,26 +67,5 @@ namespace osu.Game.Extensions container.Add(child.CreateInstance()); } } - - /// - ///

A ConditionalWeakTable is preferable to a Dictionary because a Dictionary will keep - /// orphaned references to an forever, unless manually pruned.

- ///

is used as a thin wrapper around bool because ConditionalWeakTable requires a reference type as both a key and a value.

- ///

was chosen over because it is a common ancestor between (which is required for logic) - /// and (which is required for serialization via ).

- ///

This collection is thread-safe according to the - /// documentation, - /// but the BindableBools are not unless leased.

- ///
- private static readonly ConditionalWeakTable is_drawable_using_closest_anchor_lookup = new ConditionalWeakTable(); - - /// - /// Gets or creates a representing whether is using the closest anchor point within its - /// parent. - /// - /// A whose is if the is using the closest anchor point, - /// otherwise . - public static BindableBool UsingClosestAnchor(this IDrawable drawable) => - is_drawable_using_closest_anchor_lookup.GetValue(drawable, _ => new BindableBool(true)); } } diff --git a/osu.Game/Screens/Play/HUD/SkinnableInfo.cs b/osu.Game/Screens/Play/HUD/SkinnableInfo.cs index 5c733a9bc1..a76634a1f0 100644 --- a/osu.Game/Screens/Play/HUD/SkinnableInfo.cs +++ b/osu.Game/Screens/Play/HUD/SkinnableInfo.cs @@ -55,7 +55,10 @@ namespace osu.Game.Screens.Play.HUD Scale = component.Scale; Anchor = component.Anchor; Origin = component.Origin; - UsingClosestAnchor = component.UsingClosestAnchor().Value; + + UsingClosestAnchor = + // true if it's not an ISkinnableDrawable + !(component is ISkinnableDrawable skinnable) || skinnable.UsingClosestAnchor; if (component is Container container) { diff --git a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs index 5acc949182..f4476b9de7 100644 --- a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs +++ b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs @@ -241,7 +241,7 @@ namespace osu.Game.Skinning.Editor private void updateDrawableAnchorIfUsingClosest(Drawable drawable) { - if (!drawable.UsingClosestAnchor().Value) return; + if (!(drawable is ISkinnableDrawable { UsingClosestAnchor: true })) return; var closestAnchor = getClosestAnchorForDrawable(drawable); @@ -265,8 +265,8 @@ namespace osu.Game.Skinning.Editor protected override IEnumerable GetContextMenuItemsForSelection(IEnumerable> selection) { - int checkAnchor(Drawable drawable) => - drawable.UsingClosestAnchor().Value + static int checkAnchor(Drawable drawable) => + drawable is ISkinnableDrawable { UsingClosestAnchor: true } ? closest_text_hash : (int)drawable.Anchor; @@ -331,8 +331,12 @@ namespace osu.Game.Skinning.Editor { var drawable = (Drawable)item; - var anchor = mapHashToAnchorSettings(hash, drawable); + var (usingClosest, anchor) = + hash == closest_text_hash + ? (true, getClosestAnchorForDrawable(drawable)) + : (false, (Anchor)hash); + item.UsingClosestAnchor = usingClosest; updateDrawableAnchor(drawable, anchor); } } @@ -344,20 +348,6 @@ namespace osu.Game.Skinning.Editor drawable.Position -= drawable.AnchorPosition - previousAnchor; } - private Anchor mapHashToAnchorSettings(int hash, Drawable drawable) - { - var isUsingClosestAnchor = drawable.UsingClosestAnchor(); - - if (hash == closest_text_hash) - { - isUsingClosestAnchor.Value = true; - return getClosestAnchorForDrawable(drawable); - } - - isUsingClosestAnchor.Value = false; - return (Anchor)hash; - } - private static void adjustScaleFromAnchor(ref Vector2 scale, Anchor reference) { // cancel out scale in axes we don't care about (based on which drag handle was used). From 133d72a8c0b02bc921160c1d70b2e960b11cb335 Mon Sep 17 00:00:00 2001 From: Robin Avery Date: Mon, 7 Jun 2021 00:14:36 -0400 Subject: [PATCH 11/39] Rename UsingClosestAnchor It is now "OverridesClosestAnchor". The logic is inverted accordingly. --- osu.Game/Extensions/DrawableExtensions.cs | 2 +- .../Screens/Play/HUD/DefaultAccuracyCounter.cs | 2 +- osu.Game/Screens/Play/HUD/DefaultComboCounter.cs | 2 +- .../Screens/Play/HUD/DefaultHealthDisplay.cs | 2 +- osu.Game/Screens/Play/HUD/DefaultScoreCounter.cs | 2 +- .../Play/HUD/HitErrorMeters/HitErrorMeter.cs | 2 +- osu.Game/Screens/Play/HUD/LegacyComboCounter.cs | 2 +- osu.Game/Screens/Play/HUD/SkinnableInfo.cs | 8 +++----- osu.Game/Screens/Play/SongProgress.cs | 2 +- osu.Game/Skinning/Editor/SkinSelectionHandler.cs | 16 ++++++++-------- osu.Game/Skinning/ISkinnableDrawable.cs | 6 +++--- osu.Game/Skinning/LegacyAccuracyCounter.cs | 2 +- osu.Game/Skinning/LegacyHealthDisplay.cs | 2 +- osu.Game/Skinning/LegacyScoreCounter.cs | 2 +- .../SkinnableTargetComponentsContainer.cs | 2 +- 15 files changed, 26 insertions(+), 28 deletions(-) diff --git a/osu.Game/Extensions/DrawableExtensions.cs b/osu.Game/Extensions/DrawableExtensions.cs index 1199db587a..11a8112ecf 100644 --- a/osu.Game/Extensions/DrawableExtensions.cs +++ b/osu.Game/Extensions/DrawableExtensions.cs @@ -59,7 +59,7 @@ namespace osu.Game.Extensions component.Origin = info.Origin; if (component is ISkinnableDrawable skinnable) - skinnable.UsingClosestAnchor = info.UsingClosestAnchor; + skinnable.OverridesClosestAnchor = info.OverridesClosestAnchor; if (component is Container container) { diff --git a/osu.Game/Screens/Play/HUD/DefaultAccuracyCounter.cs b/osu.Game/Screens/Play/HUD/DefaultAccuracyCounter.cs index 5c6bb7299a..31edab5bc2 100644 --- a/osu.Game/Screens/Play/HUD/DefaultAccuracyCounter.cs +++ b/osu.Game/Screens/Play/HUD/DefaultAccuracyCounter.cs @@ -12,7 +12,7 @@ namespace osu.Game.Screens.Play.HUD [Resolved(canBeNull: true)] private HUDOverlay hud { get; set; } - public bool UsingClosestAnchor { get; set; } = true; + public bool OverridesClosestAnchor { get; set; } [BackgroundDependencyLoader] private void load(OsuColour colours) diff --git a/osu.Game/Screens/Play/HUD/DefaultComboCounter.cs b/osu.Game/Screens/Play/HUD/DefaultComboCounter.cs index 97f33c0e07..bf9bed3fe3 100644 --- a/osu.Game/Screens/Play/HUD/DefaultComboCounter.cs +++ b/osu.Game/Screens/Play/HUD/DefaultComboCounter.cs @@ -17,7 +17,7 @@ namespace osu.Game.Screens.Play.HUD [Resolved(canBeNull: true)] private HUDOverlay hud { get; set; } - public bool UsingClosestAnchor { get; set; } = true; + public bool OverridesClosestAnchor { get; set; } public DefaultComboCounter() { diff --git a/osu.Game/Screens/Play/HUD/DefaultHealthDisplay.cs b/osu.Game/Screens/Play/HUD/DefaultHealthDisplay.cs index 82f014d1e3..3488de70a9 100644 --- a/osu.Game/Screens/Play/HUD/DefaultHealthDisplay.cs +++ b/osu.Game/Screens/Play/HUD/DefaultHealthDisplay.cs @@ -72,7 +72,7 @@ namespace osu.Game.Screens.Play.HUD } } - public bool UsingClosestAnchor { get; set; } = true; + public bool OverridesClosestAnchor { get; set; } public DefaultHealthDisplay() { diff --git a/osu.Game/Screens/Play/HUD/DefaultScoreCounter.cs b/osu.Game/Screens/Play/HUD/DefaultScoreCounter.cs index 56eaab4408..de534f516a 100644 --- a/osu.Game/Screens/Play/HUD/DefaultScoreCounter.cs +++ b/osu.Game/Screens/Play/HUD/DefaultScoreCounter.cs @@ -20,7 +20,7 @@ namespace osu.Game.Screens.Play.HUD [Resolved(canBeNull: true)] private HUDOverlay hud { get; set; } - public bool UsingClosestAnchor { get; set; } = true; + public bool OverridesClosestAnchor { get; set; } [BackgroundDependencyLoader] private void load(OsuColour colours) diff --git a/osu.Game/Screens/Play/HUD/HitErrorMeters/HitErrorMeter.cs b/osu.Game/Screens/Play/HUD/HitErrorMeters/HitErrorMeter.cs index f263b5accc..a16adfebbc 100644 --- a/osu.Game/Screens/Play/HUD/HitErrorMeters/HitErrorMeter.cs +++ b/osu.Game/Screens/Play/HUD/HitErrorMeters/HitErrorMeter.cs @@ -22,7 +22,7 @@ namespace osu.Game.Screens.Play.HUD.HitErrorMeters [Resolved] private OsuColour colours { get; set; } - public bool UsingClosestAnchor { get; set; } = true; + public bool OverridesClosestAnchor { get; set; } [BackgroundDependencyLoader(true)] private void load(DrawableRuleset drawableRuleset) diff --git a/osu.Game/Screens/Play/HUD/LegacyComboCounter.cs b/osu.Game/Screens/Play/HUD/LegacyComboCounter.cs index 9640bf1d4d..aff98b843b 100644 --- a/osu.Game/Screens/Play/HUD/LegacyComboCounter.cs +++ b/osu.Game/Screens/Play/HUD/LegacyComboCounter.cs @@ -59,7 +59,7 @@ namespace osu.Game.Screens.Play.HUD set => counterContainer.Alpha = value ? 1 : 0; } - public bool UsingClosestAnchor { get; set; } = true; + public bool OverridesClosestAnchor { get; set; } public LegacyComboCounter() { diff --git a/osu.Game/Screens/Play/HUD/SkinnableInfo.cs b/osu.Game/Screens/Play/HUD/SkinnableInfo.cs index a76634a1f0..207bea77ce 100644 --- a/osu.Game/Screens/Play/HUD/SkinnableInfo.cs +++ b/osu.Game/Screens/Play/HUD/SkinnableInfo.cs @@ -32,8 +32,8 @@ namespace osu.Game.Screens.Play.HUD public Anchor Origin { get; set; } - /// - public bool UsingClosestAnchor { get; set; } = true; + /// + public bool OverridesClosestAnchor { get; set; } public List Children { get; } = new List(); @@ -56,9 +56,7 @@ namespace osu.Game.Screens.Play.HUD Anchor = component.Anchor; Origin = component.Origin; - UsingClosestAnchor = - // true if it's not an ISkinnableDrawable - !(component is ISkinnableDrawable skinnable) || skinnable.UsingClosestAnchor; + OverridesClosestAnchor = component is ISkinnableDrawable { OverridesClosestAnchor: true }; if (component is Container container) { diff --git a/osu.Game/Screens/Play/SongProgress.cs b/osu.Game/Screens/Play/SongProgress.cs index e8687b9ab1..a00a2c0275 100644 --- a/osu.Game/Screens/Play/SongProgress.cs +++ b/osu.Game/Screens/Play/SongProgress.cs @@ -78,7 +78,7 @@ namespace osu.Game.Screens.Play private IClock referenceClock; - public bool UsingClosestAnchor { get; set; } = true; + public bool OverridesClosestAnchor { get; set; } public SongProgress() { diff --git a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs index f4476b9de7..7ddcebd662 100644 --- a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs +++ b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs @@ -241,7 +241,7 @@ namespace osu.Game.Skinning.Editor private void updateDrawableAnchorIfUsingClosest(Drawable drawable) { - if (!(drawable is ISkinnableDrawable { UsingClosestAnchor: true })) return; + if (drawable is ISkinnableDrawable { OverridesClosestAnchor: true }) return; var closestAnchor = getClosestAnchorForDrawable(drawable); @@ -266,9 +266,9 @@ namespace osu.Game.Skinning.Editor protected override IEnumerable GetContextMenuItemsForSelection(IEnumerable> selection) { static int checkAnchor(Drawable drawable) => - drawable is ISkinnableDrawable { UsingClosestAnchor: true } - ? closest_text_hash - : (int)drawable.Anchor; + drawable is ISkinnableDrawable { OverridesClosestAnchor: true } + ? (int)drawable.Anchor + : closest_text_hash; yield return new OsuMenuItem(nameof(Anchor)) { @@ -331,12 +331,12 @@ namespace osu.Game.Skinning.Editor { var drawable = (Drawable)item; - var (usingClosest, anchor) = + var (overridesClosest, anchor) = hash == closest_text_hash - ? (true, getClosestAnchorForDrawable(drawable)) - : (false, (Anchor)hash); + ? (false, getClosestAnchorForDrawable(drawable)) + : (true, (Anchor)hash); - item.UsingClosestAnchor = usingClosest; + item.OverridesClosestAnchor = overridesClosest; updateDrawableAnchor(drawable, anchor); } } diff --git a/osu.Game/Skinning/ISkinnableDrawable.cs b/osu.Game/Skinning/ISkinnableDrawable.cs index a0c7ae43a2..1b5c0df1b2 100644 --- a/osu.Game/Skinning/ISkinnableDrawable.cs +++ b/osu.Game/Skinning/ISkinnableDrawable.cs @@ -16,9 +16,9 @@ namespace osu.Game.Skinning bool IsEditable => true; /// - /// if this 's is automatically determined by proximity, - /// if the user has overridden it. + /// if this 's is automatically determined by proximity, + /// if the user has overridden it. /// - bool UsingClosestAnchor { get; set; } + bool OverridesClosestAnchor { get; set; } } } diff --git a/osu.Game/Skinning/LegacyAccuracyCounter.cs b/osu.Game/Skinning/LegacyAccuracyCounter.cs index 6c75ade365..603f0ecffb 100644 --- a/osu.Game/Skinning/LegacyAccuracyCounter.cs +++ b/osu.Game/Skinning/LegacyAccuracyCounter.cs @@ -12,7 +12,7 @@ namespace osu.Game.Skinning { public class LegacyAccuracyCounter : GameplayAccuracyCounter, ISkinnableDrawable { - public bool UsingClosestAnchor { get; set; } = true; + public bool OverridesClosestAnchor { get; set; } public LegacyAccuracyCounter() { diff --git a/osu.Game/Skinning/LegacyHealthDisplay.cs b/osu.Game/Skinning/LegacyHealthDisplay.cs index d70d672189..a419f981b5 100644 --- a/osu.Game/Skinning/LegacyHealthDisplay.cs +++ b/osu.Game/Skinning/LegacyHealthDisplay.cs @@ -30,7 +30,7 @@ namespace osu.Game.Skinning private bool isNewStyle; - public bool UsingClosestAnchor { get; set; } = true; + public bool OverridesClosestAnchor { get; set; } [BackgroundDependencyLoader] private void load() diff --git a/osu.Game/Skinning/LegacyScoreCounter.cs b/osu.Game/Skinning/LegacyScoreCounter.cs index 944e42ed0e..206e10943d 100644 --- a/osu.Game/Skinning/LegacyScoreCounter.cs +++ b/osu.Game/Skinning/LegacyScoreCounter.cs @@ -13,7 +13,7 @@ namespace osu.Game.Skinning protected override double RollingDuration => 1000; protected override Easing RollingEasing => Easing.Out; - public bool UsingClosestAnchor { get; set; } = true; + public bool OverridesClosestAnchor { get; set; } public LegacyScoreCounter() : base(6) diff --git a/osu.Game/Skinning/SkinnableTargetComponentsContainer.cs b/osu.Game/Skinning/SkinnableTargetComponentsContainer.cs index 6acd917991..b661a02ca5 100644 --- a/osu.Game/Skinning/SkinnableTargetComponentsContainer.cs +++ b/osu.Game/Skinning/SkinnableTargetComponentsContainer.cs @@ -17,7 +17,7 @@ namespace osu.Game.Skinning { public bool IsEditable => false; - public bool UsingClosestAnchor { get; set; } = true; + public bool OverridesClosestAnchor { get; set; } private readonly Action applyDefaults; From 29fa4fdf573176d68dda7996b8ccd6b5314caa0c Mon Sep 17 00:00:00 2001 From: Robin Avery Date: Mon, 7 Jun 2021 01:08:39 -0400 Subject: [PATCH 12/39] Refactor unacceptable syntax --- osu.Game/Screens/Play/HUD/SkinnableInfo.cs | 3 +- .../Skinning/Editor/SkinSelectionHandler.cs | 30 ++++++++++++------- 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/osu.Game/Screens/Play/HUD/SkinnableInfo.cs b/osu.Game/Screens/Play/HUD/SkinnableInfo.cs index 207bea77ce..1549ba47e0 100644 --- a/osu.Game/Screens/Play/HUD/SkinnableInfo.cs +++ b/osu.Game/Screens/Play/HUD/SkinnableInfo.cs @@ -56,7 +56,8 @@ namespace osu.Game.Screens.Play.HUD Anchor = component.Anchor; Origin = component.Origin; - OverridesClosestAnchor = component is ISkinnableDrawable { OverridesClosestAnchor: true }; + if (component is ISkinnableDrawable skinnable) + OverridesClosestAnchor = skinnable.OverridesClosestAnchor; if (component is Container container) { diff --git a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs index 7ddcebd662..74609a6a1a 100644 --- a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs +++ b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs @@ -233,15 +233,17 @@ namespace osu.Game.Skinning.Editor Drawable drawable = (Drawable)c.Item; drawable.Position += drawable.ScreenSpaceDeltaToParentSpace(moveEvent.ScreenSpaceDelta); - updateDrawableAnchorIfUsingClosest(drawable); + updateDrawableAnchorIfUsingClosest(c.Item); } return true; } - private void updateDrawableAnchorIfUsingClosest(Drawable drawable) + private void updateDrawableAnchorIfUsingClosest(ISkinnableDrawable item) { - if (drawable is ISkinnableDrawable { OverridesClosestAnchor: true }) return; + if (item.OverridesClosestAnchor) return; + + var drawable = (Drawable)item; var closestAnchor = getClosestAnchorForDrawable(drawable); @@ -265,10 +267,16 @@ namespace osu.Game.Skinning.Editor protected override IEnumerable GetContextMenuItemsForSelection(IEnumerable> selection) { - static int checkAnchor(Drawable drawable) => - drawable is ISkinnableDrawable { OverridesClosestAnchor: true } - ? (int)drawable.Anchor - : closest_text_hash; + static int checkAnchor(ISkinnableDrawable item) + { + if (item.OverridesClosestAnchor) + { + var drawable = (Drawable)item; + return (int)drawable.Anchor; + } + + return closest_text_hash; + } yield return new OsuMenuItem(nameof(Anchor)) { @@ -278,20 +286,20 @@ namespace osu.Game.Skinning.Editor yield return new OsuMenuItem(nameof(Origin)) { // Origins can't be "closest" so we just cast to int - Items = createAnchorItems(origin_menu_items, d => (int)d.Origin, applyOrigin).ToArray() + Items = createAnchorItems(origin_menu_items, d => (int)((Drawable)d).Origin, applyOrigin).ToArray() }; foreach (var item in base.GetContextMenuItemsForSelection(selection)) yield return item; - IEnumerable createAnchorItems(IDictionary items, Func checkFunction, Action applyFunction) => + IEnumerable createAnchorItems(IDictionary items, Func checkFunction, Action applyFunction) => items.Select(pair => { var (hash, text) = pair; return new TernaryStateRadioMenuItem(text, MenuItemType.Standard, _ => applyFunction(hash)) { - State = { Value = GetStateFromSelection(selection, c => checkFunction((Drawable)c.Item) == hash) } + State = { Value = GetStateFromSelection(selection, c => checkFunction(c.Item) == hash) } }; }); } @@ -314,7 +322,7 @@ namespace osu.Game.Skinning.Editor drawable.Origin = anchor; drawable.Position += drawable.OriginPosition - previousOrigin; - updateDrawableAnchorIfUsingClosest(drawable); + updateDrawableAnchorIfUsingClosest(item); } } From 6c9594ee350a1710cc9a9eb783a8a9bad27feff6 Mon Sep 17 00:00:00 2001 From: Robin Avery Date: Mon, 7 Jun 2021 02:40:15 -0400 Subject: [PATCH 13/39] Simplify and rearrange SkinSelectionHandler The file has been restructured and reworded such that there are as few differences as possible from b36b40cb3430e6bd511390f82c39ba23195dc8cb. --- .../Skinning/Editor/SkinSelectionHandler.cs | 205 ++++++++---------- 1 file changed, 90 insertions(+), 115 deletions(-) diff --git a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs index 74609a6a1a..b7fe5beb5c 100644 --- a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs +++ b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs @@ -20,87 +20,6 @@ namespace osu.Game.Skinning.Editor { public class SkinSelectionHandler : SelectionHandler { - private const string closest_text = @"Closest"; - - /// - /// The hash code of the "Closest" menu item in the anchor point context menu. - /// - /// Needs only be constant and distinct from any values. - private static readonly int closest_text_hash = closest_text.GetHashCode(); - - private static readonly Dictionary anchor_menu_items; - private static readonly Dictionary origin_menu_items; - - static SkinSelectionHandler() - { - var anchorMenuSubset = new[] - { - Anchor.TopLeft, - Anchor.TopCentre, - Anchor.TopRight, - Anchor.CentreLeft, - Anchor.Centre, - Anchor.CentreRight, - Anchor.BottomLeft, - Anchor.BottomCentre, - Anchor.BottomRight, - }; - - var texts = anchorMenuSubset.Select(a => a.ToString()).Prepend(closest_text).ToArray(); - var hashes = anchorMenuSubset.Cast().Prepend(closest_text_hash).ToArray(); - - var anchorPairs = hashes.Zip(texts, (k, v) => new KeyValuePair(k, v)).ToArray(); - anchor_menu_items = new Dictionary(anchorPairs); - var originPairs = anchorPairs.Where(pair => pair.Key != closest_text_hash); - origin_menu_items = new Dictionary(originPairs); - } - - private Anchor getClosestAnchorForDrawable(Drawable drawable) - { - var parent = drawable.Parent; - if (parent == null) - return drawable.Anchor; - - var screenPosition = getOriginPositionFromQuad(drawable.ScreenSpaceDrawQuad, drawable.Origin); - var absolutePosition = parent.ToLocalSpace(screenPosition); - var factor = parent.RelativeToAbsoluteFactor; - - var result = default(Anchor); - - result |= getTieredComponent(absolutePosition.X / factor.X, Anchor.x0, Anchor.x1, Anchor.x2); - result |= getTieredComponent(absolutePosition.Y / factor.Y, Anchor.y0, Anchor.y1, Anchor.y2); - - return result; - } - - private static Anchor getTieredComponent(float component, Anchor tier0, Anchor tier1, Anchor tier2) - { - if (component >= 2 / 3f) - return tier2; - - if (component >= 1 / 3f) - return tier1; - - return tier0; - } - - private Vector2 getOriginPositionFromQuad(in Quad quad, Anchor origin) - { - var result = quad.TopLeft; - - if (origin.HasFlagFast(Anchor.x2)) - result.X += quad.Width; - else if (origin.HasFlagFast(Anchor.x1)) - result.X += quad.Width / 2f; - - if (origin.HasFlagFast(Anchor.y2)) - result.Y += quad.Height; - else if (origin.HasFlagFast(Anchor.y1)) - result.Y += quad.Height / 2f; - - return result; - } - [Resolved] private SkinEditor skinEditor { get; set; } @@ -267,41 +186,48 @@ namespace osu.Game.Skinning.Editor protected override IEnumerable GetContextMenuItemsForSelection(IEnumerable> selection) { - static int checkAnchor(ISkinnableDrawable item) + var closestItem = new TernaryStateRadioMenuItem("Closest", MenuItemType.Standard, _ => applyClosestAnchor()) { - if (item.OverridesClosestAnchor) - { - var drawable = (Drawable)item; - return (int)drawable.Anchor; - } - - return closest_text_hash; - } - - yield return new OsuMenuItem(nameof(Anchor)) - { - Items = createAnchorItems(anchor_menu_items, checkAnchor, applyAnchor).ToArray() + State = { Value = GetStateFromSelection(selection, c => !c.Item.OverridesClosestAnchor) } }; - yield return new OsuMenuItem(nameof(Origin)) + yield return new OsuMenuItem("Anchor") { - // Origins can't be "closest" so we just cast to int - Items = createAnchorItems(origin_menu_items, d => (int)((Drawable)d).Origin, applyOrigin).ToArray() + Items = createAnchorItems((i, a) => i.OverridesClosestAnchor && ((Drawable)i).Anchor == a, applyAnchor) + .Prepend(closestItem) + .ToArray() + }; + + yield return new OsuMenuItem("Origin") + { + Items = createAnchorItems((i, o) => ((Drawable)i).Origin == o, applyOrigin).ToArray() }; foreach (var item in base.GetContextMenuItemsForSelection(selection)) yield return item; - IEnumerable createAnchorItems(IDictionary items, Func checkFunction, Action applyFunction) => - items.Select(pair => + IEnumerable createAnchorItems(Func checkFunction, Action applyFunction) + { + var displayableAnchors = new[] { - var (hash, text) = pair; - - return new TernaryStateRadioMenuItem(text, MenuItemType.Standard, _ => applyFunction(hash)) + Anchor.TopLeft, + Anchor.TopCentre, + Anchor.TopRight, + Anchor.CentreLeft, + Anchor.Centre, + Anchor.CentreRight, + Anchor.BottomLeft, + Anchor.BottomCentre, + Anchor.BottomRight, + }; + return displayableAnchors.Select(a => + { + return new TernaryStateRadioMenuItem(a.ToString(), MenuItemType.Standard, _ => applyFunction(a)) { - State = { Value = GetStateFromSelection(selection, c => checkFunction(c.Item) == hash) } + State = { Value = GetStateFromSelection(selection, c => checkFunction(c.Item, a)) } }; }); + } } private static void updateDrawablePosition(Drawable drawable, Vector2 screenSpacePosition) @@ -310,10 +236,8 @@ namespace osu.Game.Skinning.Editor drawable.Parent.ToLocalSpace(screenSpacePosition) - drawable.AnchorPosition; } - private void applyOrigin(int hash) + private void applyOrigin(Anchor anchor) { - var anchor = (Anchor)hash; - foreach (var item in SelectedItems) { var drawable = (Drawable)item; @@ -321,8 +245,6 @@ namespace osu.Game.Skinning.Editor var previousOrigin = drawable.OriginPosition; drawable.Origin = anchor; drawable.Position += drawable.OriginPosition - previousOrigin; - - updateDrawableAnchorIfUsingClosest(item); } } @@ -333,22 +255,75 @@ namespace osu.Game.Skinning.Editor private Quad getSelectionQuad() => GetSurroundingQuad(SelectedBlueprints.SelectMany(b => b.Item.ScreenSpaceDrawQuad.GetVertices().ToArray())); - private void applyAnchor(int hash) + private void applyAnchor(Anchor anchor) { foreach (var item in SelectedItems) { var drawable = (Drawable)item; - var (overridesClosest, anchor) = - hash == closest_text_hash - ? (false, getClosestAnchorForDrawable(drawable)) - : (true, (Anchor)hash); - - item.OverridesClosestAnchor = overridesClosest; + item.OverridesClosestAnchor = true; updateDrawableAnchor(drawable, anchor); } } + private void applyClosestAnchor() + { + foreach (var item in SelectedItems) + { + var drawable = (Drawable)item; + + item.OverridesClosestAnchor = false; + updateDrawableAnchor(drawable, getClosestAnchorForDrawable(drawable)); + } + } + + private static Anchor getClosestAnchorForDrawable(Drawable drawable) + { + var parent = drawable.Parent; + + if (parent == null) + return drawable.Anchor; + + var screenPosition = getOriginPositionFromQuad(drawable.ScreenSpaceDrawQuad, drawable.Origin); + var absolutePosition = parent.ToLocalSpace(screenPosition); + var factor = parent.RelativeToAbsoluteFactor; + + var result = default(Anchor); + + result |= getTieredComponent(absolutePosition.X / factor.X, Anchor.x0, Anchor.x1, Anchor.x2); + result |= getTieredComponent(absolutePosition.Y / factor.Y, Anchor.y0, Anchor.y1, Anchor.y2); + + return result; + } + + private static Vector2 getOriginPositionFromQuad(in Quad quad, Anchor origin) + { + var result = quad.TopLeft; + + if (origin.HasFlagFast(Anchor.x2)) + result.X += quad.Width; + else if (origin.HasFlagFast(Anchor.x1)) + result.X += quad.Width / 2f; + + if (origin.HasFlagFast(Anchor.y2)) + result.Y += quad.Height; + else if (origin.HasFlagFast(Anchor.y1)) + result.Y += quad.Height / 2f; + + return result; + } + + private static Anchor getTieredComponent(float component, Anchor tier0, Anchor tier1, Anchor tier2) + { + if (component >= 2 / 3f) + return tier2; + + if (component >= 1 / 3f) + return tier1; + + return tier0; + } + private static void updateDrawableAnchor(Drawable drawable, Anchor anchor) { var previousAnchor = drawable.AnchorPosition; From 65f594f860b5e0be75e0c4b4eb09d50e859b6ba8 Mon Sep 17 00:00:00 2001 From: Robin Avery Date: Mon, 7 Jun 2021 05:08:18 -0400 Subject: [PATCH 14/39] Rename `applyAnchor` to `applyCustomAnchor` --- osu.Game/Skinning/Editor/SkinSelectionHandler.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs index b7fe5beb5c..118c3308b2 100644 --- a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs +++ b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs @@ -193,7 +193,7 @@ namespace osu.Game.Skinning.Editor yield return new OsuMenuItem("Anchor") { - Items = createAnchorItems((i, a) => i.OverridesClosestAnchor && ((Drawable)i).Anchor == a, applyAnchor) + Items = createAnchorItems((i, a) => i.OverridesClosestAnchor && ((Drawable)i).Anchor == a, applyCustomAnchor) .Prepend(closestItem) .ToArray() }; @@ -255,7 +255,7 @@ namespace osu.Game.Skinning.Editor private Quad getSelectionQuad() => GetSurroundingQuad(SelectedBlueprints.SelectMany(b => b.Item.ScreenSpaceDrawQuad.GetVertices().ToArray())); - private void applyAnchor(Anchor anchor) + private void applyCustomAnchor(Anchor anchor) { foreach (var item in SelectedItems) { From dc50ae40b9b604785c1093c3c5cdc9dbb6160876 Mon Sep 17 00:00:00 2001 From: Robin Avery Date: Tue, 8 Jun 2021 08:22:35 -0400 Subject: [PATCH 15/39] Rename `OverridesClosestAnchor` to `UsesFixedAnchor` --- osu.Game/Extensions/DrawableExtensions.cs | 2 +- osu.Game/Screens/Play/HUD/DefaultAccuracyCounter.cs | 2 +- osu.Game/Screens/Play/HUD/DefaultComboCounter.cs | 2 +- osu.Game/Screens/Play/HUD/DefaultHealthDisplay.cs | 2 +- osu.Game/Screens/Play/HUD/DefaultScoreCounter.cs | 2 +- .../Screens/Play/HUD/HitErrorMeters/HitErrorMeter.cs | 2 +- osu.Game/Screens/Play/HUD/LegacyComboCounter.cs | 2 +- osu.Game/Screens/Play/HUD/SkinnableInfo.cs | 6 +++--- osu.Game/Screens/Play/SongProgress.cs | 2 +- osu.Game/Skinning/Editor/SkinSelectionHandler.cs | 10 +++++----- osu.Game/Skinning/ISkinnableDrawable.cs | 4 ++-- osu.Game/Skinning/LegacyAccuracyCounter.cs | 2 +- osu.Game/Skinning/LegacyHealthDisplay.cs | 2 +- osu.Game/Skinning/LegacyScoreCounter.cs | 2 +- .../Skinning/SkinnableTargetComponentsContainer.cs | 2 +- 15 files changed, 22 insertions(+), 22 deletions(-) diff --git a/osu.Game/Extensions/DrawableExtensions.cs b/osu.Game/Extensions/DrawableExtensions.cs index 11a8112ecf..52d8230fb6 100644 --- a/osu.Game/Extensions/DrawableExtensions.cs +++ b/osu.Game/Extensions/DrawableExtensions.cs @@ -59,7 +59,7 @@ namespace osu.Game.Extensions component.Origin = info.Origin; if (component is ISkinnableDrawable skinnable) - skinnable.OverridesClosestAnchor = info.OverridesClosestAnchor; + skinnable.UsesFixedAnchor = info.UsesFixedAnchor; if (component is Container container) { diff --git a/osu.Game/Screens/Play/HUD/DefaultAccuracyCounter.cs b/osu.Game/Screens/Play/HUD/DefaultAccuracyCounter.cs index 31edab5bc2..324e5d43b5 100644 --- a/osu.Game/Screens/Play/HUD/DefaultAccuracyCounter.cs +++ b/osu.Game/Screens/Play/HUD/DefaultAccuracyCounter.cs @@ -12,7 +12,7 @@ namespace osu.Game.Screens.Play.HUD [Resolved(canBeNull: true)] private HUDOverlay hud { get; set; } - public bool OverridesClosestAnchor { get; set; } + public bool UsesFixedAnchor { get; set; } [BackgroundDependencyLoader] private void load(OsuColour colours) diff --git a/osu.Game/Screens/Play/HUD/DefaultComboCounter.cs b/osu.Game/Screens/Play/HUD/DefaultComboCounter.cs index bf9bed3fe3..718ae24cf1 100644 --- a/osu.Game/Screens/Play/HUD/DefaultComboCounter.cs +++ b/osu.Game/Screens/Play/HUD/DefaultComboCounter.cs @@ -17,7 +17,7 @@ namespace osu.Game.Screens.Play.HUD [Resolved(canBeNull: true)] private HUDOverlay hud { get; set; } - public bool OverridesClosestAnchor { get; set; } + public bool UsesFixedAnchor { get; set; } public DefaultComboCounter() { diff --git a/osu.Game/Screens/Play/HUD/DefaultHealthDisplay.cs b/osu.Game/Screens/Play/HUD/DefaultHealthDisplay.cs index 3488de70a9..4f93868a66 100644 --- a/osu.Game/Screens/Play/HUD/DefaultHealthDisplay.cs +++ b/osu.Game/Screens/Play/HUD/DefaultHealthDisplay.cs @@ -72,7 +72,7 @@ namespace osu.Game.Screens.Play.HUD } } - public bool OverridesClosestAnchor { get; set; } + public bool UsesFixedAnchor { get; set; } public DefaultHealthDisplay() { diff --git a/osu.Game/Screens/Play/HUD/DefaultScoreCounter.cs b/osu.Game/Screens/Play/HUD/DefaultScoreCounter.cs index de534f516a..63de5c8de5 100644 --- a/osu.Game/Screens/Play/HUD/DefaultScoreCounter.cs +++ b/osu.Game/Screens/Play/HUD/DefaultScoreCounter.cs @@ -20,7 +20,7 @@ namespace osu.Game.Screens.Play.HUD [Resolved(canBeNull: true)] private HUDOverlay hud { get; set; } - public bool OverridesClosestAnchor { get; set; } + public bool UsesFixedAnchor { get; set; } [BackgroundDependencyLoader] private void load(OsuColour colours) diff --git a/osu.Game/Screens/Play/HUD/HitErrorMeters/HitErrorMeter.cs b/osu.Game/Screens/Play/HUD/HitErrorMeters/HitErrorMeter.cs index a16adfebbc..ab8b5c6cb1 100644 --- a/osu.Game/Screens/Play/HUD/HitErrorMeters/HitErrorMeter.cs +++ b/osu.Game/Screens/Play/HUD/HitErrorMeters/HitErrorMeter.cs @@ -22,7 +22,7 @@ namespace osu.Game.Screens.Play.HUD.HitErrorMeters [Resolved] private OsuColour colours { get; set; } - public bool OverridesClosestAnchor { get; set; } + public bool UsesFixedAnchor { get; set; } [BackgroundDependencyLoader(true)] private void load(DrawableRuleset drawableRuleset) diff --git a/osu.Game/Screens/Play/HUD/LegacyComboCounter.cs b/osu.Game/Screens/Play/HUD/LegacyComboCounter.cs index aff98b843b..acff949353 100644 --- a/osu.Game/Screens/Play/HUD/LegacyComboCounter.cs +++ b/osu.Game/Screens/Play/HUD/LegacyComboCounter.cs @@ -59,7 +59,7 @@ namespace osu.Game.Screens.Play.HUD set => counterContainer.Alpha = value ? 1 : 0; } - public bool OverridesClosestAnchor { get; set; } + public bool UsesFixedAnchor { get; set; } public LegacyComboCounter() { diff --git a/osu.Game/Screens/Play/HUD/SkinnableInfo.cs b/osu.Game/Screens/Play/HUD/SkinnableInfo.cs index 1549ba47e0..b64e5ca98f 100644 --- a/osu.Game/Screens/Play/HUD/SkinnableInfo.cs +++ b/osu.Game/Screens/Play/HUD/SkinnableInfo.cs @@ -32,8 +32,8 @@ namespace osu.Game.Screens.Play.HUD public Anchor Origin { get; set; } - /// - public bool OverridesClosestAnchor { get; set; } + /// + public bool UsesFixedAnchor { get; set; } public List Children { get; } = new List(); @@ -57,7 +57,7 @@ namespace osu.Game.Screens.Play.HUD Origin = component.Origin; if (component is ISkinnableDrawable skinnable) - OverridesClosestAnchor = skinnable.OverridesClosestAnchor; + UsesFixedAnchor = skinnable.UsesFixedAnchor; if (component is Container container) { diff --git a/osu.Game/Screens/Play/SongProgress.cs b/osu.Game/Screens/Play/SongProgress.cs index a00a2c0275..bd861dc598 100644 --- a/osu.Game/Screens/Play/SongProgress.cs +++ b/osu.Game/Screens/Play/SongProgress.cs @@ -78,7 +78,7 @@ namespace osu.Game.Screens.Play private IClock referenceClock; - public bool OverridesClosestAnchor { get; set; } + public bool UsesFixedAnchor { get; set; } public SongProgress() { diff --git a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs index 118c3308b2..78173bfda1 100644 --- a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs +++ b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs @@ -160,7 +160,7 @@ namespace osu.Game.Skinning.Editor private void updateDrawableAnchorIfUsingClosest(ISkinnableDrawable item) { - if (item.OverridesClosestAnchor) return; + if (item.UsesFixedAnchor) return; var drawable = (Drawable)item; @@ -188,12 +188,12 @@ namespace osu.Game.Skinning.Editor { var closestItem = new TernaryStateRadioMenuItem("Closest", MenuItemType.Standard, _ => applyClosestAnchor()) { - State = { Value = GetStateFromSelection(selection, c => !c.Item.OverridesClosestAnchor) } + State = { Value = GetStateFromSelection(selection, c => !c.Item.UsesFixedAnchor) } }; yield return new OsuMenuItem("Anchor") { - Items = createAnchorItems((i, a) => i.OverridesClosestAnchor && ((Drawable)i).Anchor == a, applyCustomAnchor) + Items = createAnchorItems((i, a) => i.UsesFixedAnchor && ((Drawable)i).Anchor == a, applyCustomAnchor) .Prepend(closestItem) .ToArray() }; @@ -261,7 +261,7 @@ namespace osu.Game.Skinning.Editor { var drawable = (Drawable)item; - item.OverridesClosestAnchor = true; + item.UsesFixedAnchor = true; updateDrawableAnchor(drawable, anchor); } } @@ -272,7 +272,7 @@ namespace osu.Game.Skinning.Editor { var drawable = (Drawable)item; - item.OverridesClosestAnchor = false; + item.UsesFixedAnchor = false; updateDrawableAnchor(drawable, getClosestAnchorForDrawable(drawable)); } } diff --git a/osu.Game/Skinning/ISkinnableDrawable.cs b/osu.Game/Skinning/ISkinnableDrawable.cs index 1b5c0df1b2..9625a9eb6d 100644 --- a/osu.Game/Skinning/ISkinnableDrawable.cs +++ b/osu.Game/Skinning/ISkinnableDrawable.cs @@ -17,8 +17,8 @@ namespace osu.Game.Skinning /// /// if this 's is automatically determined by proximity, - /// if the user has overridden it. + /// if the user has chosen a fixed anchor point. /// - bool OverridesClosestAnchor { get; set; } + bool UsesFixedAnchor { get; set; } } } diff --git a/osu.Game/Skinning/LegacyAccuracyCounter.cs b/osu.Game/Skinning/LegacyAccuracyCounter.cs index 603f0ecffb..fd5a9500d9 100644 --- a/osu.Game/Skinning/LegacyAccuracyCounter.cs +++ b/osu.Game/Skinning/LegacyAccuracyCounter.cs @@ -12,7 +12,7 @@ namespace osu.Game.Skinning { public class LegacyAccuracyCounter : GameplayAccuracyCounter, ISkinnableDrawable { - public bool OverridesClosestAnchor { get; set; } + public bool UsesFixedAnchor { get; set; } public LegacyAccuracyCounter() { diff --git a/osu.Game/Skinning/LegacyHealthDisplay.cs b/osu.Game/Skinning/LegacyHealthDisplay.cs index a419f981b5..c8df99762b 100644 --- a/osu.Game/Skinning/LegacyHealthDisplay.cs +++ b/osu.Game/Skinning/LegacyHealthDisplay.cs @@ -30,7 +30,7 @@ namespace osu.Game.Skinning private bool isNewStyle; - public bool OverridesClosestAnchor { get; set; } + public bool UsesFixedAnchor { get; set; } [BackgroundDependencyLoader] private void load() diff --git a/osu.Game/Skinning/LegacyScoreCounter.cs b/osu.Game/Skinning/LegacyScoreCounter.cs index 206e10943d..a12defe87e 100644 --- a/osu.Game/Skinning/LegacyScoreCounter.cs +++ b/osu.Game/Skinning/LegacyScoreCounter.cs @@ -13,7 +13,7 @@ namespace osu.Game.Skinning protected override double RollingDuration => 1000; protected override Easing RollingEasing => Easing.Out; - public bool OverridesClosestAnchor { get; set; } + public bool UsesFixedAnchor { get; set; } public LegacyScoreCounter() : base(6) diff --git a/osu.Game/Skinning/SkinnableTargetComponentsContainer.cs b/osu.Game/Skinning/SkinnableTargetComponentsContainer.cs index b661a02ca5..67114de948 100644 --- a/osu.Game/Skinning/SkinnableTargetComponentsContainer.cs +++ b/osu.Game/Skinning/SkinnableTargetComponentsContainer.cs @@ -17,7 +17,7 @@ namespace osu.Game.Skinning { public bool IsEditable => false; - public bool OverridesClosestAnchor { get; set; } + public bool UsesFixedAnchor { get; set; } private readonly Action applyDefaults; From 6b127f50f21afb89a5bbc2ff8f5364a4bd1f9763 Mon Sep 17 00:00:00 2001 From: Robin Avery Date: Tue, 8 Jun 2021 09:14:04 -0400 Subject: [PATCH 16/39] Inline updateDrawableAnchorIfUsingClosest --- .../Skinning/Editor/SkinSelectionHandler.cs | 21 +++++++------------ 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs index 78173bfda1..7c904d5007 100644 --- a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs +++ b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs @@ -152,25 +152,18 @@ namespace osu.Game.Skinning.Editor Drawable drawable = (Drawable)c.Item; drawable.Position += drawable.ScreenSpaceDeltaToParentSpace(moveEvent.ScreenSpaceDelta); - updateDrawableAnchorIfUsingClosest(c.Item); + if (c.Item.UsesFixedAnchor) continue; + + var closestAnchor = getClosestAnchorForDrawable(drawable); + + if (closestAnchor == drawable.Anchor) continue; + + updateDrawableAnchor(drawable, closestAnchor); } return true; } - private void updateDrawableAnchorIfUsingClosest(ISkinnableDrawable item) - { - if (item.UsesFixedAnchor) return; - - var drawable = (Drawable)item; - - var closestAnchor = getClosestAnchorForDrawable(drawable); - - if (closestAnchor == drawable.Anchor) return; - - updateDrawableAnchor(drawable, closestAnchor); - } - protected override void OnSelectionChanged() { base.OnSelectionChanged(); From 01da73daf211047392b85717621dba921d09d7b5 Mon Sep 17 00:00:00 2001 From: Robin Avery Date: Tue, 8 Jun 2021 09:25:49 -0400 Subject: [PATCH 17/39] Refactor `updateDrawableAnchorIfUsingClosest` --- .../Skinning/Editor/SkinSelectionHandler.cs | 28 +++++++++++-------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs index 7c904d5007..4aa6e79fb8 100644 --- a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs +++ b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs @@ -149,21 +149,29 @@ namespace osu.Game.Skinning.Editor { foreach (var c in SelectedBlueprints) { - Drawable drawable = (Drawable)c.Item; + var skinnableDrawable = c.Item; + Drawable drawable = (Drawable)skinnableDrawable; drawable.Position += drawable.ScreenSpaceDeltaToParentSpace(moveEvent.ScreenSpaceDelta); - if (c.Item.UsesFixedAnchor) continue; - - var closestAnchor = getClosestAnchorForDrawable(drawable); - - if (closestAnchor == drawable.Anchor) continue; - - updateDrawableAnchor(drawable, closestAnchor); + checkAndApplyClosestAnchor(skinnableDrawable); } return true; } + private static void checkAndApplyClosestAnchor(ISkinnableDrawable item) + { + if (item.UsesFixedAnchor) return; + + var drawable = (Drawable)item; + + var closestAnchor = getClosestAnchorForDrawable(drawable); + + if (closestAnchor == drawable.Anchor) return; + + updateDrawableAnchor(drawable, closestAnchor); + } + protected override void OnSelectionChanged() { base.OnSelectionChanged(); @@ -263,10 +271,8 @@ namespace osu.Game.Skinning.Editor { foreach (var item in SelectedItems) { - var drawable = (Drawable)item; - item.UsesFixedAnchor = false; - updateDrawableAnchor(drawable, getClosestAnchorForDrawable(drawable)); + checkAndApplyClosestAnchor(item); } } From 529a80871b3faaae3ae04a8787a7756d1287c48f Mon Sep 17 00:00:00 2001 From: Robin Avery Date: Tue, 8 Jun 2021 09:44:42 -0400 Subject: [PATCH 18/39] Rename some methods for clarity Methods which operate on a collection of `ISkinnableDrawable`s are now plural; ones which take a single item are singular. This also allows cutting down the name of `getClosestAnchorForDrawable` to just `getClosestAnchor`. --- .../Skinning/Editor/SkinSelectionHandler.cs | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs index 4aa6e79fb8..206f39da5c 100644 --- a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs +++ b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs @@ -165,11 +165,11 @@ namespace osu.Game.Skinning.Editor var drawable = (Drawable)item; - var closestAnchor = getClosestAnchorForDrawable(drawable); + var closestAnchor = getClosestAnchor(drawable); if (closestAnchor == drawable.Anchor) return; - updateDrawableAnchor(drawable, closestAnchor); + applyAnchor(drawable, closestAnchor); } protected override void OnSelectionChanged() @@ -187,21 +187,21 @@ namespace osu.Game.Skinning.Editor protected override IEnumerable GetContextMenuItemsForSelection(IEnumerable> selection) { - var closestItem = new TernaryStateRadioMenuItem("Closest", MenuItemType.Standard, _ => applyClosestAnchor()) + var closestItem = new TernaryStateRadioMenuItem("Closest", MenuItemType.Standard, _ => applyClosestAnchors()) { State = { Value = GetStateFromSelection(selection, c => !c.Item.UsesFixedAnchor) } }; yield return new OsuMenuItem("Anchor") { - Items = createAnchorItems((i, a) => i.UsesFixedAnchor && ((Drawable)i).Anchor == a, applyCustomAnchor) + Items = createAnchorItems((i, a) => i.UsesFixedAnchor && ((Drawable)i).Anchor == a, applyCustomAnchors) .Prepend(closestItem) .ToArray() }; yield return new OsuMenuItem("Origin") { - Items = createAnchorItems((i, o) => ((Drawable)i).Origin == o, applyOrigin).ToArray() + Items = createAnchorItems((i, o) => ((Drawable)i).Origin == o, applyOrigins).ToArray() }; foreach (var item in base.GetContextMenuItemsForSelection(selection)) @@ -237,7 +237,7 @@ namespace osu.Game.Skinning.Editor drawable.Parent.ToLocalSpace(screenSpacePosition) - drawable.AnchorPosition; } - private void applyOrigin(Anchor anchor) + private void applyOrigins(Anchor anchor) { foreach (var item in SelectedItems) { @@ -256,18 +256,18 @@ namespace osu.Game.Skinning.Editor private Quad getSelectionQuad() => GetSurroundingQuad(SelectedBlueprints.SelectMany(b => b.Item.ScreenSpaceDrawQuad.GetVertices().ToArray())); - private void applyCustomAnchor(Anchor anchor) + private void applyCustomAnchors(Anchor anchor) { foreach (var item in SelectedItems) { var drawable = (Drawable)item; item.UsesFixedAnchor = true; - updateDrawableAnchor(drawable, anchor); + applyAnchor(drawable, anchor); } } - private void applyClosestAnchor() + private void applyClosestAnchors() { foreach (var item in SelectedItems) { @@ -276,7 +276,7 @@ namespace osu.Game.Skinning.Editor } } - private static Anchor getClosestAnchorForDrawable(Drawable drawable) + private static Anchor getClosestAnchor(Drawable drawable) { var parent = drawable.Parent; @@ -323,7 +323,7 @@ namespace osu.Game.Skinning.Editor return tier0; } - private static void updateDrawableAnchor(Drawable drawable, Anchor anchor) + private static void applyAnchor(Drawable drawable, Anchor anchor) { var previousAnchor = drawable.AnchorPosition; drawable.Anchor = anchor; From f22cc981d15554665b54bf72956f7397eb7c42f6 Mon Sep 17 00:00:00 2001 From: Robin Avery Date: Tue, 8 Jun 2021 09:51:39 -0400 Subject: [PATCH 19/39] Move guard clause from `checkAndApplyClosestAnchor` to `applyAnchor` --- osu.Game/Skinning/Editor/SkinSelectionHandler.cs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs index 206f39da5c..a9b985fda2 100644 --- a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs +++ b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs @@ -165,11 +165,7 @@ namespace osu.Game.Skinning.Editor var drawable = (Drawable)item; - var closestAnchor = getClosestAnchor(drawable); - - if (closestAnchor == drawable.Anchor) return; - - applyAnchor(drawable, closestAnchor); + applyAnchor(drawable, getClosestAnchor(drawable)); } protected override void OnSelectionChanged() @@ -325,6 +321,8 @@ namespace osu.Game.Skinning.Editor private static void applyAnchor(Drawable drawable, Anchor anchor) { + if (anchor == drawable.Anchor) return; + var previousAnchor = drawable.AnchorPosition; drawable.Anchor = anchor; drawable.Position -= drawable.AnchorPosition - previousAnchor; From 2c88e6df8de34ac1bc5e637bc4d1564d48699dd1 Mon Sep 17 00:00:00 2001 From: Robin Avery Date: Tue, 8 Jun 2021 10:08:02 -0400 Subject: [PATCH 20/39] Simplify `applyClosestAnchor` to one line by moving another guard clause --- .../Skinning/Editor/SkinSelectionHandler.cs | 20 ++++++++----------- 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs index a9b985fda2..500020ca46 100644 --- a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs +++ b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs @@ -149,24 +149,20 @@ namespace osu.Game.Skinning.Editor { foreach (var c in SelectedBlueprints) { - var skinnableDrawable = c.Item; - Drawable drawable = (Drawable)skinnableDrawable; + var item = c.Item; + Drawable drawable = (Drawable)item; + drawable.Position += drawable.ScreenSpaceDeltaToParentSpace(moveEvent.ScreenSpaceDelta); - checkAndApplyClosestAnchor(skinnableDrawable); + if (item.UsesFixedAnchor) continue; + + applyClosestAnchor(drawable); } return true; } - private static void checkAndApplyClosestAnchor(ISkinnableDrawable item) - { - if (item.UsesFixedAnchor) return; - - var drawable = (Drawable)item; - - applyAnchor(drawable, getClosestAnchor(drawable)); - } + private static void applyClosestAnchor(Drawable drawable) => applyAnchor(drawable, getClosestAnchor(drawable)); protected override void OnSelectionChanged() { @@ -268,7 +264,7 @@ namespace osu.Game.Skinning.Editor foreach (var item in SelectedItems) { item.UsesFixedAnchor = false; - checkAndApplyClosestAnchor(item); + applyClosestAnchor((Drawable)item); } } From d212918d67059ecd111155f93c039cf50435a2a8 Mon Sep 17 00:00:00 2001 From: Robin Avery Date: Tue, 8 Jun 2021 10:14:07 -0400 Subject: [PATCH 21/39] Rename `applyCustomAnchors` to `applyFixedAnchors` for consistency with `UsesFixedAnchor` --- osu.Game/Skinning/Editor/SkinSelectionHandler.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs index 500020ca46..e52b1c68d4 100644 --- a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs +++ b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs @@ -186,7 +186,7 @@ namespace osu.Game.Skinning.Editor yield return new OsuMenuItem("Anchor") { - Items = createAnchorItems((i, a) => i.UsesFixedAnchor && ((Drawable)i).Anchor == a, applyCustomAnchors) + Items = createAnchorItems((i, a) => i.UsesFixedAnchor && ((Drawable)i).Anchor == a, applyFixedAnchors) .Prepend(closestItem) .ToArray() }; @@ -248,7 +248,7 @@ namespace osu.Game.Skinning.Editor private Quad getSelectionQuad() => GetSurroundingQuad(SelectedBlueprints.SelectMany(b => b.Item.ScreenSpaceDrawQuad.GetVertices().ToArray())); - private void applyCustomAnchors(Anchor anchor) + private void applyFixedAnchors(Anchor anchor) { foreach (var item in SelectedItems) { From 10b6b7290923a112dfee58c8a8c820dc32934611 Mon Sep 17 00:00:00 2001 From: Robin Avery Date: Tue, 8 Jun 2021 10:29:11 -0400 Subject: [PATCH 22/39] Add guard clause to `applyOrigins` and rename parameter --- osu.Game/Skinning/Editor/SkinSelectionHandler.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs index e52b1c68d4..ccfb0cb6e0 100644 --- a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs +++ b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs @@ -229,14 +229,16 @@ namespace osu.Game.Skinning.Editor drawable.Parent.ToLocalSpace(screenSpacePosition) - drawable.AnchorPosition; } - private void applyOrigins(Anchor anchor) + private void applyOrigins(Anchor origin) { foreach (var item in SelectedItems) { var drawable = (Drawable)item; + if (origin == drawable.Origin) continue; + var previousOrigin = drawable.OriginPosition; - drawable.Origin = anchor; + drawable.Origin = origin; drawable.Position += drawable.OriginPosition - previousOrigin; } } From a506f2a77699f955cd5b985f2ea4009810f76493 Mon Sep 17 00:00:00 2001 From: Robin Avery Date: Fri, 11 Jun 2021 06:22:24 -0400 Subject: [PATCH 23/39] Revert rename of lambda variables --- osu.Game/Skinning/Editor/SkinSelectionHandler.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs index ccfb0cb6e0..de9a7273c5 100644 --- a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs +++ b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs @@ -186,14 +186,14 @@ namespace osu.Game.Skinning.Editor yield return new OsuMenuItem("Anchor") { - Items = createAnchorItems((i, a) => i.UsesFixedAnchor && ((Drawable)i).Anchor == a, applyFixedAnchors) + Items = createAnchorItems((d, a) => d.UsesFixedAnchor && ((Drawable)d).Anchor == a, applyFixedAnchors) .Prepend(closestItem) .ToArray() }; yield return new OsuMenuItem("Origin") { - Items = createAnchorItems((i, o) => ((Drawable)i).Origin == o, applyOrigins).ToArray() + Items = createAnchorItems((d, o) => ((Drawable)d).Origin == o, applyOrigins).ToArray() }; foreach (var item in base.GetContextMenuItemsForSelection(selection)) From 635300b3115f721188ca3ab6593b5af0cc99c921 Mon Sep 17 00:00:00 2001 From: Robin Avery Date: Fri, 11 Jun 2021 06:28:30 -0400 Subject: [PATCH 24/39] Recalculate closest anchor when origin is changed --- osu.Game/Skinning/Editor/SkinSelectionHandler.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs index de9a7273c5..dedab1f5f5 100644 --- a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs +++ b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs @@ -240,6 +240,10 @@ namespace osu.Game.Skinning.Editor var previousOrigin = drawable.OriginPosition; drawable.Origin = origin; drawable.Position += drawable.OriginPosition - previousOrigin; + + if (item.UsesFixedAnchor) continue; + + applyClosestAnchor(drawable); } } From a76eaeb52d7fccc6ff36fbd7727461392df8e9ff Mon Sep 17 00:00:00 2001 From: Robin Avery Date: Fri, 11 Jun 2021 06:51:12 -0400 Subject: [PATCH 25/39] Make `getTieredComponent` local --- .../Skinning/Editor/SkinSelectionHandler.cs | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs index dedab1f5f5..c2db79b9ef 100644 --- a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs +++ b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs @@ -287,6 +287,17 @@ namespace osu.Game.Skinning.Editor var result = default(Anchor); + static Anchor getTieredComponent(float component, Anchor tier0, Anchor tier1, Anchor tier2) + { + if (component >= 2 / 3f) + return tier2; + + if (component >= 1 / 3f) + return tier1; + + return tier0; + } + result |= getTieredComponent(absolutePosition.X / factor.X, Anchor.x0, Anchor.x1, Anchor.x2); result |= getTieredComponent(absolutePosition.Y / factor.Y, Anchor.y0, Anchor.y1, Anchor.y2); @@ -310,17 +321,6 @@ namespace osu.Game.Skinning.Editor return result; } - private static Anchor getTieredComponent(float component, Anchor tier0, Anchor tier1, Anchor tier2) - { - if (component >= 2 / 3f) - return tier2; - - if (component >= 1 / 3f) - return tier1; - - return tier0; - } - private static void applyAnchor(Drawable drawable, Anchor anchor) { if (anchor == drawable.Anchor) return; From 6e181a6b6327658e4139cefc2d723eaf42e930c2 Mon Sep 17 00:00:00 2001 From: Robin Avery Date: Fri, 11 Jun 2021 06:53:04 -0400 Subject: [PATCH 26/39] Rename parameters of `getTieredComponent` --- osu.Game/Skinning/Editor/SkinSelectionHandler.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs index c2db79b9ef..048ba6556b 100644 --- a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs +++ b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs @@ -287,15 +287,15 @@ namespace osu.Game.Skinning.Editor var result = default(Anchor); - static Anchor getTieredComponent(float component, Anchor tier0, Anchor tier1, Anchor tier2) + static Anchor getTieredComponent(float component, Anchor anchor0, Anchor anchor1, Anchor anchor2) { if (component >= 2 / 3f) - return tier2; + return anchor2; if (component >= 1 / 3f) - return tier1; + return anchor1; - return tier0; + return anchor0; } result |= getTieredComponent(absolutePosition.X / factor.X, Anchor.x0, Anchor.x1, Anchor.x2); From 1bc8460902a1e230ef2015717d9ac92fb8a1878f Mon Sep 17 00:00:00 2001 From: Robin Avery Date: Fri, 11 Jun 2021 06:53:40 -0400 Subject: [PATCH 27/39] Rename `getTieredComponent` to `getAnchorFromPosition` Also rename parameter `component` to `xOrY`. --- osu.Game/Skinning/Editor/SkinSelectionHandler.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs index 048ba6556b..295b1377ed 100644 --- a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs +++ b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs @@ -287,19 +287,19 @@ namespace osu.Game.Skinning.Editor var result = default(Anchor); - static Anchor getTieredComponent(float component, Anchor anchor0, Anchor anchor1, Anchor anchor2) + static Anchor getAnchorFromPosition(float xOrY, Anchor anchor0, Anchor anchor1, Anchor anchor2) { - if (component >= 2 / 3f) + if (xOrY >= 2 / 3f) return anchor2; - if (component >= 1 / 3f) + if (xOrY >= 1 / 3f) return anchor1; return anchor0; } - result |= getTieredComponent(absolutePosition.X / factor.X, Anchor.x0, Anchor.x1, Anchor.x2); - result |= getTieredComponent(absolutePosition.Y / factor.Y, Anchor.y0, Anchor.y1, Anchor.y2); + result |= getAnchorFromPosition(absolutePosition.X / factor.X, Anchor.x0, Anchor.x1, Anchor.x2); + result |= getAnchorFromPosition(absolutePosition.Y / factor.Y, Anchor.y0, Anchor.y1, Anchor.y2); return result; } From c9b4f9eb717e9d7d36a9a3d8e502216bac529296 Mon Sep 17 00:00:00 2001 From: Robin Avery Date: Fri, 11 Jun 2021 06:55:47 -0400 Subject: [PATCH 28/39] Make `getOriginPositionFromQuad` local --- .../Skinning/Editor/SkinSelectionHandler.cs | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs index 295b1377ed..cd9e82997c 100644 --- a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs +++ b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs @@ -281,6 +281,23 @@ namespace osu.Game.Skinning.Editor if (parent == null) return drawable.Anchor; + static Vector2 getOriginPositionFromQuad(in Quad quad, Anchor origin) + { + var result = quad.TopLeft; + + if (origin.HasFlagFast(Anchor.x2)) + result.X += quad.Width; + else if (origin.HasFlagFast(Anchor.x1)) + result.X += quad.Width / 2f; + + if (origin.HasFlagFast(Anchor.y2)) + result.Y += quad.Height; + else if (origin.HasFlagFast(Anchor.y1)) + result.Y += quad.Height / 2f; + + return result; + } + var screenPosition = getOriginPositionFromQuad(drawable.ScreenSpaceDrawQuad, drawable.Origin); var absolutePosition = parent.ToLocalSpace(screenPosition); var factor = parent.RelativeToAbsoluteFactor; @@ -304,23 +321,6 @@ namespace osu.Game.Skinning.Editor return result; } - private static Vector2 getOriginPositionFromQuad(in Quad quad, Anchor origin) - { - var result = quad.TopLeft; - - if (origin.HasFlagFast(Anchor.x2)) - result.X += quad.Width; - else if (origin.HasFlagFast(Anchor.x1)) - result.X += quad.Width / 2f; - - if (origin.HasFlagFast(Anchor.y2)) - result.Y += quad.Height; - else if (origin.HasFlagFast(Anchor.y1)) - result.Y += quad.Height / 2f; - - return result; - } - private static void applyAnchor(Drawable drawable, Anchor anchor) { if (anchor == drawable.Anchor) return; From a6774eb5b52a9f0272da5b9de51644fee4e44710 Mon Sep 17 00:00:00 2001 From: Robin Avery Date: Fri, 11 Jun 2021 06:59:00 -0400 Subject: [PATCH 29/39] Inline `getOriginPositionFromQuad` --- osu.Game/Skinning/Editor/SkinSelectionHandler.cs | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs index cd9e82997c..8e1f0ce7a3 100644 --- a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs +++ b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs @@ -281,24 +281,21 @@ namespace osu.Game.Skinning.Editor if (parent == null) return drawable.Anchor; - static Vector2 getOriginPositionFromQuad(in Quad quad, Anchor origin) + var screenPosition = drawable.ScreenSpaceDrawQuad.TopLeft; { - var result = quad.TopLeft; + var origin = drawable.Origin; if (origin.HasFlagFast(Anchor.x2)) - result.X += quad.Width; + screenPosition.X += drawable.ScreenSpaceDrawQuad.Width; else if (origin.HasFlagFast(Anchor.x1)) - result.X += quad.Width / 2f; + screenPosition.X += drawable.ScreenSpaceDrawQuad.Width / 2f; if (origin.HasFlagFast(Anchor.y2)) - result.Y += quad.Height; + screenPosition.Y += drawable.ScreenSpaceDrawQuad.Height; else if (origin.HasFlagFast(Anchor.y1)) - result.Y += quad.Height / 2f; - - return result; + screenPosition.Y += drawable.ScreenSpaceDrawQuad.Height / 2f; } - var screenPosition = getOriginPositionFromQuad(drawable.ScreenSpaceDrawQuad, drawable.Origin); var absolutePosition = parent.ToLocalSpace(screenPosition); var factor = parent.RelativeToAbsoluteFactor; From 0c8851f4b7a39ff14136cea8fdcb30f4b60a88a9 Mon Sep 17 00:00:00 2001 From: Robin Avery Date: Fri, 11 Jun 2021 07:06:22 -0400 Subject: [PATCH 30/39] Extract `drawable.ScreenSpaceDrawQuad` to a variable --- osu.Game/Skinning/Editor/SkinSelectionHandler.cs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs index 8e1f0ce7a3..c2ad08f0dc 100644 --- a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs +++ b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs @@ -281,19 +281,22 @@ namespace osu.Game.Skinning.Editor if (parent == null) return drawable.Anchor; - var screenPosition = drawable.ScreenSpaceDrawQuad.TopLeft; + Vector2 screenPosition; { + var quad = drawable.ScreenSpaceDrawQuad; var origin = drawable.Origin; + screenPosition = quad.TopLeft; + if (origin.HasFlagFast(Anchor.x2)) - screenPosition.X += drawable.ScreenSpaceDrawQuad.Width; + screenPosition.X += quad.Width; else if (origin.HasFlagFast(Anchor.x1)) - screenPosition.X += drawable.ScreenSpaceDrawQuad.Width / 2f; + screenPosition.X += quad.Width / 2f; if (origin.HasFlagFast(Anchor.y2)) - screenPosition.Y += drawable.ScreenSpaceDrawQuad.Height; + screenPosition.Y += quad.Height; else if (origin.HasFlagFast(Anchor.y1)) - screenPosition.Y += drawable.ScreenSpaceDrawQuad.Height / 2f; + screenPosition.Y += quad.Height / 2f; } var absolutePosition = parent.ToLocalSpace(screenPosition); From 3eb088f89a2fd3cc3b6490574a6a6963715ecc3f Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Mon, 21 Jun 2021 15:30:04 +0200 Subject: [PATCH 31/39] Add low difficulty overlaps check --- .../Edit/Checks/CheckLowDiffOverlaps.cs | 107 ++++++++++++++++++ 1 file changed, 107 insertions(+) create mode 100644 osu.Game.Rulesets.Osu/Edit/Checks/CheckLowDiffOverlaps.cs diff --git a/osu.Game.Rulesets.Osu/Edit/Checks/CheckLowDiffOverlaps.cs b/osu.Game.Rulesets.Osu/Edit/Checks/CheckLowDiffOverlaps.cs new file mode 100644 index 0000000000..488bdfd972 --- /dev/null +++ b/osu.Game.Rulesets.Osu/Edit/Checks/CheckLowDiffOverlaps.cs @@ -0,0 +1,107 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System.Collections.Generic; +using osu.Game.Beatmaps; +using osu.Game.Rulesets.Edit; +using osu.Game.Rulesets.Edit.Checks.Components; +using osu.Game.Rulesets.Objects; +using osu.Game.Rulesets.Osu.Objects; + +namespace osu.Game.Rulesets.Osu.Edit.Checks +{ + public class CheckLowDiffOverlaps : ICheck + { + // For the lowest difficulties, the osu! Ranking Criteria encourages overlapping ~180 BPM 1/2, but discourages ~180 BPM 1/1. + private const double should_overlap_threshold = 150; // 200 BPM 1/2 + private const double should_probably_overlap_threshold = 175; // 170 BPM 1/2 + private const double should_not_overlap_threshold = 250; // 120 BPM 1/2 = 240 BPM 1/1 + + // Objects need to overlap this much before being treated as an overlap, else it may just be the borders slightly touching. + private const double overlap_leniency = 5; + + public CheckMetadata Metadata { get; } = new CheckMetadata(CheckCategory.Spread, "Missing or unexpected overlaps"); + + public IEnumerable PossibleTemplates => new IssueTemplate[] + { + new IssueTemplateShouldOverlap(this), + new IssueTemplateShouldProbablyOverlap(this), + new IssueTemplateShouldNotOverlap(this) + }; + + public IEnumerable Run(BeatmapVerifierContext context) + { + // TODO: This should also apply to *lowest difficulty* Normals - they are skipped for now. + if (context.InterpretedDifficulty > DifficultyRating.Easy) + yield break; + + var hitObjects = context.Beatmap.HitObjects; + + for (int i = 0; i < hitObjects.Count - 1; ++i) + { + if (!(hitObjects[i] is OsuHitObject hitObject) || hitObject is Spinner) + continue; + + if (!(hitObjects[i + 1] is OsuHitObject nextHitObject) || nextHitObject is Spinner) + continue; + + var deltaTime = nextHitObject.StartTime - hitObject.GetEndTime(); + if (deltaTime >= hitObject.TimeFadeIn + hitObject.TimePreempt) + // The objects are not visible at the same time (without mods), hence skipping. + continue; + + var distanceSq = (hitObject.StackedEndPosition - nextHitObject.StackedPosition).LengthSquared; + var diameter = (hitObject.Radius - overlap_leniency) * 2; + var diameterSq = diameter * diameter; + + bool areOverlapping = distanceSq < diameterSq; + + // Slider ends do not need to be overlapped because of slider leniency. + if (!areOverlapping && !(hitObject is Slider)) + { + if (deltaTime < should_overlap_threshold) + yield return new IssueTemplateShouldOverlap(this).Create(deltaTime, hitObject, nextHitObject); + else if (deltaTime < should_probably_overlap_threshold) + yield return new IssueTemplateShouldProbablyOverlap(this).Create(deltaTime, hitObject, nextHitObject); + } + + if (areOverlapping && deltaTime > should_not_overlap_threshold) + yield return new IssueTemplateShouldNotOverlap(this).Create(deltaTime, hitObject, nextHitObject); + } + } + + public abstract class IssueTemplateOverlap : IssueTemplate + { + protected IssueTemplateOverlap(ICheck check, IssueType issueType, string unformattedMessage) + : base(check, issueType, unformattedMessage) + { + } + + public Issue Create(double deltaTime, params HitObject[] hitObjects) => new Issue(hitObjects, this, deltaTime); + } + + public class IssueTemplateShouldOverlap : IssueTemplateOverlap + { + public IssueTemplateShouldOverlap(ICheck check) + : base(check, IssueType.Problem, "These are {0} ms apart and so should be overlapping.") + { + } + } + + public class IssueTemplateShouldProbablyOverlap : IssueTemplateOverlap + { + public IssueTemplateShouldProbablyOverlap(ICheck check) + : base(check, IssueType.Warning, "These are {0} ms apart and so should probably be overlapping.") + { + } + } + + public class IssueTemplateShouldNotOverlap : IssueTemplateOverlap + { + public IssueTemplateShouldNotOverlap(ICheck check) + : base(check, IssueType.Problem, "These are {0} ms apart and so should NOT be overlapping.") + { + } + } + } +} From fcb918d0e11868c7bfd3954939b2143165bf4e45 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Mon, 21 Jun 2021 15:30:23 +0200 Subject: [PATCH 32/39] Add time distance equality check --- .../Edit/Checks/CheckTimeDistanceEquality.cs | 157 ++++++++++++++++++ 1 file changed, 157 insertions(+) create mode 100644 osu.Game.Rulesets.Osu/Edit/Checks/CheckTimeDistanceEquality.cs diff --git a/osu.Game.Rulesets.Osu/Edit/Checks/CheckTimeDistanceEquality.cs b/osu.Game.Rulesets.Osu/Edit/Checks/CheckTimeDistanceEquality.cs new file mode 100644 index 0000000000..db48878dd3 --- /dev/null +++ b/osu.Game.Rulesets.Osu/Edit/Checks/CheckTimeDistanceEquality.cs @@ -0,0 +1,157 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System; +using System.Collections.Generic; +using System.Linq; +using osu.Game.Beatmaps; +using osu.Game.Rulesets.Edit; +using osu.Game.Rulesets.Edit.Checks.Components; +using osu.Game.Rulesets.Objects; +using osu.Game.Rulesets.Osu.Objects; + +namespace osu.Game.Rulesets.Osu.Edit.Checks +{ + public class CheckTimeDistanceEquality : ICheck + { + private const double pattern_lifetime = 600; // Two objects this many ms apart or more are skipped. (200 BPM 2/1) + private const double stack_leniency = 12; // Two objects this distance apart or less are skipped. + + private const double observation_lifetime = 4000; // How long an observation is relevant for comparison. (120 BPM 8/1) + private const double similar_time_leniency = 16; // How different two delta times can be to still be compared. (240 BPM 1/16) + + private const double distance_leniency_absolute_warning = 10; // How many pixels are subtracted from the difference between current and expected distance. + private const double distance_leniency_percent_warning = 0.15; // How much of the current distance that the difference can make out. + private const double distance_leniency_absolute_problem = 20; + private const double distance_leniency_percent_problem = 0.3; + + public CheckMetadata Metadata { get; } = new CheckMetadata(CheckCategory.Spread, "Object too close or far away from previous"); + + public IEnumerable PossibleTemplates => new IssueTemplate[] + { + new IssueTemplateIrregularSpacingProblem(this), + new IssueTemplateIrregularSpacingWarning(this) + }; + + /// + /// Represents an observation of the time and distance between two objects. + /// + private readonly struct ObservedTimeDistance + { + public readonly double ObservationTime; + public readonly double DeltaTime; + public readonly double Distance; + + public ObservedTimeDistance(double observationTime, double deltaTime, double distance) + { + ObservationTime = observationTime; + DeltaTime = deltaTime; + Distance = distance; + } + } + + public IEnumerable Run(BeatmapVerifierContext context) + { + if (context.InterpretedDifficulty > DifficultyRating.Normal) + yield break; + + var prevObservedTimeDistances = new List(); + var hitObjects = context.Beatmap.HitObjects; + + for (int i = 0; i < hitObjects.Count - 1; ++i) + { + if (!(hitObjects[i] is OsuHitObject hitObject) || hitObject is Spinner) + continue; + + if (!(hitObjects[i + 1] is OsuHitObject nextHitObject) || nextHitObject is Spinner) + continue; + + var deltaTime = nextHitObject.StartTime - hitObject.GetEndTime(); + + // Ignore objects that are far enough apart in time to not be considered the same pattern. + if (deltaTime > pattern_lifetime) + continue; + + // Relying on FastInvSqrt is probably good enough here. We'll be taking the difference between distances later, hence square not being sufficient. + var distance = (hitObject.StackedEndPosition - nextHitObject.StackedPosition).LengthFast; + + // Ignore stacks and half-stacks, as these are close enough to where they can't be confused for being time-distanced. + if (distance < stack_leniency) + continue; + + var observedTimeDistance = new ObservedTimeDistance(nextHitObject.StartTime, deltaTime, distance); + var expectedDistance = getExpectedDistance(prevObservedTimeDistances, observedTimeDistance); + + if (expectedDistance == 0) + { + // There was nothing relevant to compare to. + prevObservedTimeDistances.Add(observedTimeDistance); + continue; + } + + if ((Math.Abs(expectedDistance - distance) - distance_leniency_absolute_problem) / distance > distance_leniency_percent_problem) + yield return new IssueTemplateIrregularSpacingProblem(this).Create(expectedDistance, distance, hitObject, nextHitObject); + else if ((Math.Abs(expectedDistance - distance) - distance_leniency_absolute_warning) / distance > distance_leniency_percent_warning) + yield return new IssueTemplateIrregularSpacingWarning(this).Create(expectedDistance, distance, hitObject, nextHitObject); + else + { + // We use `else` here to prevent issues from cascading; an object spaced too far could cause regular spacing to be considered "too short" otherwise. + prevObservedTimeDistances.Add(observedTimeDistance); + } + } + } + + private double getExpectedDistance(IEnumerable prevObservedTimeDistances, ObservedTimeDistance observedTimeDistance) + { + var observations = prevObservedTimeDistances.Count(); + + int count = 0; + double sum = 0; + + // Looping this in reverse allows us to break before going through all elements, as we're only interested in the most recent ones. + for (int i = observations - 1; i >= 0; --i) + { + var prevObservedTimeDistance = prevObservedTimeDistances.ElementAt(i); + + // Only consider observations within the last few seconds - this allows the map to build spacing up/down over time, but prevents it from being too sudden. + if (observedTimeDistance.ObservationTime - prevObservedTimeDistance.ObservationTime > observation_lifetime) + break; + + // Only consider observations which have a similar time difference - this leniency allows handling of multi-BPM maps which speed up/down slowly. + if (Math.Abs(observedTimeDistance.DeltaTime - prevObservedTimeDistance.DeltaTime) > similar_time_leniency) + break; + + count += 1; + sum += prevObservedTimeDistance.Distance / Math.Max(prevObservedTimeDistance.DeltaTime, 1); + } + + return sum / Math.Max(count, 1) * observedTimeDistance.DeltaTime; + } + + public abstract class IssueTemplateIrregularSpacing : IssueTemplate + { + protected IssueTemplateIrregularSpacing(ICheck check, IssueType issueType) + : base(check, issueType, "Expected {0:0} px spacing like previous objects, currently {1:0}.") + { + } + + public Issue Create(double expected, double actual, params HitObject[] hitObjects) => new Issue(hitObjects, this, expected, actual); + } + + public class IssueTemplateIrregularSpacingProblem : IssueTemplateIrregularSpacing + { + public IssueTemplateIrregularSpacingProblem(ICheck check) + : base(check, IssueType.Problem) + { + } + } + + public class IssueTemplateIrregularSpacingWarning : IssueTemplateIrregularSpacing + { + public IssueTemplateIrregularSpacingWarning(ICheck check) + : base(check, IssueType.Warning) + { + } + } + } +} From 2f3f4f3e4b8b660b05d6652a9b1261f09901bcea Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Mon, 21 Jun 2021 15:30:45 +0200 Subject: [PATCH 33/39] Add new checks to verifier --- osu.Game.Rulesets.Osu/Edit/OsuBeatmapVerifier.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/osu.Game.Rulesets.Osu/Edit/OsuBeatmapVerifier.cs b/osu.Game.Rulesets.Osu/Edit/OsuBeatmapVerifier.cs index 04e881fbf3..896e904f3f 100644 --- a/osu.Game.Rulesets.Osu/Edit/OsuBeatmapVerifier.cs +++ b/osu.Game.Rulesets.Osu/Edit/OsuBeatmapVerifier.cs @@ -13,7 +13,12 @@ namespace osu.Game.Rulesets.Osu.Edit { private readonly List checks = new List { - new CheckOffscreenObjects() + // Compose + new CheckOffscreenObjects(), + + // Spread + new CheckTimeDistanceEquality(), + new CheckLowDiffOverlaps() }; public IEnumerable Run(BeatmapVerifierContext context) From e11139eadfe1a6d0e09e247abc02915fede9f21f Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Mon, 21 Jun 2021 15:33:50 +0200 Subject: [PATCH 34/39] Add low difficulty overlap tests Moq is introduced to mock sliders' end time/position. This is already used similarly in `osu.Game.Tests`. --- .../Editor/Checks/CheckLowDiffOverlapsTest.cs | 260 ++++++++++++++++++ .../osu.Game.Rulesets.Osu.Tests.csproj | 1 + 2 files changed, 261 insertions(+) create mode 100644 osu.Game.Rulesets.Osu.Tests/Editor/Checks/CheckLowDiffOverlapsTest.cs diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/Checks/CheckLowDiffOverlapsTest.cs b/osu.Game.Rulesets.Osu.Tests/Editor/Checks/CheckLowDiffOverlapsTest.cs new file mode 100644 index 0000000000..fd17d11d10 --- /dev/null +++ b/osu.Game.Rulesets.Osu.Tests/Editor/Checks/CheckLowDiffOverlapsTest.cs @@ -0,0 +1,260 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System.Collections.Generic; +using System.Linq; +using Moq; +using NUnit.Framework; +using osu.Game.Beatmaps; +using osu.Game.Rulesets.Edit; +using osu.Game.Rulesets.Objects; +using osu.Game.Rulesets.Objects.Types; +using osu.Game.Rulesets.Osu.Edit.Checks; +using osu.Game.Rulesets.Osu.Objects; +using osu.Game.Tests.Beatmaps; +using osuTK; + +namespace osu.Game.Rulesets.Osu.Tests.Editor.Checks +{ + [TestFixture] + public class CheckLowDiffOverlapsTest + { + private CheckLowDiffOverlaps check; + + [SetUp] + public void Setup() + { + check = new CheckLowDiffOverlaps(); + } + + [Test] + public void TestNoOverlapFarApart() + { + assertOk(new Beatmap + { + HitObjects = new List + { + new HitCircle { StartTime = 0, Position = new Vector2(0) }, + new HitCircle { StartTime = 500, Position = new Vector2(200, 0) } + } + }); + } + + [Test] + public void TestNoOverlapClose() + { + assertShouldProbablyOverlap(new Beatmap + { + HitObjects = new List + { + new HitCircle { StartTime = 0, Position = new Vector2(0) }, + new HitCircle { StartTime = 167, Position = new Vector2(200, 0) } + } + }); + } + + [Test] + public void TestNoOverlapTooClose() + { + assertShouldOverlap(new Beatmap + { + HitObjects = new List + { + new HitCircle { StartTime = 0, Position = new Vector2(0) }, + new HitCircle { StartTime = 100, Position = new Vector2(200, 0) } + } + }); + } + + [Test] + public void TestNoOverlapTooCloseExpert() + { + assertOk(new Beatmap + { + HitObjects = new List + { + new HitCircle { StartTime = 0, Position = new Vector2(0) }, + new HitCircle { StartTime = 100, Position = new Vector2(200, 0) } + } + }, DifficultyRating.Expert); + } + + [Test] + public void TestOverlapClose() + { + assertOk(new Beatmap + { + HitObjects = new List + { + new HitCircle { StartTime = 0, Position = new Vector2(0) }, + new HitCircle { StartTime = 167, Position = new Vector2(20, 0) } + } + }); + } + + [Test] + public void TestOverlapFarApart() + { + assertShouldNotOverlap(new Beatmap + { + HitObjects = new List + { + new HitCircle { StartTime = 0, Position = new Vector2(0) }, + new HitCircle { StartTime = 500, Position = new Vector2(20, 0) } + } + }); + } + + [Test] + public void TestAlmostOverlapFarApart() + { + assertOk(new Beatmap + { + HitObjects = new List + { + // Default circle diameter is 128 px, but part of that is the fade/border of the circle. + // We want this to only be a problem when it actually looks like an overlap. + new HitCircle { StartTime = 0, Position = new Vector2(0) }, + new HitCircle { StartTime = 500, Position = new Vector2(125, 0) } + } + }); + } + + [Test] + public void TestAlmostNotOverlapFarApart() + { + assertShouldNotOverlap(new Beatmap + { + HitObjects = new List + { + new HitCircle { StartTime = 0, Position = new Vector2(0) }, + new HitCircle { StartTime = 500, Position = new Vector2(110, 0) } + } + }); + } + + [Test] + public void TestOverlapFarApartExpert() + { + assertOk(new Beatmap + { + HitObjects = new List + { + new HitCircle { StartTime = 0, Position = new Vector2(0) }, + new HitCircle { StartTime = 500, Position = new Vector2(20, 0) } + } + }, DifficultyRating.Expert); + } + + [Test] + public void TestOverlapTooFarApart() + { + // Far apart enough to where the objects are not visible at the same time, and so overlapping is fine. + assertOk(new Beatmap + { + HitObjects = new List + { + new HitCircle { StartTime = 0, Position = new Vector2(0) }, + new HitCircle { StartTime = 2000, Position = new Vector2(20, 0) } + } + }); + } + + [Test] + public void TestSliderTailOverlapFarApart() + { + assertShouldNotOverlap(new Beatmap + { + HitObjects = new List + { + getSliderMock(startTime: 0, endTime: 500, startPosition: new Vector2(0, 0), endPosition: new Vector2(100, 0)).Object, + new HitCircle { StartTime = 1000, Position = new Vector2(120, 0) } + } + }); + } + + [Test] + public void TestSliderTailOverlapClose() + { + assertOk(new Beatmap + { + HitObjects = new List + { + getSliderMock(startTime: 0, endTime: 900, startPosition: new Vector2(0, 0), endPosition: new Vector2(100, 0)).Object, + new HitCircle { StartTime = 1000, Position = new Vector2(120, 0) } + } + }); + } + + [Test] + public void TestSliderTailNoOverlapFarApart() + { + assertOk(new Beatmap + { + HitObjects = new List + { + getSliderMock(startTime: 0, endTime: 500, startPosition: new Vector2(0, 0), endPosition: new Vector2(100, 0)).Object, + new HitCircle { StartTime = 1000, Position = new Vector2(300, 0) } + } + }); + } + + [Test] + public void TestSliderTailNoOverlapClose() + { + // If these were circles they would need to overlap, but overlapping with slider tails is not required. + assertOk(new Beatmap + { + HitObjects = new List + { + getSliderMock(startTime: 0, endTime: 900, startPosition: new Vector2(0, 0), endPosition: new Vector2(100, 0)).Object, + new HitCircle { StartTime = 1000, Position = new Vector2(300, 0) } + } + }); + } + + private Mock getSliderMock(double startTime, double endTime, Vector2 startPosition, Vector2 endPosition) + { + var mockSlider = new Mock(); + mockSlider.SetupGet(s => s.StartTime).Returns(startTime); + mockSlider.SetupGet(s => s.Position).Returns(startPosition); + mockSlider.SetupGet(s => s.EndPosition).Returns(endPosition); + mockSlider.As().Setup(d => d.EndTime).Returns(endTime); + + return mockSlider; + } + + private void assertOk(IBeatmap beatmap, DifficultyRating difficultyRating = DifficultyRating.Easy) + { + var context = new BeatmapVerifierContext(beatmap, new TestWorkingBeatmap(beatmap), difficultyRating); + Assert.That(check.Run(context), Is.Empty); + } + + private void assertShouldProbablyOverlap(IBeatmap beatmap, int count = 1) + { + var context = new BeatmapVerifierContext(beatmap, new TestWorkingBeatmap(beatmap), DifficultyRating.Easy); + var issues = check.Run(context).ToList(); + + Assert.That(issues, Has.Count.EqualTo(count)); + Assert.That(issues.All(issue => issue.Template is CheckLowDiffOverlaps.IssueTemplateShouldProbablyOverlap)); + } + + private void assertShouldOverlap(IBeatmap beatmap, int count = 1) + { + var context = new BeatmapVerifierContext(beatmap, new TestWorkingBeatmap(beatmap), DifficultyRating.Easy); + var issues = check.Run(context).ToList(); + + Assert.That(issues, Has.Count.EqualTo(count)); + Assert.That(issues.All(issue => issue.Template is CheckLowDiffOverlaps.IssueTemplateShouldOverlap)); + } + + private void assertShouldNotOverlap(IBeatmap beatmap, int count = 1) + { + var context = new BeatmapVerifierContext(beatmap, new TestWorkingBeatmap(beatmap), DifficultyRating.Easy); + var issues = check.Run(context).ToList(); + + Assert.That(issues, Has.Count.EqualTo(count)); + Assert.That(issues.All(issue => issue.Template is CheckLowDiffOverlaps.IssueTemplateShouldNotOverlap)); + } + } +} diff --git a/osu.Game.Rulesets.Osu.Tests/osu.Game.Rulesets.Osu.Tests.csproj b/osu.Game.Rulesets.Osu.Tests/osu.Game.Rulesets.Osu.Tests.csproj index ebe642803b..1efd19f49d 100644 --- a/osu.Game.Rulesets.Osu.Tests/osu.Game.Rulesets.Osu.Tests.csproj +++ b/osu.Game.Rulesets.Osu.Tests/osu.Game.Rulesets.Osu.Tests.csproj @@ -3,6 +3,7 @@ + From 629c98e6a0d64e4a0dfb97ca1f876b5252e2d5dd Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Mon, 21 Jun 2021 15:34:11 +0200 Subject: [PATCH 35/39] Add time distance equality tests --- .../Checks/CheckTimeDistanceEqualityTest.cs | 324 ++++++++++++++++++ 1 file changed, 324 insertions(+) create mode 100644 osu.Game.Rulesets.Osu.Tests/Editor/Checks/CheckTimeDistanceEqualityTest.cs diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/Checks/CheckTimeDistanceEqualityTest.cs b/osu.Game.Rulesets.Osu.Tests/Editor/Checks/CheckTimeDistanceEqualityTest.cs new file mode 100644 index 0000000000..49a6fd12fa --- /dev/null +++ b/osu.Game.Rulesets.Osu.Tests/Editor/Checks/CheckTimeDistanceEqualityTest.cs @@ -0,0 +1,324 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System.Collections.Generic; +using System.Linq; +using Moq; +using NUnit.Framework; +using osu.Game.Beatmaps; +using osu.Game.Rulesets.Edit; +using osu.Game.Rulesets.Objects; +using osu.Game.Rulesets.Objects.Types; +using osu.Game.Rulesets.Osu.Edit.Checks; +using osu.Game.Rulesets.Osu.Objects; +using osu.Game.Tests.Beatmaps; +using osuTK; + +namespace osu.Game.Rulesets.Osu.Tests.Editor.Checks +{ + [TestFixture] + public class CheckTimeDistanceEqualityTest + { + private CheckTimeDistanceEquality check; + + [SetUp] + public void Setup() + { + check = new CheckTimeDistanceEquality(); + } + + [Test] + public void TestCirclesEquidistant() + { + assertOk(new Beatmap + { + HitObjects = new List + { + new HitCircle { StartTime = 0, Position = new Vector2(0) }, + new HitCircle { StartTime = 500, Position = new Vector2(50, 0) }, + new HitCircle { StartTime = 1000, Position = new Vector2(100, 0) }, + new HitCircle { StartTime = 1500, Position = new Vector2(150, 0) } + } + }); + } + + [Test] + public void TestCirclesOneSlightlyOff() + { + assertWarning(new Beatmap + { + HitObjects = new List + { + new HitCircle { StartTime = 0, Position = new Vector2(0) }, + new HitCircle { StartTime = 500, Position = new Vector2(50, 0) }, + new HitCircle { StartTime = 1000, Position = new Vector2(80, 0) }, // Distance a quite low compared to previous. + new HitCircle { StartTime = 1500, Position = new Vector2(130, 0) } + } + }); + } + + [Test] + public void TestCirclesOneOff() + { + assertProblem(new Beatmap + { + HitObjects = new List + { + new HitCircle { StartTime = 0, Position = new Vector2(0) }, + new HitCircle { StartTime = 500, Position = new Vector2(50, 0) }, + new HitCircle { StartTime = 1000, Position = new Vector2(150, 0) }, // Twice the regular spacing. + new HitCircle { StartTime = 1500, Position = new Vector2(100, 0) } + } + }); + } + + [Test] + public void TestCirclesTwoOff() + { + assertProblem(new Beatmap + { + HitObjects = new List + { + new HitCircle { StartTime = 0, Position = new Vector2(0) }, + new HitCircle { StartTime = 500, Position = new Vector2(50, 0) }, + new HitCircle { StartTime = 1000, Position = new Vector2(150, 0) }, // Twice the regular spacing. + new HitCircle { StartTime = 1500, Position = new Vector2(250, 0) } // Also twice the regular spacing. + } + }, count: 2); + } + + [Test] + public void TestCirclesStacked() + { + assertOk(new Beatmap + { + HitObjects = new List + { + new HitCircle { StartTime = 0, Position = new Vector2(0) }, + new HitCircle { StartTime = 500, Position = new Vector2(50, 0) }, + new HitCircle { StartTime = 1000, Position = new Vector2(50, 0) }, // Stacked, is fine. + new HitCircle { StartTime = 1500, Position = new Vector2(100, 0) } + } + }); + } + + [Test] + public void TestCirclesStacking() + { + assertWarning(new Beatmap + { + HitObjects = new List + { + new HitCircle { StartTime = 0, Position = new Vector2(0) }, + new HitCircle { StartTime = 500, Position = new Vector2(50, 0) }, + new HitCircle { StartTime = 1000, Position = new Vector2(50, 0), StackHeight = 1 }, + new HitCircle { StartTime = 1500, Position = new Vector2(50, 0), StackHeight = 2 }, + new HitCircle { StartTime = 2000, Position = new Vector2(50, 0), StackHeight = 3 }, + new HitCircle { StartTime = 2500, Position = new Vector2(50, 0), StackHeight = 4 }, // Ends up far from (50; 0), causing irregular spacing. + new HitCircle { StartTime = 3000, Position = new Vector2(100, 0) } + } + }); + } + + [Test] + public void TestCirclesHalfStack() + { + assertOk(new Beatmap + { + HitObjects = new List + { + new HitCircle { StartTime = 0, Position = new Vector2(0) }, + new HitCircle { StartTime = 500, Position = new Vector2(50, 0) }, + new HitCircle { StartTime = 1000, Position = new Vector2(55, 0) }, // Basically stacked, so is fine. + new HitCircle { StartTime = 1500, Position = new Vector2(105, 0) } + } + }); + } + + [Test] + public void TestCirclesPartialOverlap() + { + assertProblem(new Beatmap + { + HitObjects = new List + { + new HitCircle { StartTime = 0, Position = new Vector2(0) }, + new HitCircle { StartTime = 500, Position = new Vector2(50, 0) }, + new HitCircle { StartTime = 1000, Position = new Vector2(65, 0) }, // Really low distance compared to previous. + new HitCircle { StartTime = 1500, Position = new Vector2(115, 0) } + } + }); + } + + [Test] + public void TestCirclesSlightlyDifferent() + { + assertOk(new Beatmap + { + HitObjects = new List + { + // Does not need to be perfect, as long as the distance is approximately correct it's sight-readable. + new HitCircle { StartTime = 0, Position = new Vector2(0) }, + new HitCircle { StartTime = 500, Position = new Vector2(52, 0) }, + new HitCircle { StartTime = 1000, Position = new Vector2(97, 0) }, + new HitCircle { StartTime = 1500, Position = new Vector2(165, 0) } + } + }); + } + + [Test] + public void TestCirclesSlowlyChanging() + { + const float multiplier = 1.2f; + + assertOk(new Beatmap + { + HitObjects = new List + { + new HitCircle { StartTime = 0, Position = new Vector2(0) }, + new HitCircle { StartTime = 500, Position = new Vector2(50, 0) }, + new HitCircle { StartTime = 1000, Position = new Vector2(50 + 50 * multiplier, 0) }, + // This gap would be a warning if it weren't for the previous pushing the average spacing up. + new HitCircle { StartTime = 1500, Position = new Vector2(50 + 50 * multiplier + 50 * multiplier * multiplier, 0) } + } + }); + } + + [Test] + public void TestCirclesQuicklyChanging() + { + const float multiplier = 1.6f; + + var beatmap = new Beatmap + { + HitObjects = new List + { + new HitCircle { StartTime = 0, Position = new Vector2(0) }, + new HitCircle { StartTime = 500, Position = new Vector2(50, 0) }, + new HitCircle { StartTime = 1000, Position = new Vector2(50 + 50 * multiplier, 0) }, // Warning + new HitCircle { StartTime = 1500, Position = new Vector2(50 + 50 * multiplier + 50 * multiplier * multiplier, 0) } // Problem + } + }; + + var context = new BeatmapVerifierContext(beatmap, new TestWorkingBeatmap(beatmap), DifficultyRating.Easy); + var issues = check.Run(context).ToList(); + + Assert.That(issues, Has.Count.EqualTo(2)); + Assert.That(issues.First().Template is CheckTimeDistanceEquality.IssueTemplateIrregularSpacingWarning); + Assert.That(issues.Last().Template is CheckTimeDistanceEquality.IssueTemplateIrregularSpacingProblem); + } + + [Test] + public void TestCirclesTooFarApart() + { + assertOk(new Beatmap + { + HitObjects = new List + { + new HitCircle { StartTime = 0, Position = new Vector2(0) }, + new HitCircle { StartTime = 500, Position = new Vector2(50, 0) }, + new HitCircle { StartTime = 4000, Position = new Vector2(200, 0) }, // 2 seconds apart from previous, so can start from wherever. + new HitCircle { StartTime = 4500, Position = new Vector2(250, 0) } + } + }); + } + + [Test] + public void TestCirclesOneOffExpert() + { + assertOk(new Beatmap + { + HitObjects = new List + { + new HitCircle { StartTime = 0, Position = new Vector2(0) }, + new HitCircle { StartTime = 500, Position = new Vector2(50, 0) }, + new HitCircle { StartTime = 1000, Position = new Vector2(150, 0) }, // Jumps are allowed in higher difficulties. + new HitCircle { StartTime = 1500, Position = new Vector2(100, 0) } + } + }, DifficultyRating.Expert); + } + + [Test] + public void TestSpinner() + { + assertOk(new Beatmap + { + HitObjects = new List + { + new HitCircle { StartTime = 0, Position = new Vector2(0) }, + new HitCircle { StartTime = 500, Position = new Vector2(50, 0) }, + new Spinner { StartTime = 500, EndTime = 1000 }, // Distance to and from the spinner should be ignored. If it isn't this should give a problem. + new HitCircle { StartTime = 1500, Position = new Vector2(100, 0) }, + new HitCircle { StartTime = 2000, Position = new Vector2(150, 0) } + } + }); + } + + [Test] + public void TestSliders() + { + assertOk(new Beatmap + { + HitObjects = new List + { + new HitCircle { StartTime = 0, Position = new Vector2(0) }, + new HitCircle { StartTime = 500, Position = new Vector2(50, 0) }, + getSliderMock(startTime: 1000, endTime: 1500, startPosition: new Vector2(100, 0), endPosition: new Vector2(150, 0)).Object, + getSliderMock(startTime: 2000, endTime: 2500, startPosition: new Vector2(200, 0), endPosition: new Vector2(250, 0)).Object, + new HitCircle { StartTime = 2500, Position = new Vector2(300, 0) } + } + }); + } + + [Test] + public void TestSlidersOneOff() + { + assertProblem(new Beatmap + { + HitObjects = new List + { + new HitCircle { StartTime = 0, Position = new Vector2(0) }, + new HitCircle { StartTime = 500, Position = new Vector2(50, 0) }, + getSliderMock(startTime: 1000, endTime: 1500, startPosition: new Vector2(100, 0), endPosition: new Vector2(150, 0)).Object, + getSliderMock(startTime: 2000, endTime: 2500, startPosition: new Vector2(250, 0), endPosition: new Vector2(300, 0)).Object, // Twice the spacing. + new HitCircle { StartTime = 2500, Position = new Vector2(300, 0) } + } + }); + } + + private Mock getSliderMock(double startTime, double endTime, Vector2 startPosition, Vector2 endPosition) + { + var mockSlider = new Mock(); + mockSlider.SetupGet(s => s.StartTime).Returns(startTime); + mockSlider.SetupGet(s => s.Position).Returns(startPosition); + mockSlider.SetupGet(s => s.EndPosition).Returns(endPosition); + mockSlider.As().Setup(d => d.EndTime).Returns(endTime); + + return mockSlider; + } + + private void assertOk(IBeatmap beatmap, DifficultyRating difficultyRating = DifficultyRating.Easy) + { + var context = new BeatmapVerifierContext(beatmap, new TestWorkingBeatmap(beatmap), difficultyRating); + Assert.That(check.Run(context), Is.Empty); + } + + private void assertWarning(IBeatmap beatmap, int count = 1) + { + var context = new BeatmapVerifierContext(beatmap, new TestWorkingBeatmap(beatmap), DifficultyRating.Easy); + var issues = check.Run(context).ToList(); + + Assert.That(issues, Has.Count.EqualTo(count)); + Assert.That(issues.All(issue => issue.Template is CheckTimeDistanceEquality.IssueTemplateIrregularSpacingWarning)); + } + + private void assertProblem(IBeatmap beatmap, int count = 1) + { + var context = new BeatmapVerifierContext(beatmap, new TestWorkingBeatmap(beatmap), DifficultyRating.Easy); + var issues = check.Run(context).ToList(); + + Assert.That(issues, Has.Count.EqualTo(count)); + Assert.That(issues.All(issue => issue.Template is CheckTimeDistanceEquality.IssueTemplateIrregularSpacingProblem)); + } + } +} From e9339d6100b43c72308862582215c00ff3a2950d Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 22 Jun 2021 15:16:19 +0900 Subject: [PATCH 36/39] Move some inline comments on `const`s to xmldoc instead --- .../Edit/Checks/CheckLowDiffOverlaps.cs | 4 ++- .../Edit/Checks/CheckTimeDistanceEquality.cs | 34 +++++++++++++++---- 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Edit/Checks/CheckLowDiffOverlaps.cs b/osu.Game.Rulesets.Osu/Edit/Checks/CheckLowDiffOverlaps.cs index 488bdfd972..1dd859b5b8 100644 --- a/osu.Game.Rulesets.Osu/Edit/Checks/CheckLowDiffOverlaps.cs +++ b/osu.Game.Rulesets.Osu/Edit/Checks/CheckLowDiffOverlaps.cs @@ -17,7 +17,9 @@ namespace osu.Game.Rulesets.Osu.Edit.Checks private const double should_probably_overlap_threshold = 175; // 170 BPM 1/2 private const double should_not_overlap_threshold = 250; // 120 BPM 1/2 = 240 BPM 1/1 - // Objects need to overlap this much before being treated as an overlap, else it may just be the borders slightly touching. + /// + /// Objects need to overlap this much before being treated as an overlap, else it may just be the borders slightly touching. + /// private const double overlap_leniency = 5; public CheckMetadata Metadata { get; } = new CheckMetadata(CheckCategory.Spread, "Missing or unexpected overlaps"); diff --git a/osu.Game.Rulesets.Osu/Edit/Checks/CheckTimeDistanceEquality.cs b/osu.Game.Rulesets.Osu/Edit/Checks/CheckTimeDistanceEquality.cs index db48878dd3..6420d9558e 100644 --- a/osu.Game.Rulesets.Osu/Edit/Checks/CheckTimeDistanceEquality.cs +++ b/osu.Game.Rulesets.Osu/Edit/Checks/CheckTimeDistanceEquality.cs @@ -14,14 +14,36 @@ namespace osu.Game.Rulesets.Osu.Edit.Checks { public class CheckTimeDistanceEquality : ICheck { - private const double pattern_lifetime = 600; // Two objects this many ms apart or more are skipped. (200 BPM 2/1) - private const double stack_leniency = 12; // Two objects this distance apart or less are skipped. + /// + /// Two objects this many ms apart or more are skipped. (200 BPM 2/1) + /// + private const double pattern_lifetime = 600; - private const double observation_lifetime = 4000; // How long an observation is relevant for comparison. (120 BPM 8/1) - private const double similar_time_leniency = 16; // How different two delta times can be to still be compared. (240 BPM 1/16) + /// + /// Two objects this distance apart or less are skipped. + /// + private const double stack_leniency = 12; + + /// + /// How long an observation is relevant for comparison. (120 BPM 8/1) + /// + private const double observation_lifetime = 4000; + + /// + /// How different two delta times can be to still be compared. (240 BPM 1/16) + /// + private const double similar_time_leniency = 16; + + /// + /// How many pixels are subtracted from the difference between current and expected distance. + /// + private const double distance_leniency_absolute_warning = 10; + + /// + /// How much of the current distance that the difference can make out. + /// + private const double distance_leniency_percent_warning = 0.15; - private const double distance_leniency_absolute_warning = 10; // How many pixels are subtracted from the difference between current and expected distance. - private const double distance_leniency_percent_warning = 0.15; // How much of the current distance that the difference can make out. private const double distance_leniency_absolute_problem = 20; private const double distance_leniency_percent_problem = 0.3; From b54e82eb993d2c41d0f0b98b094c8188414cdf05 Mon Sep 17 00:00:00 2001 From: ekrctb Date: Tue, 22 Jun 2021 12:34:34 +0900 Subject: [PATCH 37/39] Remove unused argument from `CatchPlayfield` --- osu.Game.Rulesets.Catch/UI/CatchPlayfield.cs | 3 +-- osu.Game.Rulesets.Catch/UI/DrawableCatchRuleset.cs | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/osu.Game.Rulesets.Catch/UI/CatchPlayfield.cs b/osu.Game.Rulesets.Catch/UI/CatchPlayfield.cs index 0e1ef90737..644facdabc 100644 --- a/osu.Game.Rulesets.Catch/UI/CatchPlayfield.cs +++ b/osu.Game.Rulesets.Catch/UI/CatchPlayfield.cs @@ -1,7 +1,6 @@ // Copyright (c) ppy Pty Ltd . 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.Graphics; using osu.Framework.Graphics.Containers; @@ -34,7 +33,7 @@ namespace osu.Game.Rulesets.Catch.UI // only check the X position; handle all vertical space. base.ReceivePositionalInputAt(new Vector2(screenSpacePos.X, ScreenSpaceDrawQuad.Centre.Y)); - public CatchPlayfield(BeatmapDifficulty difficulty, Func> createDrawableRepresentation) + public CatchPlayfield(BeatmapDifficulty difficulty) { var droppedObjectContainer = new Container { diff --git a/osu.Game.Rulesets.Catch/UI/DrawableCatchRuleset.cs b/osu.Game.Rulesets.Catch/UI/DrawableCatchRuleset.cs index 9389fa803b..8b6a074426 100644 --- a/osu.Game.Rulesets.Catch/UI/DrawableCatchRuleset.cs +++ b/osu.Game.Rulesets.Catch/UI/DrawableCatchRuleset.cs @@ -34,7 +34,7 @@ namespace osu.Game.Rulesets.Catch.UI protected override ReplayRecorder CreateReplayRecorder(Score score) => new CatchReplayRecorder(score, (CatchPlayfield)Playfield); - protected override Playfield CreatePlayfield() => new CatchPlayfield(Beatmap.BeatmapInfo.BaseDifficulty, CreateDrawableRepresentation); + protected override Playfield CreatePlayfield() => new CatchPlayfield(Beatmap.BeatmapInfo.BaseDifficulty); public override PlayfieldAdjustmentContainer CreatePlayfieldAdjustmentContainer() => new CatchPlayfieldAdjustmentContainer(); From ffac32a848b7755aa5e2f17f912b75946ab08014 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 22 Jun 2021 16:40:48 +0900 Subject: [PATCH 38/39] Reword xmldoc --- osu.Game/Skinning/ISkinnableDrawable.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/osu.Game/Skinning/ISkinnableDrawable.cs b/osu.Game/Skinning/ISkinnableDrawable.cs index 9625a9eb6d..60b40982e5 100644 --- a/osu.Game/Skinning/ISkinnableDrawable.cs +++ b/osu.Game/Skinning/ISkinnableDrawable.cs @@ -16,8 +16,9 @@ namespace osu.Game.Skinning bool IsEditable => true; /// - /// if this 's is automatically determined by proximity, - /// if the user has chosen a fixed anchor point. + /// In the context of the skin layout editor, whether this has a permanent anchor defined. + /// If , this 's is automatically determined by proximity, + /// If , a fixed anchor point has been defined. /// bool UsesFixedAnchor { get; set; } } From 4b3165084d72b4fefb2fe81ec9fb401da2c5fdc5 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 22 Jun 2021 16:40:56 +0900 Subject: [PATCH 39/39] Move scoped functionality into local function --- .../Skinning/Editor/SkinSelectionHandler.cs | 38 ++++++++++--------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs index c2ad08f0dc..23f36ffe5b 100644 --- a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs +++ b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs @@ -281,23 +281,7 @@ namespace osu.Game.Skinning.Editor if (parent == null) return drawable.Anchor; - Vector2 screenPosition; - { - var quad = drawable.ScreenSpaceDrawQuad; - var origin = drawable.Origin; - - screenPosition = quad.TopLeft; - - if (origin.HasFlagFast(Anchor.x2)) - screenPosition.X += quad.Width; - else if (origin.HasFlagFast(Anchor.x1)) - screenPosition.X += quad.Width / 2f; - - if (origin.HasFlagFast(Anchor.y2)) - screenPosition.Y += quad.Height; - else if (origin.HasFlagFast(Anchor.y1)) - screenPosition.Y += quad.Height / 2f; - } + var screenPosition = getScreenPosition(); var absolutePosition = parent.ToLocalSpace(screenPosition); var factor = parent.RelativeToAbsoluteFactor; @@ -319,6 +303,26 @@ namespace osu.Game.Skinning.Editor result |= getAnchorFromPosition(absolutePosition.Y / factor.Y, Anchor.y0, Anchor.y1, Anchor.y2); return result; + + Vector2 getScreenPosition() + { + var quad = drawable.ScreenSpaceDrawQuad; + var origin = drawable.Origin; + + var pos = quad.TopLeft; + + if (origin.HasFlagFast(Anchor.x2)) + pos.X += quad.Width; + else if (origin.HasFlagFast(Anchor.x1)) + pos.X += quad.Width / 2f; + + if (origin.HasFlagFast(Anchor.y2)) + pos.Y += quad.Height; + else if (origin.HasFlagFast(Anchor.y1)) + pos.Y += quad.Height / 2f; + + return pos; + } } private static void applyAnchor(Drawable drawable, Anchor anchor)