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

Implement redesigned health bar display for "Argon" skin #24980

Merged
merged 21 commits into from
Oct 3, 2023

Conversation

frenzibyte
Copy link
Member

@frenzibyte frenzibyte commented Sep 30, 2023

Note that this only adds an implementation and uses it in a test scene, it is not integrated in the "Argon" skin yet.

Visual overview:

CleanShot.2023-10-01.at.00.57.29.mp4

I haven't tried to perfectly implement the source design of the health bar, as the way it's designed depends on adding a "blurred inner shadow", with properties that I'm not sure how I can specify accurately in Path. But I've tried to mimic it as much as possible, and I'm happy with the result.

The health bar also has a cap regardless of whether there's a red bar next to it, unlike in the source design. Mainly because we don't support Paths with no caps and I'm not really sure how to implement it (masking may be used in this situation, but not sure how that would look in the curved area of the health bar).

Here's a side-to-side comparison for reference:

CleanShot 2023-10-01 at 01 22 56@2x

@frenzibyte frenzibyte requested a review from a team September 30, 2023 22:23
@frenzibyte frenzibyte self-assigned this Sep 30, 2023
@frenzibyte
Copy link
Member Author

Tests are failing due to SkinDeserialisationTest not being updated with the new component's metadata. Not entirely sure what the steps are to properly cover it (cc @peppy).

@peppy
Copy link
Member

peppy commented Oct 1, 2023

Tests are failing due to SkinDeserialisationTest not being updated with the new component's metadata. Not entirely sure what the steps are to properly cover it (cc @peppy).

Take the last .osk, add your new element, and export a new .osk.

@peppy peppy self-requested a review October 1, 2023 04:21
@peppy
Copy link
Member

peppy commented Oct 1, 2023

Here's a jank diff for anyone looking to review this PR:

diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSkinnableHealthDisplay.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSkinnableHealthDisplay.cs
index 4d8ddcd581..687b06bb88 100644
--- a/osu.Game.Tests/Visual/Gameplay/TestSceneSkinnableHealthDisplay.cs
+++ b/osu.Game.Tests/Visual/Gameplay/TestSceneSkinnableHealthDisplay.cs
@@ -12,6 +12,7 @@
 using osu.Game.Rulesets.Scoring;
 using osu.Game.Screens.Play.HUD;
 using osu.Game.Skinning;
+using osuTK;
 
 namespace osu.Game.Tests.Visual.Gameplay
 {
@@ -20,7 +21,7 @@ public partial class TestSceneSkinnableHealthDisplay : SkinnableHUDComponentTest
         [Cached(typeof(HealthProcessor))]
         private HealthProcessor healthProcessor = new DrainingHealthProcessor(0);
 
-        protected override Drawable CreateArgonImplementation() => new ArgonHealthDisplay();
+        protected override Drawable CreateArgonImplementation() => new ArgonHealthDisplay { Scale = new Vector2(2) };
         protected override Drawable CreateDefaultImplementation() => new DefaultHealthDisplay();
         protected override Drawable CreateLegacyImplementation() => new LegacyHealthDisplay();
 
@@ -62,4 +63,4 @@ public void TestHealthDisplayIncrementing()
             }, 3);
         }
     }
-}
\ No newline at end of file
+}
diff --git a/osu.Game/Tests/Visual/SkinnableTestScene.cs b/osu.Game/Tests/Visual/SkinnableTestScene.cs
index aab1b72990..9ec57a2506 100644
--- a/osu.Game/Tests/Visual/SkinnableTestScene.cs
+++ b/osu.Game/Tests/Visual/SkinnableTestScene.cs
@@ -40,7 +40,7 @@ public abstract partial class SkinnableTestScene : OsuGridTestScene, IStorageRes
         private GameHost host { get; set; }
 
         protected SkinnableTestScene()
-            : base(2, 3)
+            : base(1, 1)
         {
         }
 
@@ -66,11 +66,11 @@ protected void SetContents(Func<ISkin, Drawable> creationFunction)
             var beatmap = CreateBeatmapForSkinProvider();
 
             Cell(0).Child = createProvider(argonSkin, creationFunction, beatmap);
