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

Improve visual appearance of mania selection blueprints #30512

Merged
merged 14 commits into from
Nov 15, 2024

Conversation

bdach
Copy link
Collaborator

@bdach bdach commented Nov 5, 2024

Been bothering me for a long while now.

This isn't by all means supposed to be the end product, it's 80% of the target result in my mind in 20% of the time. Placement blueprints are all unchanged because attempting to access anything skinnable in the placement blueprint, before there is a DHO placed in an actual column, is hell. So I'm leaving that for later.

before after
osu_2024-11-05_15-30-51 osu_2024-11-05_15-33-02
osu_2024-11-05_15-30-59 osu_2024-11-05_15-33-09
osu_2024-11-05_15-31-07 osu_2024-11-05_15-33-16
osu_2024-11-05_15-31-15 osu_2024-11-05_15-33-24
osu_2024-11-05_15-31-25 osu_2024-11-05_15-33-32
osu_2024-11-05_15-32-11 osu_2024-11-05_15-33-55

@bdach bdach changed the title Improve visual appearance mania selection blueprints Improve visual appearance of mania selection blueprints Nov 5, 2024
@peppy
Copy link
Member

peppy commented Nov 6, 2024

Hmm, just going by screenshots, the case of the DDR skin gets a bit dicey because you can no longer easily see which tick the object is on. Also I think some of the blueprint lines are too thick with the new styling.

I think it's maybe okay to draw boxes as you have, but the actual placement line likely needs to still exist in some way. Unless we want to leave it up to users to pick a good "editor" skin and not worry about this at all.

@bdach
Copy link
Collaborator Author

bdach commented Nov 6, 2024

the case of the DDR skin gets a bit dicey because you can no longer easily see which tick the object is on

Looking at a few screenshots of other games that use DDR style sprites, the editors seem to either anchor notes to centre or use a grid style instead. That is not something I can easily implement. Maybe I shouldn't just bother with trying to display circles on DDR style skins and just use boxes everywhere?

Also I think some of the blueprint lines are too thick with the new styling

The thickness was chosen such that the selection markers on classic and triangles skins are completely filled. If you're fine with gaps showing on the inside on either or both, I can reduce it.

@smoogipoo
Copy link
Contributor

just use boxes everywhere

Would agree with this.

@peppy
Copy link
Member

peppy commented Nov 6, 2024

Just using boxes seems like a good starting point yeah.

@pull-request-size pull-request-size bot added size/L and removed size/M labels Nov 7, 2024
@bdach

This comment was marked as outdated.

@peppy
Copy link
Member

peppy commented Nov 7, 2024

I was going to suggest a gradient but it maybe doesn't look that amazing. Perhaps a solid line for the bottom part of the rectangle and a different colour or less-opaque line for the remaining three sides would feel better..

@bdach
Copy link
Collaborator Author

bdach commented Nov 7, 2024

Something like these, then?

osu_2024-11-07_13-38-54
osu_2024-11-07_13-39-06
osu_2024-11-07_13-39-15
osu_2024-11-07_13-39-26
osu_2024-11-07_13-39-35
osu_2024-11-07_13-39-47

@peppy peppy self-requested a review November 11, 2024 15:05
@peppy
Copy link
Member

peppy commented Nov 11, 2024

This is the best I've come up with so far, but it's still not good:

diff --git a/osu.Game.Rulesets.Mania/Edit/Blueprints/Components/EditBodyPiece.cs b/osu.Game.Rulesets.Mania/Edit/Blueprints/Components/EditBodyPiece.cs
index 6a12ec5088..652baccaa6 100644
--- a/osu.Game.Rulesets.Mania/Edit/Blueprints/Components/EditBodyPiece.cs
+++ b/osu.Game.Rulesets.Mania/Edit/Blueprints/Components/EditBodyPiece.cs
@@ -3,21 +3,42 @@
 
 using osu.Framework.Allocation;
 using osu.Framework.Graphics;
+using osu.Framework.Graphics.Containers;
+using osu.Framework.Graphics.Shapes;
 using osu.Game.Graphics;
