Rewrite BeatmapCarousel with ModelView implementation #1697

peppy · 2017-12-14T11:41:44Z

The old one was horrible, so I rewrote it with better and tests.

osu.Game.Tests/Visual/TestCasePlaySongSelect.cs

        protected override IReadOnlyDependencyContainer CreateLocalDependencies(IReadOnlyDependencyContainer parent) => dependencies = new DependencyContainer(parent);

        [BackgroundDependencyLoader]
-        private void load()
+        private void load(BeatmapManager baseMaanger)


… lookup failure

Also exposes SelectedBeatmapSet.

Tests accompany of course

oops

Also fixes a lot of bad interactions and simplifies further.

… level Was causing BeatmapSet's Set to run *after* newer events were received.

smoogipoo · 2017-12-18T01:26:23Z

[ERROR] FATAL UNHANDLED EXCEPTION: System.InvalidOperationException: Sequence contains no elements
  at System.Linq.Enumerable.First[TSource] (System.Collections.Generic.IEnumerable`1[T] source) [0x0000b] in /Users/builder/data/lanes/4992/mono-mac-sdk/external/bockbuild/builds/mono-x64/external/corefx/src/System.Linq/src/System/Linq/First.cs:18 
  at osu.Game.Screens.Select.BeatmapCarousel.SelectNext (System.Int32 direction, System.Boolean skipDifficulties) [0x00011] in /Users/smoogipoo/Repos/Temp/temp-osu/osu.Game/Screens/Select/BeatmapCarousel.cs:180 
  at osu.Game.Screens.Select.BeatmapCarousel.OnKeyDown (osu.Framework.Input.InputState state, osu.Framework.Input.KeyDownEventArgs args) [0x0004f] in /Users/smoogipoo/Repos/Temp/temp-osu/osu.Game/Screens/Select/BeatmapCarousel.cs:463 
  at osu.Framework.Graphics.Drawable.TriggerOnKeyDown (osu.Framework.Input.InputState screenSpaceState, osu.Framework.Input.KeyDownEventArgs args) [0x00000] in /Users/smoogipoo/Repos/Temp/temp-osu/osu-framework/osu.Framework/Graphics/Drawable.cs:1802 
  at osu.Framework.Input.InputManager+<>c__DisplayClass62_0.<PropagateKeyDown>b__0 (osu.Framework.Graphics.Drawable target) [0x00000] in /Users/smoogipoo/Repos/Temp/temp-osu/osu-framework/osu.Framework/Input/InputManager.cs:688 
  at System.Linq.Enumerable.TryGetFirst[TSource] (System.Collections.Generic.IEnumerable`1[T] source, System.Func`2[T,TResult] predicate, System.Boolean& found) [0x0003f] in /Users/builder/data/lanes/4992/mono-mac-sdk/external/bockbuild/builds/mono-x64/external/corefx/src/System.Linq/src/System/Linq/First.cs:107 
  at System.Linq.Enumerable.FirstOrDefault[TSource] (System.Collections.Generic.IEnumerable`1[T] source, System.Func`2[T,TResult] predicate) [0x00000] in /Users/builder/data/lanes/4992/mono-mac-sdk/external/bockbuild/builds/mono-x64/external/corefx/src/System.Linq/src/System/Linq/First.cs:46 
  at osu.Framework.Input.InputManager.PropagateKeyDown (System.Collections.Generic.IEnumerable`1[T] drawables, osu.Framework.Input.InputState state, osu.Framework.Input.KeyDownEventArgs args) [0x00015] in /Users/smoogipoo/Repos/Temp/temp-osu/osu-framework/osu.Framework/Input/InputManager.cs:688 
  at osu.Framework.Input.InputManager.handleKeyDown (osu.Framework.Input.InputState state, OpenTK.Input.Key key, System.Boolean repeat) [0x0002b] in /Users/smoogipoo/Repos/Temp/temp-osu/osu-framework/osu.Framework/Input/InputManager.cs:676

smoogipoo · 2017-12-18T01:51:17Z

954bc77#diff-17cb1f4d2c61ec9f4581df7ddae8f3adL63 Unused variable (why does resharper not pick up on this?).

osu.Game/Screens/Select/Carousel/CarouselItem.cs

+
+        public IReadOnlyList<CarouselItem> Children => InternalChildren;
+
+        protected List<CarouselItem> InternalChildren { get; set; }


osu.Game/Screens/Select/Carousel/CarouselGroup.cs

