From 668f89d8b230103d8daee7fd639fde069baa4d99 Mon Sep 17 00:00:00 2001 From: Endrik Tombak Date: Tue, 22 Dec 2020 17:33:11 +0200 Subject: [PATCH 01/16] Copy test from #11019 --- .../TestSceneSectionsContainer.cs | 122 ++++++++++++++++++ 1 file changed, 122 insertions(+) create mode 100644 osu.Game.Tests/Visual/UserInterface/TestSceneSectionsContainer.cs diff --git a/osu.Game.Tests/Visual/UserInterface/TestSceneSectionsContainer.cs b/osu.Game.Tests/Visual/UserInterface/TestSceneSectionsContainer.cs new file mode 100644 index 0000000000..5c2e6e457d --- /dev/null +++ b/osu.Game.Tests/Visual/UserInterface/TestSceneSectionsContainer.cs @@ -0,0 +1,122 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System.Linq; +using NUnit.Framework; +using osu.Framework.Graphics; +using osu.Framework.Graphics.Colour; +using osu.Framework.Graphics.Shapes; +using osu.Game.Graphics.Containers; +using osuTK.Graphics; + +namespace osu.Game.Tests.Visual.UserInterface +{ + public class TestSceneSectionsContainer : OsuManualInputManagerTestScene + { + private readonly SectionsContainer container; + private float custom; + private const float header_height = 100; + + public TestSceneSectionsContainer() + { + container = new SectionsContainer + { + RelativeSizeAxes = Axes.Y, + Width = 300, + Origin = Anchor.Centre, + Anchor = Anchor.Centre, + FixedHeader = new Box + { + Alpha = 0.5f, + Width = 300, + Height = header_height, + Colour = Color4.Red + } + }; + container.SelectedSection.ValueChanged += section => + { + if (section.OldValue != null) + section.OldValue.Selected = false; + if (section.NewValue != null) + section.NewValue.Selected = true; + }; + Add(container); + } + + [Test] + public void TestSelection() + { + AddStep("clear", () => container.Clear()); + AddStep("add 1/8th", () => append(1 / 8.0f)); + AddStep("add third", () => append(1 / 3.0f)); + AddStep("add half", () => append(1 / 2.0f)); + AddStep("add full", () => append(1)); + AddSliderStep("set custom", 0.1f, 1.1f, 0.5f, i => custom = i); + AddStep("add custom", () => append(custom)); + AddStep("scroll to previous", () => container.ScrollTo( + container.Children.Reverse().SkipWhile(s => s != container.SelectedSection.Value).Skip(1).FirstOrDefault() ?? container.Children.First() + )); + AddStep("scroll to next", () => container.ScrollTo( + container.Children.SkipWhile(s => s != container.SelectedSection.Value).Skip(1).FirstOrDefault() ?? container.Children.Last() + )); + AddStep("scroll up", () => triggerUserScroll(1)); + AddStep("scroll down", () => triggerUserScroll(-1)); + } + + [Test] + public void TestCorrectSectionSelected() + { + const int sections_count = 11; + float[] alternating = { 0.07f, 0.33f, 0.16f, 0.33f }; + AddStep("clear", () => container.Clear()); + AddStep("fill with sections", () => + { + for (int i = 0; i < sections_count; i++) + append(alternating[i % alternating.Length]); + }); + + void step(int scrollIndex) + { + AddStep($"scroll to section {scrollIndex + 1}", () => container.ScrollTo(container.Children[scrollIndex])); + AddUntilStep("correct section selected", () => container.SelectedSection.Value == container.Children[scrollIndex]); + } + + for (int i = 1; i < sections_count; i++) + step(i); + for (int i = sections_count - 2; i >= 0; i--) + step(i); + + AddStep("scroll almost to end", () => container.ScrollTo(container.Children[sections_count - 2])); + AddUntilStep("correct section selected", () => container.SelectedSection.Value == container.Children[sections_count - 2]); + AddStep("scroll down", () => triggerUserScroll(-1)); + AddUntilStep("correct section selected", () => container.SelectedSection.Value == container.Children[sections_count - 1]); + } + + private static readonly ColourInfo selected_colour = ColourInfo.GradientVertical(Color4.Yellow, Color4.Gold); + private static readonly ColourInfo default_colour = ColourInfo.GradientVertical(Color4.White, Color4.DarkGray); + + private void append(float multiplier) + { + container.Add(new TestSection + { + Width = 300, + Height = (container.ChildSize.Y - header_height) * multiplier, + Colour = default_colour + }); + } + + private void triggerUserScroll(float direction) + { + InputManager.MoveMouseTo(container); + InputManager.ScrollVerticalBy(direction); + } + + private class TestSection : Box + { + public bool Selected + { + set => Colour = value ? selected_colour : default_colour; + } + } + } +} From 78c14fd69693f688f40595699eba849486f49463 Mon Sep 17 00:00:00 2001 From: Endrik Tombak Date: Tue, 22 Dec 2020 17:36:44 +0200 Subject: [PATCH 02/16] Refactor code into UserTrackingScrollContainer --- .../Graphics/Containers/SectionsContainer.cs | 4 +- .../Containers/UserTrackingScrollContainer.cs | 49 +++++++++++++++++++ osu.Game/Overlays/OverlayScrollContainer.cs | 4 +- osu.Game/Overlays/UserProfileOverlay.cs | 2 +- osu.Game/Screens/Select/BeatmapCarousel.cs | 20 +------- 5 files changed, 55 insertions(+), 24 deletions(-) create mode 100644 osu.Game/Graphics/Containers/UserTrackingScrollContainer.cs diff --git a/osu.Game/Graphics/Containers/SectionsContainer.cs b/osu.Game/Graphics/Containers/SectionsContainer.cs index 81968de304..6e9520ef8f 100644 --- a/osu.Game/Graphics/Containers/SectionsContainer.cs +++ b/osu.Game/Graphics/Containers/SectionsContainer.cs @@ -95,7 +95,7 @@ namespace osu.Game.Graphics.Containers protected override Container Content => scrollContentContainer; - private readonly OsuScrollContainer scrollContainer; + private readonly UserTrackingScrollContainer scrollContainer; private readonly Container headerBackgroundContainer; private readonly MarginPadding originalSectionsMargin; private Drawable expandableHeader, fixedHeader, footer, headerBackground; @@ -139,7 +139,7 @@ namespace osu.Game.Graphics.Containers public void ScrollToTop() => scrollContainer.ScrollTo(0); [NotNull] - protected virtual OsuScrollContainer CreateScrollContainer() => new OsuScrollContainer(); + protected virtual UserTrackingScrollContainer CreateScrollContainer() => new UserTrackingScrollContainer(); [NotNull] protected virtual FlowContainer CreateScrollContentContainer() => diff --git a/osu.Game/Graphics/Containers/UserTrackingScrollContainer.cs b/osu.Game/Graphics/Containers/UserTrackingScrollContainer.cs new file mode 100644 index 0000000000..b8ce34b204 --- /dev/null +++ b/osu.Game/Graphics/Containers/UserTrackingScrollContainer.cs @@ -0,0 +1,49 @@ +// 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.Graphics; + +namespace osu.Game.Graphics.Containers +{ + public class UserTrackingScrollContainer : UserTrackingScrollContainer + { + public UserTrackingScrollContainer() + { + } + + public UserTrackingScrollContainer(Direction direction) + : base(direction) + { + } + } + + public class UserTrackingScrollContainer : OsuScrollContainer + where T : Drawable + { + /// + /// Whether the last scroll event was user triggered, directly on the scroll container. + /// + public bool UserScrolling { get; private set; } + + public UserTrackingScrollContainer() + { + } + + public UserTrackingScrollContainer(Direction direction) + : base(direction) + { + } + + protected override void OnUserScroll(float value, bool animated = true, double? distanceDecay = default) + { + UserScrolling = true; + base.OnUserScroll(value, animated, distanceDecay); + } + + public new void ScrollTo(float value, bool animated = true, double? distanceDecay = null) + { + UserScrolling = false; + base.ScrollTo(value, animated, distanceDecay); + } + } +} diff --git a/osu.Game/Overlays/OverlayScrollContainer.cs b/osu.Game/Overlays/OverlayScrollContainer.cs index b67d5db1a4..0004719b87 100644 --- a/osu.Game/Overlays/OverlayScrollContainer.cs +++ b/osu.Game/Overlays/OverlayScrollContainer.cs @@ -17,9 +17,9 @@ using osuTK.Graphics; namespace osu.Game.Overlays { /// - /// which provides . Mostly used in . + /// which provides . Mostly used in . /// - public class OverlayScrollContainer : OsuScrollContainer + public class OverlayScrollContainer : UserTrackingScrollContainer { /// /// Scroll position at which the will be shown. diff --git a/osu.Game/Overlays/UserProfileOverlay.cs b/osu.Game/Overlays/UserProfileOverlay.cs index 81027667fa..7f29545c2e 100644 --- a/osu.Game/Overlays/UserProfileOverlay.cs +++ b/osu.Game/Overlays/UserProfileOverlay.cs @@ -202,7 +202,7 @@ namespace osu.Game.Overlays RelativeSizeAxes = Axes.Both; } - protected override OsuScrollContainer CreateScrollContainer() => new OverlayScrollContainer(); + protected override UserTrackingScrollContainer CreateScrollContainer() => new OverlayScrollContainer(); protected override FlowContainer CreateScrollContentContainer() => new FillFlowContainer { diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index d76f0abb9e..e9a8d28316 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -901,15 +901,10 @@ namespace osu.Game.Screens.Select } } - protected class CarouselScrollContainer : OsuScrollContainer + protected class CarouselScrollContainer : UserTrackingScrollContainer { private bool rightMouseScrollBlocked; - /// - /// Whether the last scroll event was user triggered, directly on the scroll container. - /// - public bool UserScrolling { get; private set; } - public CarouselScrollContainer() { // size is determined by the carousel itself, due to not all content necessarily being loaded. @@ -919,19 +914,6 @@ namespace osu.Game.Screens.Select Masking = false; } - // ReSharper disable once OptionalParameterHierarchyMismatch 2020.3 EAP4 bug. (https://youtrack.jetbrains.com/issue/RSRP-481535?p=RIDER-51910) - protected override void OnUserScroll(float value, bool animated = true, double? distanceDecay = default) - { - UserScrolling = true; - base.OnUserScroll(value, animated, distanceDecay); - } - - public new void ScrollTo(float value, bool animated = true, double? distanceDecay = null) - { - UserScrolling = false; - base.ScrollTo(value, animated, distanceDecay); - } - protected override bool OnMouseDown(MouseDownEvent e) { if (e.Button == MouseButton.Right) From 2cf76ebc75c6e0dc4bfbe428a9ee099c21a5eeb9 Mon Sep 17 00:00:00 2001 From: Endrik Tombak Date: Tue, 22 Dec 2020 17:51:12 +0200 Subject: [PATCH 03/16] Scroll to 20% and select section intersecting below there --- .../Graphics/Containers/SectionsContainer.cs | 43 +++++++++++++++---- 1 file changed, 35 insertions(+), 8 deletions(-) diff --git a/osu.Game/Graphics/Containers/SectionsContainer.cs b/osu.Game/Graphics/Containers/SectionsContainer.cs index 6e9520ef8f..b5f81c516a 100644 --- a/osu.Game/Graphics/Containers/SectionsContainer.cs +++ b/osu.Game/Graphics/Containers/SectionsContainer.cs @@ -9,6 +9,7 @@ using osu.Framework.Bindables; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Layout; +using osu.Framework.Utils; namespace osu.Game.Graphics.Containers { @@ -20,6 +21,8 @@ namespace osu.Game.Graphics.Containers where T : Drawable { public Bindable SelectedSection { get; } = new Bindable(); + private Drawable lastClickedSection; + private T smallestSection; public Drawable ExpandableHeader { @@ -131,10 +134,21 @@ namespace osu.Game.Graphics.Containers lastKnownScroll = float.NaN; headerHeight = float.NaN; footerHeight = float.NaN; + + if (drawable == null) + return; + + if (smallestSection == null || smallestSection.Height > drawable.Height) + smallestSection = drawable; } - public void ScrollTo(Drawable section) => - scrollContainer.ScrollTo(scrollContainer.GetChildPosInContent(section) - (FixedHeader?.BoundingBox.Height ?? 0)); + private const float scroll_target_multiplier = 0.2f; + + public void ScrollTo(Drawable section) + { + lastClickedSection = section; + scrollContainer.ScrollTo(scrollContainer.GetChildPosInContent(section) - scrollContainer.DisplayableContent * scroll_target_multiplier - (FixedHeader?.BoundingBox.Height ?? 0)); + } public void ScrollToTop() => scrollContainer.ScrollTo(0); @@ -183,6 +197,10 @@ namespace osu.Game.Graphics.Containers { lastKnownScroll = currentScroll; + // reset last clicked section because user started scrolling themselves + if (scrollContainer.UserScrolling) + lastClickedSection = null; + if (ExpandableHeader != null && FixedHeader != null) { float offset = Math.Min(ExpandableHeader.LayoutSize.Y, currentScroll); @@ -194,18 +212,27 @@ namespace osu.Game.Graphics.Containers headerBackgroundContainer.Height = (ExpandableHeader?.LayoutSize.Y ?? 0) + (FixedHeader?.LayoutSize.Y ?? 0); headerBackgroundContainer.Y = ExpandableHeader?.Y ?? 0; - float scrollOffset = FixedHeader?.LayoutSize.Y ?? 0; + // scroll offset is our fixed header height if we have it plus 20% of content height + // plus 5% to fix floating point errors and to not have a section instantly unselect when scrolling upwards + // but the 5% can't be bigger than our smallest section height, otherwise it won't get selected correctly + float sectionOrContent = Math.Min(smallestSection?.Height / 2.0f ?? 0, scrollContainer.DisplayableContent * 0.05f); + float scrollOffset = (FixedHeader?.LayoutSize.Y ?? 0) + scrollContainer.DisplayableContent * scroll_target_multiplier + sectionOrContent; Func diff = section => scrollContainer.GetChildPosInContent(section) - currentScroll - scrollOffset; - if (scrollContainer.IsScrolledToEnd()) + if (Precision.AlmostBigger(0, scrollContainer.Current)) { - SelectedSection.Value = Children.LastOrDefault(); + SelectedSection.Value = lastClickedSection as T ?? Children.FirstOrDefault(); + return; } - else + + if (Precision.AlmostBigger(scrollContainer.Current, scrollContainer.ScrollableExtent)) { - SelectedSection.Value = Children.TakeWhile(section => diff(section) <= 0).LastOrDefault() - ?? Children.FirstOrDefault(); + SelectedSection.Value = lastClickedSection as T ?? Children.LastOrDefault(); + return; } + + SelectedSection.Value = Children.TakeWhile(section => diff(section) <= 0).LastOrDefault() + ?? Children.FirstOrDefault(); } } From 0fcf61d352197ef5f4d8e10d8bf1d19d55c266b0 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 21 Jan 2021 14:07:02 +0900 Subject: [PATCH 04/16] Replace null check with assert --- osu.Game/Graphics/Containers/SectionsContainer.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/osu.Game/Graphics/Containers/SectionsContainer.cs b/osu.Game/Graphics/Containers/SectionsContainer.cs index b5f81c516a..d378b67acf 100644 --- a/osu.Game/Graphics/Containers/SectionsContainer.cs +++ b/osu.Game/Graphics/Containers/SectionsContainer.cs @@ -2,6 +2,7 @@ // See the LICENCE file in the repository root for full licence text. using System; +using System.Diagnostics; using System.Linq; using JetBrains.Annotations; using osu.Framework.Allocation; @@ -131,13 +132,13 @@ namespace osu.Game.Graphics.Containers public override void Add(T drawable) { base.Add(drawable); + + Debug.Assert(drawable != null); + lastKnownScroll = float.NaN; headerHeight = float.NaN; footerHeight = float.NaN; - if (drawable == null) - return; - if (smallestSection == null || smallestSection.Height > drawable.Height) smallestSection = drawable; } From 8f9089d1aefa26c90a883e226dcce91b9cabfd8e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 21 Jan 2021 14:30:22 +0900 Subject: [PATCH 05/16] Move constant to a better place --- osu.Game/Graphics/Containers/SectionsContainer.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Graphics/Containers/SectionsContainer.cs b/osu.Game/Graphics/Containers/SectionsContainer.cs index d378b67acf..fd8b98e767 100644 --- a/osu.Game/Graphics/Containers/SectionsContainer.cs +++ b/osu.Game/Graphics/Containers/SectionsContainer.cs @@ -109,6 +109,8 @@ namespace osu.Game.Graphics.Containers private float lastKnownScroll; + private const float scroll_target_multiplier = 0.2f; + public SectionsContainer() { AddRangeInternal(new Drawable[] @@ -143,8 +145,6 @@ namespace osu.Game.Graphics.Containers smallestSection = drawable; } - private const float scroll_target_multiplier = 0.2f; - public void ScrollTo(Drawable section) { lastClickedSection = section; From 555abcdc3695c437efe3277fc738e54b637fe235 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 21 Jan 2021 14:31:31 +0900 Subject: [PATCH 06/16] Replace nan usage with nullable float --- .../Graphics/Containers/SectionsContainer.cs | 25 ++++++++++--------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/osu.Game/Graphics/Containers/SectionsContainer.cs b/osu.Game/Graphics/Containers/SectionsContainer.cs index fd8b98e767..2140e251f8 100644 --- a/osu.Game/Graphics/Containers/SectionsContainer.cs +++ b/osu.Game/Graphics/Containers/SectionsContainer.cs @@ -40,7 +40,7 @@ namespace osu.Game.Graphics.Containers if (value == null) return; AddInternal(expandableHeader); - lastKnownScroll = float.NaN; + lastKnownScroll = null; } } @@ -56,7 +56,7 @@ namespace osu.Game.Graphics.Containers if (value == null) return; AddInternal(fixedHeader); - lastKnownScroll = float.NaN; + lastKnownScroll = null; } } @@ -75,7 +75,7 @@ namespace osu.Game.Graphics.Containers footer.Anchor |= Anchor.y2; footer.Origin |= Anchor.y2; scrollContainer.Add(footer); - lastKnownScroll = float.NaN; + lastKnownScroll = null; } } @@ -93,7 +93,7 @@ namespace osu.Game.Graphics.Containers headerBackgroundContainer.Add(headerBackground); - lastKnownScroll = float.NaN; + lastKnownScroll = null; } } @@ -105,9 +105,9 @@ namespace osu.Game.Graphics.Containers private Drawable expandableHeader, fixedHeader, footer, headerBackground; private FlowContainer scrollContentContainer; - private float headerHeight, footerHeight; + private float? headerHeight, footerHeight; - private float lastKnownScroll; + private float? lastKnownScroll; private const float scroll_target_multiplier = 0.2f; @@ -137,9 +137,9 @@ namespace osu.Game.Graphics.Containers Debug.Assert(drawable != null); - lastKnownScroll = float.NaN; - headerHeight = float.NaN; - footerHeight = float.NaN; + lastKnownScroll = null; + headerHeight = null; + footerHeight = null; if (smallestSection == null || smallestSection.Height > drawable.Height) smallestSection = drawable; @@ -171,7 +171,7 @@ namespace osu.Game.Graphics.Containers if (source == InvalidationSource.Child && (invalidation & Invalidation.DrawSize) != 0) { - lastKnownScroll = -1; + lastKnownScroll = null; result = true; } @@ -242,8 +242,9 @@ namespace osu.Game.Graphics.Containers if (!Children.Any()) return; var newMargin = originalSectionsMargin; - newMargin.Top += headerHeight; - newMargin.Bottom += footerHeight; + + newMargin.Top += (headerHeight ?? 0); + newMargin.Bottom += (footerHeight ?? 0); scrollContentContainer.Margin = newMargin; } From 6d167b7865688b4f73cb94106fbbe233e739ce40 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 21 Jan 2021 14:40:55 +0900 Subject: [PATCH 07/16] Remove the need to store the smallest section --- osu.Game/Graphics/Containers/SectionsContainer.cs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/osu.Game/Graphics/Containers/SectionsContainer.cs b/osu.Game/Graphics/Containers/SectionsContainer.cs index 2140e251f8..6607193cc6 100644 --- a/osu.Game/Graphics/Containers/SectionsContainer.cs +++ b/osu.Game/Graphics/Containers/SectionsContainer.cs @@ -23,7 +23,6 @@ namespace osu.Game.Graphics.Containers { public Bindable SelectedSection { get; } = new Bindable(); private Drawable lastClickedSection; - private T smallestSection; public Drawable ExpandableHeader { @@ -140,9 +139,6 @@ namespace osu.Game.Graphics.Containers lastKnownScroll = null; headerHeight = null; footerHeight = null; - - if (smallestSection == null || smallestSection.Height > drawable.Height) - smallestSection = drawable; } public void ScrollTo(Drawable section) @@ -213,10 +209,12 @@ namespace osu.Game.Graphics.Containers headerBackgroundContainer.Height = (ExpandableHeader?.LayoutSize.Y ?? 0) + (FixedHeader?.LayoutSize.Y ?? 0); headerBackgroundContainer.Y = ExpandableHeader?.Y ?? 0; + var smallestSectionHeight = Children.Count > 0 ? Children.Min(d => d.Height) : 0; + // scroll offset is our fixed header height if we have it plus 20% of content height // plus 5% to fix floating point errors and to not have a section instantly unselect when scrolling upwards // but the 5% can't be bigger than our smallest section height, otherwise it won't get selected correctly - float sectionOrContent = Math.Min(smallestSection?.Height / 2.0f ?? 0, scrollContainer.DisplayableContent * 0.05f); + float sectionOrContent = Math.Min(smallestSectionHeight / 2.0f, scrollContainer.DisplayableContent * 0.05f); float scrollOffset = (FixedHeader?.LayoutSize.Y ?? 0) + scrollContainer.DisplayableContent * scroll_target_multiplier + sectionOrContent; Func diff = section => scrollContainer.GetChildPosInContent(section) - currentScroll - scrollOffset; From e5eec27e9574037e13287b11438c044d8cc1c0da Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 21 Jan 2021 14:44:47 +0900 Subject: [PATCH 08/16] Simplify selected section resolution --- osu.Game/Graphics/Containers/SectionsContainer.cs | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/osu.Game/Graphics/Containers/SectionsContainer.cs b/osu.Game/Graphics/Containers/SectionsContainer.cs index 6607193cc6..cae2dd7e37 100644 --- a/osu.Game/Graphics/Containers/SectionsContainer.cs +++ b/osu.Game/Graphics/Containers/SectionsContainer.cs @@ -215,23 +215,16 @@ namespace osu.Game.Graphics.Containers // plus 5% to fix floating point errors and to not have a section instantly unselect when scrolling upwards // but the 5% can't be bigger than our smallest section height, otherwise it won't get selected correctly float sectionOrContent = Math.Min(smallestSectionHeight / 2.0f, scrollContainer.DisplayableContent * 0.05f); + float scrollOffset = (FixedHeader?.LayoutSize.Y ?? 0) + scrollContainer.DisplayableContent * scroll_target_multiplier + sectionOrContent; Func diff = section => scrollContainer.GetChildPosInContent(section) - currentScroll - scrollOffset; if (Precision.AlmostBigger(0, scrollContainer.Current)) - { SelectedSection.Value = lastClickedSection as T ?? Children.FirstOrDefault(); - return; - } - - if (Precision.AlmostBigger(scrollContainer.Current, scrollContainer.ScrollableExtent)) - { + else if (Precision.AlmostBigger(scrollContainer.Current, scrollContainer.ScrollableExtent)) SelectedSection.Value = lastClickedSection as T ?? Children.LastOrDefault(); - return; - } - - SelectedSection.Value = Children.TakeWhile(section => diff(section) <= 0).LastOrDefault() - ?? Children.FirstOrDefault(); + else + SelectedSection.Value = Children.TakeWhile(section => diff(section) <= 0).LastOrDefault() ?? Children.FirstOrDefault(); } } From a85f952a381ca32869e06a16a23c149955a88dbc Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 21 Jan 2021 14:46:35 +0900 Subject: [PATCH 09/16] Inline single use function --- osu.Game/Graphics/Containers/SectionsContainer.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/osu.Game/Graphics/Containers/SectionsContainer.cs b/osu.Game/Graphics/Containers/SectionsContainer.cs index cae2dd7e37..cf361abadc 100644 --- a/osu.Game/Graphics/Containers/SectionsContainer.cs +++ b/osu.Game/Graphics/Containers/SectionsContainer.cs @@ -217,14 +217,17 @@ namespace osu.Game.Graphics.Containers float sectionOrContent = Math.Min(smallestSectionHeight / 2.0f, scrollContainer.DisplayableContent * 0.05f); float scrollOffset = (FixedHeader?.LayoutSize.Y ?? 0) + scrollContainer.DisplayableContent * scroll_target_multiplier + sectionOrContent; - Func diff = section => scrollContainer.GetChildPosInContent(section) - currentScroll - scrollOffset; if (Precision.AlmostBigger(0, scrollContainer.Current)) SelectedSection.Value = lastClickedSection as T ?? Children.FirstOrDefault(); else if (Precision.AlmostBigger(scrollContainer.Current, scrollContainer.ScrollableExtent)) SelectedSection.Value = lastClickedSection as T ?? Children.LastOrDefault(); else - SelectedSection.Value = Children.TakeWhile(section => diff(section) <= 0).LastOrDefault() ?? Children.FirstOrDefault(); + { + SelectedSection.Value = Children + .TakeWhile(section => scrollContainer.GetChildPosInContent(section) - currentScroll - scrollOffset <= 0) + .LastOrDefault() ?? Children.FirstOrDefault(); + } } } From 9daf29fedcb0b4e74a7a3d17579557c68fd4723a Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 21 Jan 2021 14:52:41 +0900 Subject: [PATCH 10/16] Extract out commonly used variables --- osu.Game/Graphics/Containers/SectionsContainer.cs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/osu.Game/Graphics/Containers/SectionsContainer.cs b/osu.Game/Graphics/Containers/SectionsContainer.cs index cf361abadc..5afe74db18 100644 --- a/osu.Game/Graphics/Containers/SectionsContainer.cs +++ b/osu.Game/Graphics/Containers/SectionsContainer.cs @@ -178,7 +178,10 @@ namespace osu.Game.Graphics.Containers { base.UpdateAfterChildren(); - float headerH = (ExpandableHeader?.LayoutSize.Y ?? 0) + (FixedHeader?.LayoutSize.Y ?? 0); + float fixedHeaderSize = (FixedHeader?.LayoutSize.Y ?? 0); + float expandableHeaderSize = ExpandableHeader?.LayoutSize.Y ?? 0; + + float headerH = expandableHeaderSize + fixedHeaderSize; float footerH = Footer?.LayoutSize.Y ?? 0; if (headerH != headerHeight || footerH != footerHeight) @@ -200,13 +203,13 @@ namespace osu.Game.Graphics.Containers if (ExpandableHeader != null && FixedHeader != null) { - float offset = Math.Min(ExpandableHeader.LayoutSize.Y, currentScroll); + float offset = Math.Min(expandableHeaderSize, currentScroll); ExpandableHeader.Y = -offset; - FixedHeader.Y = -offset + ExpandableHeader.LayoutSize.Y; + FixedHeader.Y = -offset + expandableHeaderSize; } - headerBackgroundContainer.Height = (ExpandableHeader?.LayoutSize.Y ?? 0) + (FixedHeader?.LayoutSize.Y ?? 0); + headerBackgroundContainer.Height = expandableHeaderSize + fixedHeaderSize; headerBackgroundContainer.Y = ExpandableHeader?.Y ?? 0; var smallestSectionHeight = Children.Count > 0 ? Children.Min(d => d.Height) : 0; @@ -216,7 +219,7 @@ namespace osu.Game.Graphics.Containers // but the 5% can't be bigger than our smallest section height, otherwise it won't get selected correctly float sectionOrContent = Math.Min(smallestSectionHeight / 2.0f, scrollContainer.DisplayableContent * 0.05f); - float scrollOffset = (FixedHeader?.LayoutSize.Y ?? 0) + scrollContainer.DisplayableContent * scroll_target_multiplier + sectionOrContent; + float scrollOffset = fixedHeaderSize + scrollContainer.DisplayableContent * scroll_target_multiplier + sectionOrContent; if (Precision.AlmostBigger(0, scrollContainer.Current)) SelectedSection.Value = lastClickedSection as T ?? Children.FirstOrDefault(); From c650cbd2a70f1f333d2fc3b159a1ca10ace61fca Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 21 Jan 2021 14:56:10 +0900 Subject: [PATCH 11/16] Rename variable to something slightly better --- osu.Game/Graphics/Containers/SectionsContainer.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Graphics/Containers/SectionsContainer.cs b/osu.Game/Graphics/Containers/SectionsContainer.cs index 5afe74db18..87c3007ca4 100644 --- a/osu.Game/Graphics/Containers/SectionsContainer.cs +++ b/osu.Game/Graphics/Containers/SectionsContainer.cs @@ -217,9 +217,9 @@ namespace osu.Game.Graphics.Containers // scroll offset is our fixed header height if we have it plus 20% of content height // plus 5% to fix floating point errors and to not have a section instantly unselect when scrolling upwards // but the 5% can't be bigger than our smallest section height, otherwise it won't get selected correctly - float sectionOrContent = Math.Min(smallestSectionHeight / 2.0f, scrollContainer.DisplayableContent * 0.05f); + float scrollIntoSectionAmount = Math.Min(smallestSectionHeight / 2.0f, scrollContainer.DisplayableContent * 0.05f); - float scrollOffset = fixedHeaderSize + scrollContainer.DisplayableContent * scroll_target_multiplier + sectionOrContent; + float scrollOffset = fixedHeaderSize + scrollContainer.DisplayableContent * scroll_target_multiplier + scrollIntoSectionAmount; if (Precision.AlmostBigger(0, scrollContainer.Current)) SelectedSection.Value = lastClickedSection as T ?? Children.FirstOrDefault(); From 8853ac04d97e2419da6b4f2db2a464d680173d3b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 21 Jan 2021 15:08:36 +0900 Subject: [PATCH 12/16] Rename some variable and add xmldoc for scroll centre position --- osu.Game/Graphics/Containers/SectionsContainer.cs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/osu.Game/Graphics/Containers/SectionsContainer.cs b/osu.Game/Graphics/Containers/SectionsContainer.cs index 87c3007ca4..6ed161fa77 100644 --- a/osu.Game/Graphics/Containers/SectionsContainer.cs +++ b/osu.Game/Graphics/Containers/SectionsContainer.cs @@ -108,7 +108,10 @@ namespace osu.Game.Graphics.Containers private float? lastKnownScroll; - private const float scroll_target_multiplier = 0.2f; + /// + /// The percentage of the container to consider the centre-point for deciding the active section (and scrolling to a requested section). + /// + private const float scroll_y_centre = 0.2f; public SectionsContainer() { @@ -144,7 +147,7 @@ namespace osu.Game.Graphics.Containers public void ScrollTo(Drawable section) { lastClickedSection = section; - scrollContainer.ScrollTo(scrollContainer.GetChildPosInContent(section) - scrollContainer.DisplayableContent * scroll_target_multiplier - (FixedHeader?.BoundingBox.Height ?? 0)); + scrollContainer.ScrollTo(scrollContainer.GetChildPosInContent(section) - scrollContainer.DisplayableContent * scroll_y_centre - (FixedHeader?.BoundingBox.Height ?? 0)); } public void ScrollToTop() => scrollContainer.ScrollTo(0); @@ -217,9 +220,9 @@ namespace osu.Game.Graphics.Containers // scroll offset is our fixed header height if we have it plus 20% of content height // plus 5% to fix floating point errors and to not have a section instantly unselect when scrolling upwards // but the 5% can't be bigger than our smallest section height, otherwise it won't get selected correctly - float scrollIntoSectionAmount = Math.Min(smallestSectionHeight / 2.0f, scrollContainer.DisplayableContent * 0.05f); + float selectionLenienceAboveSection = Math.Min(smallestSectionHeight / 2.0f, scrollContainer.DisplayableContent * 0.05f); - float scrollOffset = fixedHeaderSize + scrollContainer.DisplayableContent * scroll_target_multiplier + scrollIntoSectionAmount; + float scrollCentre = fixedHeaderSize + scrollContainer.DisplayableContent * scroll_y_centre + selectionLenienceAboveSection; if (Precision.AlmostBigger(0, scrollContainer.Current)) SelectedSection.Value = lastClickedSection as T ?? Children.FirstOrDefault(); @@ -228,7 +231,7 @@ namespace osu.Game.Graphics.Containers else { SelectedSection.Value = Children - .TakeWhile(section => scrollContainer.GetChildPosInContent(section) - currentScroll - scrollOffset <= 0) + .TakeWhile(section => scrollContainer.GetChildPosInContent(section) - currentScroll - scrollCentre <= 0) .LastOrDefault() ?? Children.FirstOrDefault(); } } From bfabb1fdea25412c24088185fc0ed2c3e8a7f40c Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 22 Jan 2021 16:50:22 +0900 Subject: [PATCH 13/16] Change offset value to 10% --- osu.Game/Graphics/Containers/SectionsContainer.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Graphics/Containers/SectionsContainer.cs b/osu.Game/Graphics/Containers/SectionsContainer.cs index 6ed161fa77..16bc6e4fc5 100644 --- a/osu.Game/Graphics/Containers/SectionsContainer.cs +++ b/osu.Game/Graphics/Containers/SectionsContainer.cs @@ -110,8 +110,8 @@ namespace osu.Game.Graphics.Containers /// /// The percentage of the container to consider the centre-point for deciding the active section (and scrolling to a requested section). - /// - private const float scroll_y_centre = 0.2f; + /// + private const float scroll_y_centre = 0.1f; public SectionsContainer() { From a5f7ca485bfeeb38d88f113c6afe1bba6c49ba3c Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 22 Jan 2021 16:53:31 +0900 Subject: [PATCH 14/16] Fix unintended xmldoc tag edit --- osu.Game/Graphics/Containers/SectionsContainer.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Graphics/Containers/SectionsContainer.cs b/osu.Game/Graphics/Containers/SectionsContainer.cs index 16bc6e4fc5..741fc17695 100644 --- a/osu.Game/Graphics/Containers/SectionsContainer.cs +++ b/osu.Game/Graphics/Containers/SectionsContainer.cs @@ -110,7 +110,7 @@ namespace osu.Game.Graphics.Containers /// /// The percentage of the container to consider the centre-point for deciding the active section (and scrolling to a requested section). - /// + /// private const float scroll_y_centre = 0.1f; public SectionsContainer() From 61fcb486a8c7aea90c40b2c9f881fcc295f7b8de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 22 Jan 2021 19:47:38 +0100 Subject: [PATCH 15/16] Trim unnecessary parentheses --- osu.Game/Graphics/Containers/SectionsContainer.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Graphics/Containers/SectionsContainer.cs b/osu.Game/Graphics/Containers/SectionsContainer.cs index 741fc17695..624ec70cb6 100644 --- a/osu.Game/Graphics/Containers/SectionsContainer.cs +++ b/osu.Game/Graphics/Containers/SectionsContainer.cs @@ -181,7 +181,7 @@ namespace osu.Game.Graphics.Containers { base.UpdateAfterChildren(); - float fixedHeaderSize = (FixedHeader?.LayoutSize.Y ?? 0); + float fixedHeaderSize = FixedHeader?.LayoutSize.Y ?? 0; float expandableHeaderSize = ExpandableHeader?.LayoutSize.Y ?? 0; float headerH = expandableHeaderSize + fixedHeaderSize; From f3192877fee57db6f29d84faa208d10d1bdf41b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 22 Jan 2021 19:48:33 +0100 Subject: [PATCH 16/16] Update outdated comment --- osu.Game/Graphics/Containers/SectionsContainer.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Graphics/Containers/SectionsContainer.cs b/osu.Game/Graphics/Containers/SectionsContainer.cs index 624ec70cb6..8ab146efe7 100644 --- a/osu.Game/Graphics/Containers/SectionsContainer.cs +++ b/osu.Game/Graphics/Containers/SectionsContainer.cs @@ -217,7 +217,7 @@ namespace osu.Game.Graphics.Containers var smallestSectionHeight = Children.Count > 0 ? Children.Min(d => d.Height) : 0; - // scroll offset is our fixed header height if we have it plus 20% of content height + // scroll offset is our fixed header height if we have it plus 10% of content height // plus 5% to fix floating point errors and to not have a section instantly unselect when scrolling upwards // but the 5% can't be bigger than our smallest section height, otherwise it won't get selected correctly float selectionLenienceAboveSection = Math.Min(smallestSectionHeight / 2.0f, scrollContainer.DisplayableContent * 0.05f);