+using osu.Game.Graphics.Backgrounds;
 using osu.Game.Rulesets.Mania.Skinning.Default;
+using osuTK.Graphics;
 
 namespace osu.Game.Rulesets.Mania.Edit.Blueprints.Components
 {
-    public partial class EditBodyPiece : DefaultBodyPiece
+    public partial class EditBodyPiece : CompositeDrawable
     {
+        private readonly Container border;
+
+        public EditBodyPiece()
+        {
+            InternalChildren = new Drawable[]
+            {
+                border = new Container
+                {
+                    RelativeSizeAxes = Axes.Both,
+                    Masking = true,
+                    BorderThickness = 3,
+                    Child = new Box
+                    {
+                        RelativeSizeAxes = Axes.Both,
+                        Alpha = 0,
+                        AlwaysPresent = true,
+                    },
+                },
+            };
+        }
+
         [BackgroundDependencyLoader]
         private void load(OsuColour colours)
         {
-            AccentColour.Value = colours.Yellow;
-
-            Background.Alpha = 0.5f;
+            border.BorderColour = colours.YellowDarker;
         }
-
-        protected override Drawable CreateForeground() => base.CreateForeground().With(d => d.Alpha = 0);
     }
 }
diff --git a/osu.Game.Rulesets.Mania/Edit/Blueprints/Components/EditHoldNoteEndPiece.cs b/osu.Game.Rulesets.Mania/Edit/Blueprints/Components/EditHoldNoteEndPiece.cs
index 285226e8b4..d4b61b4661 100644
--- a/osu.Game.Rulesets.Mania/Edit/Blueprints/Components/EditHoldNoteEndPiece.cs
+++ b/osu.Game.Rulesets.Mania/Edit/Blueprints/Components/EditHoldNoteEndPiece.cs
@@ -27,9 +27,6 @@ private void load()
         {
             Height = DefaultNotePiece.NOTE_HEIGHT;
 
-            CornerRadius = 5;
-            Masking = true;
-
             InternalChild = new EditNotePiece
             {
                 RelativeSizeAxes = Axes.Both,
diff --git a/osu.Game.Rulesets.Mania/Edit/Blueprints/Components/EditNotePiece.cs b/osu.Game.Rulesets.Mania/Edit/Blueprints/Components/EditNotePiece.cs
index 3f15515ba6..1f9f26b5e2 100644
--- a/osu.Game.Rulesets.Mania/Edit/Blueprints/Components/EditNotePiece.cs
+++ b/osu.Game.Rulesets.Mania/Edit/Blueprints/Components/EditNotePiece.cs
@@ -2,12 +2,10 @@
 // See the LICENCE file in the repository root for full licence text.
 
 using osu.Framework.Allocation;
-using osu.Framework.Extensions.Color4Extensions;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Containers;
 using osu.Framework.Graphics.Shapes;
 using osu.Game.Graphics;
-using osu.Game.Rulesets.Mania.Skinning.Default;
 using osu.Game.Rulesets.Mania.UI;
 using osu.Game.Rulesets.UI.Scrolling;
 using osuTK;
@@ -17,24 +15,21 @@ namespace osu.Game.Rulesets.Mania.Edit.Blueprints.Components
 {
     public partial class EditNotePiece : CompositeDrawable
     {
+        private readonly Container border;
+        private readonly Box box;
+
         [Resolved]
         private Column? column { get; set; }
 
         public EditNotePiece()
         {
-            Masking = true;
-            CornerRadius = 5;
-            Height = DefaultNotePiece.NOTE_HEIGHT;
-
             InternalChildren = new Drawable[]
             {
-                new Container
+                border = new Container
                 {
                     RelativeSizeAxes = Axes.Both,
                     Masking = true,
-                    CornerRadius = 5,
-                    BorderThickness = 5,
-                    BorderColour = Color4.White.Opacity(0.7f),
+                    BorderThickness = 3,
                     Child = new Box
                     {
                         RelativeSizeAxes = Axes.Both,
@@ -42,10 +37,10 @@ public EditNotePiece()
                         AlwaysPresent = true,
                     },
                 },
-                new Box
+                box = new Box
                 {
                     RelativeSizeAxes = Axes.X,
-                    Height = 10,
+                    Height = 3,
                     Anchor = Anchor.BottomCentre,
                     Origin = Anchor.BottomCentre,
                 },
@@ -55,7 +50,8 @@ public EditNotePiece()
         [BackgroundDependencyLoader]
         private void load(OsuColour colours)
         {
-            Colour = colours.Yellow;
+            border.BorderColour = colours.YellowDark;
+            box.Colour = colours.YellowLight;
         }
 
         protected override void Update()
diff --git a/osu.Game.Rulesets.Mania/Edit/Blueprints/HoldNoteSelectionBlueprint.cs b/osu.Game.Rulesets.Mania/Edit/Blueprints/HoldNoteSelectionBlueprint.cs
index 4e0b0cea6c..8c8d58654e 100644
--- a/osu.Game.Rulesets.Mania/Edit/Blueprints/HoldNoteSelectionBlueprint.cs
+++ b/osu.Game.Rulesets.Mania/Edit/Blueprints/HoldNoteSelectionBlueprint.cs
@@ -47,6 +47,12 @@ private void load()
         {
             InternalChildren = new Drawable[]
             {
+                new EditBodyPiece
+                {
+                    RelativeSizeAxes = Axes.Both,
+                    Anchor = Anchor.BottomCentre,
+                    Origin = Anchor.BottomCentre,
+                },
                 head = new EditHoldNoteEndPiece
                 {
                     RelativeSizeAxes = Axes.X,
@@ -88,21 +94,6 @@ private void load()
                     },
                     DragEnded = () => changeHandler?.EndChange(),
                 },
-                new Container
-                {
-                    RelativeSizeAxes = Axes.Both,
-                    Masking = true,
-                    Anchor = Anchor.BottomCentre,
-                    Origin = Anchor.BottomCentre,
-                    BorderThickness = 1,
-                    BorderColour = colours.Yellow,
-                    Child = new Box
-                    {
-                        RelativeSizeAxes = Axes.Both,
-                        Alpha = 0,
-                        AlwaysPresent = true,
-                    }
-                }
             };
         }
 
  • Too busy (mostly due to the selection rectangle being too close and too similar and usually unnecessary, needs more thought and maybe removal from the playfield as others have proposed in the past, only showing when transform mode is enabled)
  • Argon draw height doesn't match properly and looks weird
  • I dunno, stable just feels better using tinting rather than this blueprint crap. Maybe we just tint again?

I get that this sounds negative but I'm just laying out my thoughts here. Don't close this just yet because I agree that what we have is not good and am willing to find a compromise. These remaining issues are likely things for the future and with the patch I have (and hopefully some further refinements) I think this can move forward.

@bdach
Copy link
Collaborator Author

bdach commented Nov 12, 2024

I've applied the patch verbatim.

stable just feels better using tinting rather than this blueprint crap. Maybe we just tint again?

For selection blueprints we probably could. Placement is another beast entirely, it may take me a good two or ten days to figure out how on earth to fish out a correctly looking skinnable to put in the placement blueprint from all of the column configuration sprawl.

@peppy
Copy link
Member

peppy commented Nov 12, 2024

One remaining issue if we go forward with this change is that the alignment of placement notes look incorrect now (while placement holds are okay seemingly):

2024-11-13 03 30 29@2x

Also they used to be much more present, which is probably what we still want:

2024-11-13 03 31 26@2x

@bdach
Copy link
Collaborator Author

bdach commented Nov 15, 2024

Have adjusted further:

2024-11-15.10-45-28.mp4

Also fixed the selection blueprint being broken on upscroll in 0c4c9bd.

@peppy peppy enabled auto-merge November 15, 2024 10:23
@peppy peppy disabled auto-merge November 15, 2024 10:35
@peppy peppy merged commit d28d54f into ppy:master Nov 15, 2024
9 checks passed
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.

3 participants