+
+        protected virtual void ChildItemStateChanged(CarouselItem item, CarouselItemState value)
+        {
+            // todo: check state of selected item.


…ulty using context menu

smoogipoo · 2017-12-18T03:55:45Z

I'm happy to get this merged in for now. I don't see anything obviously wrong with this for now, besides the jiggling due to the removal of the animation skipping. We can fix up any further issues later.

👍

Aergwyn reviewed Dec 14, 2017

View reviewed changes

peppy force-pushed the song-select-state-simplification branch 2 times, most recently from 756532c to b34e063 Compare December 14, 2017 12:02

peppy added 19 commits December 15, 2017 14:36

Initial carousel infrastructue changes

78dd975

More clean-ups and event bindings

99b0014

Make GetWorkingBeatmap return a sane default rather than exception on…

1146ba0

… lookup failure

Rename weird config setting

b929832

Cleanup and basic TestCase

1b85952

Clean up tests some more

ec4f99c

Get ready for more tests

48f30d2

Add testing and fix filtering only some difficulties

8646d5d

Fix random and add tests

5cbb9b9

Also exposes SelectedBeatmapSet.

More renaming

7814b2d

Add support for adding/removing items

b4b2f12

Tests accompany of course

Add sorting support

67f0597

Fix typo

2817ed0

Fix incorrect line endings

48e53a7

Shortcut non-visible panels to avoid adding as drawables

2e3332e

Avoid multiple access to selectedBeatmap during y position computation

192ceb5

Add back ctrl-enter autoplay shortcut

ed5b6cc

SongSelect tidying

59d5127

Allow tests to work with a variable number of beatmap sets loaded

e6cac4a

peppy force-pushed the song-select-state-simplification branch from bbd1125 to 04304c2 Compare December 15, 2017 05:58

peppy added 2 commits December 15, 2017 18:38

Move DrawableCarouselBeatmap initialisation to BDL

acfdd32

oops

Simplify StarCounter and SpriteIcon

fd9d900

peppy force-pushed the song-select-state-simplification branch from 04304c2 to fd9d900 Compare December 15, 2017 09:40

Add tests for (and fix) removal of last item in carousel

a8a2c23

peppy force-pushed the song-select-state-simplification branch from 82a6784 to a8a2c23 Compare December 15, 2017 15:33

peppy added 2 commits December 16, 2017 16:14

Add ToString() overrides on many classes to make debugging easier

49ce42d

Ensure correct selection after deletion of currently selected

3c40666

Also fixes a lot of bad interactions and simplifies further.

peppy added 13 commits December 17, 2017 03:04

Remove logging

d27047f

Remove unnecessary capture

c02ce16

Improve performance with large numbers of panels visible

5d7413f

Greatly improve performance when many hidden panels are on-screen

54cc6fa

Resolve scroll animation/position issues

19643ba

Re-fix null selection

942054a

Fix event handling from outside carousel being scheduled at the wrong…

30a1572

… level Was causing BeatmapSet's Set to run *after* newer events were received.

Update test case to handle scheduled removal

3759c39

Preload drawables to force asynchronous construction

482941b

Avoid redundant IndexOf calls

c102885

Move more overhead from ctor to BDL

dfd7787

Indicate unused variable

954bc77

Update framework

d6ba53a

peppy changed the title ~~BeatmapCarousel revamp~~ Rewrite BeatmapCarousel with ModelView implementation Dec 18, 2017

peppy added 2 commits December 18, 2017 11:13

Make more things private

b21c220

Remove unused variable

5bfb6d1

smoogipoo reviewed Dec 18, 2017

View reviewed changes

osu.Game/Screens/Select/Carousel/CarouselItem.cs Outdated

public IReadOnlyList<CarouselItem> Children => InternalChildren;

protected List<CarouselItem> InternalChildren { get; set; }

This comment was marked as off-topic.

Sign in to view

smoogipoo reviewed Dec 18, 2017

View reviewed changes

osu.Game/Screens/Select/Carousel/CarouselGroup.cs Outdated

protected virtual void ChildItemStateChanged(CarouselItem item, CarouselItemState value)

{

// todo: check state of selected item.

This comment was marked as off-topic.

Sign in to view

peppy added 6 commits December 18, 2017 11:25

Remove todo

4e46565

Fix incorrect flush logic when starting play from non-selected diffic…

4c1f005

…ulty using context menu

Add filter checks to difficulty selection

7173829

Move children to CarouselGroup

b2cd32e

Simplify drawable removal logic

23e014b

Remove animating skipping and reorder file a bit

6121cd3

smoogipoo approved these changes Dec 18, 2017

View reviewed changes

smoogipoo merged commit dd5e082 into ppy:master Dec 18, 2017

peppy deleted the song-select-state-simplification branch January 11, 2018 11:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rewrite BeatmapCarousel with ModelView implementation #1697

Rewrite BeatmapCarousel with ModelView implementation #1697

peppy commented Dec 14, 2017

This comment was marked as off-topic.

smoogipoo commented Dec 18, 2017

smoogipoo commented Dec 18, 2017

This comment was marked as off-topic.

This comment was marked as off-topic.

smoogipoo commented Dec 18, 2017


		public IReadOnlyList<CarouselItem> Children => InternalChildren;

		protected List<CarouselItem> InternalChildren { get; set; }

Rewrite BeatmapCarousel with ModelView implementation #1697

Rewrite BeatmapCarousel with ModelView implementation #1697

Conversation

peppy commented Dec 14, 2017

This comment was marked as off-topic.

smoogipoo commented Dec 18, 2017

smoogipoo commented Dec 18, 2017

This comment was marked as off-topic.

This comment was marked as off-topic.

smoogipoo commented Dec 18, 2017