-            Cell(1).Child = createProvider(trianglesSkin, creationFunction, beatmap);
-            Cell(2).Child = createProvider(metricsSkin, creationFunction, beatmap);
-            Cell(3).Child = createProvider(legacySkin, creationFunction, beatmap);
-            Cell(4).Child = createProvider(specialSkin, creationFunction, beatmap);
-            Cell(5).Child = createProvider(oldSkin, creationFunction, beatmap);
+            // Cell(1).Child = createProvider(trianglesSkin, creationFunction, beatmap);
+            // Cell(2).Child = createProvider(metricsSkin, creationFunction, beatmap);
+            // Cell(3).Child = createProvider(legacySkin, creationFunction, beatmap);
+            // Cell(4).Child = createProvider(specialSkin, creationFunction, beatmap);
+            // Cell(5).Child = createProvider(oldSkin, creationFunction, beatmap);
         }
 
         protected IEnumerable<Drawable> CreatedDrawables => createdDrawables;
@@ -153,7 +153,7 @@ void updateSizing()
                     }
                 }
 
-                outlineBox.Alpha = autoSize ? 1 : 0;
+                outlineBox.Alpha = 0;
             }
         }
 

Comment on lines 233 to 240
protected override Color4 ColourAt(float position)
{
if (position <= 0.128f)
return Color4.White.Opacity(0.5f);

position -= 0.128f;
return Interpolation.ValueAt(Math.Clamp(position, 0f, 1f), Color4.White.Opacity(0.5f), Color4.Black.Opacity(0.5f), -0.75f, 1f, Easing.OutQuart);
}
Copy link
Member

Choose a reason for hiding this comment

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

what on earth is this abomination. i won't even get started on how weird this code is.

can you tidy it up? ie. i don't see why you are subtracting 0.128f if you're going to apply a weird curve from a negative value. you can just add this to the weird value.

also confirm that you meant 1f here as it will never reach 1f.

Copy link
Member Author

Choose a reason for hiding this comment

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

Removing position subtraction will probably change nothing in how jank this looks. The source design uses some sort of blurred inner shadow with a radius of 10px, and I'm not totally sure how this can be applied in here. The only way I can make this less jank would be by altering the interpolation start/end colours away from what they are in the source design, while achieving the same visual result.

Comment on lines 139 to 140
else if (healthBar.Alpha < 1)
healthBar.FadeIn(300, Easing.OutQuint);
Copy link
Member

Choose a reason for hiding this comment

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

This is wrong. It will cause a slower fade with more value changes. You probably want to move this to use a ContinuousDamp function in Update or similar.

@peppy
Copy link
Member

peppy commented Oct 1, 2023

To make this look semi-decent, I'd want the miss bar to be larger in radius (to obtain a more visually appealing glow, not applied in this diff), but attempting to do this breaks positioning:

diff --git a/osu.Game/Screens/Play/HUD/ArgonHealthDisplay.cs b/osu.Game/Screens/Play/HUD/ArgonHealthDisplay.cs
index e53553361e..d89dd36ad6 100644
--- a/osu.Game/Screens/Play/HUD/ArgonHealthDisplay.cs
+++ b/osu.Game/Screens/Play/HUD/ArgonHealthDisplay.cs
@@ -104,7 +104,8 @@ private void load()
                                 GlowColour = miss_bar_glow_colour,
                                 Blending = BlendingParameters.Additive,
                                 Alpha = 0f,
-                                PathRadius = 10f,
+                                PathRadius = 20f,
+                                GlowPortion = 0.75f,
                                 Vertices = vertices
                             },
                             healthBar = new BarPath
@@ -115,6 +116,7 @@ private void load()
                                 BarColour = health_bar_colour,
                                 GlowColour = health_bar_glow_colour,
                                 PathRadius = 10f,
+                                GlowPortion = 0.6f,
                                 Vertices = vertices
                             },
                         }
@@ -274,12 +276,14 @@ public Colour4 GlowColour
                 }
             }
 
+            public float GlowPortion { get; set; } = 0.6f;
+
             protected override Color4 ColourAt(float position)
             {
-                if (position >= 0.6f)
+                if (position >= GlowPortion)
                     return BarColour;
 
-                return Interpolation.ValueAt(position, Colour4.Black.Opacity(0.0f), GlowColour, 0.0, 0.6);
+                return Interpolation.ValueAt(position, Colour4.Black.Opacity(0.0f), GlowColour, 0.0, GlowPortion);
             }
         }
     }

@frenzibyte
Copy link
Member Author

Here's a jank diff for anyone looking to review this PR:

diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSkinnableHealthDisplay.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSkinnableHealthDisplay.cs
index 4d8ddcd581..687b06bb88 100644
--- a/osu.Game.Tests/Visual/Gameplay/TestSceneSkinnableHealthDisplay.cs
+++ b/osu.Game.Tests/Visual/Gameplay/TestSceneSkinnableHealthDisplay.cs
@@ -12,6 +12,7 @@
 using osu.Game.Rulesets.Scoring;
 using osu.Game.Screens.Play.HUD;
 using osu.Game.Skinning;
+using osuTK;
 
 namespace osu.Game.Tests.Visual.Gameplay
 {
@@ -20,7 +21,7 @@ public partial class TestSceneSkinnableHealthDisplay : SkinnableHUDComponentTest
         [Cached(typeof(HealthProcessor))]
         private HealthProcessor healthProcessor = new DrainingHealthProcessor(0);
 
-        protected override Drawable CreateArgonImplementation() => new ArgonHealthDisplay();
+        protected override Drawable CreateArgonImplementation() => new ArgonHealthDisplay { Scale = new Vector2(2) };
         protected override Drawable CreateDefaultImplementation() => new DefaultHealthDisplay();
         protected override Drawable CreateLegacyImplementation() => new LegacyHealthDisplay();
 
@@ -62,4 +63,4 @@ public void TestHealthDisplayIncrementing()
             }, 3);
         }
     }
-}
\ No newline at end of file
+}
diff --git a/osu.Game/Tests/Visual/SkinnableTestScene.cs b/osu.Game/Tests/Visual/SkinnableTestScene.cs
index aab1b72990..9ec57a2506 100644
--- a/osu.Game/Tests/Visual/SkinnableTestScene.cs
+++ b/osu.Game/Tests/Visual/SkinnableTestScene.cs
@@ -40,7 +40,7 @@ public abstract partial class SkinnableTestScene : OsuGridTestScene, IStorageRes
         private GameHost host { get; set; }
 
         protected SkinnableTestScene()
-            : base(2, 3)
+            : base(1, 1)
         {
         }
 
@@ -66,11 +66,11 @@ protected void SetContents(Func<ISkin, Drawable> creationFunction)
             var beatmap = CreateBeatmapForSkinProvider();
 
             Cell(0).Child = createProvider(argonSkin, creationFunction, beatmap);
-            Cell(1).Child = createProvider(trianglesSkin, creationFunction, beatmap);
-            Cell(2).Child = createProvider(metricsSkin, creationFunction, beatmap);
-            Cell(3).Child = createProvider(legacySkin, creationFunction, beatmap);
-            Cell(4).Child = createProvider(specialSkin, creationFunction, beatmap);
-            Cell(5).Child = createProvider(oldSkin, creationFunction, beatmap);
+            // Cell(1).Child = createProvider(trianglesSkin, creationFunction, beatmap);
+            // Cell(2).Child = createProvider(metricsSkin, creationFunction, beatmap);
+            // Cell(3).Child = createProvider(legacySkin, creationFunction, beatmap);
+            // Cell(4).Child = createProvider(specialSkin, creationFunction, beatmap);
+            // Cell(5).Child = createProvider(oldSkin, creationFunction, beatmap);
         }
 
         protected IEnumerable<Drawable> CreatedDrawables => createdDrawables;
@@ -153,7 +153,7 @@ void updateSizing()
                     }
                 }
 
-                outlineBox.Alpha = autoSize ? 1 : 0;
+                outlineBox.Alpha = 0;
             }
         }
 

Added better test scene to avoid this.

@peppy
Copy link
Member

peppy commented Oct 1, 2023

I've pushed changes to bring the animation and lighting to a place I feel is nice.

2023-10-02.02.08.42.mp4

Code quality still needs another pass or three.

@peppy peppy self-requested a review October 2, 2023 06:36
@peppy
Copy link
Member

peppy commented Oct 3, 2023

I've tidied this up and going to get it in as is. In order to get this in the next release, I'll likely push a follow-up PR to make the verticality adjustable via a setting, and use a value of 0 for the initial display.

But to make this work correctly, the component will need to be converted to support relative sizing on the X axis (probably should have supported it from the start, oh well).

@peppy peppy enabled auto-merge October 3, 2023 08:49
@bdach bdach disabled auto-merge October 3, 2023 09:26
@bdach bdach merged commit 844a3b6 into ppy:master Oct 3, 2023
@frenzibyte frenzibyte deleted the gameplay-hud-redesign/health-display branch November 26, 2023 22:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

3 participants