Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add scrollable beatmap list #252

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Add scrollable beatmap list #252

wants to merge 10 commits into from

Conversation

HelloYeew
Copy link
Owner

@HelloYeew HelloYeew commented Mar 22, 2023

Just a branch that's not PR for a long time now....

This design based from an arcade game and just scroll through or click to select the song.

CleanShot.2023-03-22.at.09.44.09.mp4

There maybe a problem here...

  • The scroll container's topBox and bottomBox is not draw based on screen and it need to be... (Due to it cannot use Axes.Both on RelativeSizeAxes)
  • The MusicPlayer behavior since the song selection screen is just suspended so it will still scroll to the highlight point despite not in the screen (fixed)
  • Deadly laser on the sun when quickly scrolling through song selection

TODO

  • More review in Discord

Copy link
Collaborator

@Game4all Game4all left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code is OK for now

RelativeSizeAxes = Axes.Both,
Children = new Drawable[]
{
dummyBox = new Container()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure dummyBox is right? am open to bikeshedding here, anything but "dummy" please

@@ -24,7 +24,7 @@ public partial class BeatmapSetCard : CompositeDrawable
[Resolved]
private MaisimConfigManager gameConfig { get; set; }

private readonly BeatmapSet beatmapSet;
protected internal readonly BeatmapSet BeatmapSet;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure about the internal accessor here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants