-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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 maximum dimensions limit to skinnable gameplay elements #24706
Changes from 10 commits
2c81fe5
6674c70
351081e
d286816
f182f57
96f12cf
9d17539
57dc76b
291a91b
a373d71
fc1a39e
f963a92
b823507
ab52268
922f6f3
8f5d1b5
8e16b1d
bd66285
1316403
c4fc419
ad86bf2
b1561b6
990c545
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,6 +9,7 @@ | |
using osu.Framework.Graphics.Textures; | ||
using osu.Game.Graphics; | ||
using osu.Game.Skinning; | ||
using osuTK; | ||
using osuTK.Graphics; | ||
|
||
namespace osu.Game.Rulesets.Taiko.Skinning.Legacy | ||
|
@@ -47,13 +48,13 @@ private void load(ISkinSource skin, OsuColour colours) | |
Anchor = Anchor.CentreRight, | ||
Origin = Anchor.CentreLeft, | ||
RelativeSizeAxes = Axes.Both, | ||
Texture = skin.GetTexture("taiko-roll-end", WrapMode.ClampToEdge, WrapMode.ClampToEdge), | ||
Texture = skin.GetTexture("taiko-roll-end", WrapMode.ClampToEdge, WrapMode.ClampToEdge)?.WithMaximumSize(new Vector2(128, 256)), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are these 256 based, not 128? Also adjusting this doesn't affect the test scene at all? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Uhh...9d17539 And yes, adjusting doesn't affect because the sprite is resized regardless of the texture, see #24706 (comment). |
||
FillMode = FillMode.Fit, | ||
}, | ||
body = new Sprite | ||
{ | ||
RelativeSizeAxes = Axes.Both, | ||
Texture = skin.GetTexture("taiko-roll-middle", WrapMode.ClampToEdge, WrapMode.ClampToEdge), | ||
Texture = skin.GetTexture("taiko-roll-middle", WrapMode.ClampToEdge, WrapMode.ClampToEdge)?.WithMaximumSize(new Vector2(2, 256)), | ||
}, | ||
headCircle = new LegacyCirclePiece | ||
{ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,73 @@ | ||
// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence. | ||
// See the LICENCE file in the repository root for full licence text. | ||
|
||
using System; | ||
using System.Collections.Generic; | ||
using System.Linq; | ||
using osu.Framework.Allocation; | ||
using osu.Framework.Graphics.Textures; | ||
using osu.Framework.Testing; | ||
using osu.Game.IO; | ||
using osu.Game.Rulesets; | ||
using osu.Game.Screens.Play; | ||
using osu.Game.Skinning; | ||
|
||
namespace osu.Game.Tests.Visual.Gameplay | ||
{ | ||
public partial class TestSceneGameplayElementDimensions : TestSceneAllRulesetPlayers | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What on earth is this naming.. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I...don't know? |
||
{ | ||
protected override IReadOnlyDependencyContainer CreateChildDependencies(IReadOnlyDependencyContainer parent) | ||
{ | ||
var dependencies = new DependencyContainer(base.CreateChildDependencies(parent)); | ||
|
||
// for now this only applies to legacy skins, as modern skins don't have texture-based gameplay elements yet. | ||
dependencies.CacheAs<ISkinSource>(new UpscaledLegacySkin(dependencies.Get<SkinManager>())); | ||
|
||
return dependencies; | ||
} | ||
|
||
protected override void AddCheckSteps() | ||
{ | ||
} | ||
|
||
protected override Player CreatePlayer(Ruleset ruleset) | ||
{ | ||
var player = base.CreatePlayer(ruleset); | ||
player.OnLoadComplete += _ => | ||
{ | ||
// this test scene focuses on gameplay elements, so let's hide the hud. | ||
var hudOverlay = player.ChildrenOfType<HUDOverlay>().Single(); | ||
hudOverlay.ShowHud.Value = false; | ||
hudOverlay.ShowHud.Disabled = true; | ||
}; | ||
return player; | ||
} | ||
|
||
private class UpscaledLegacySkin : DefaultLegacySkin, ISkinSource | ||
{ | ||
public UpscaledLegacySkin(IStorageResourceProvider resources) | ||
: base(resources) | ||
{ | ||
} | ||
|
||
public event Action? SourceChanged | ||
{ | ||
add { } | ||
remove { } | ||
} | ||
|
||
public override Texture? GetTexture(string componentName, WrapMode wrapModeS, WrapMode wrapModeT) | ||
{ | ||
var texture = base.GetTexture(componentName, wrapModeS, wrapModeT); | ||
|
||
if (texture != null) | ||
texture.ScaleAdjust /= 5f; | ||
|
||
return texture; | ||
} | ||
|
||
public ISkin? FindProvider(Func<ISkin, bool> lookupFunction) => this; | ||
public IEnumerable<ISkin> AllSources => new[] { this }; | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,23 +11,24 @@ | |
using osu.Framework.Graphics.Animations; | ||
using osu.Framework.Graphics.Sprites; | ||
using osu.Framework.Graphics.Textures; | ||
using osuTK; | ||
using static osu.Game.Skinning.SkinConfiguration; | ||
|
||
namespace osu.Game.Skinning | ||
{ | ||
public static partial class LegacySkinExtensions | ||
{ | ||
public static Drawable? GetAnimation(this ISkin? source, string componentName, bool animatable, bool looping, bool applyConfigFrameRate = false, string animationSeparator = "-", | ||
bool startAtCurrentTime = true, double? frameLength = null) | ||
=> source.GetAnimation(componentName, default, default, animatable, looping, applyConfigFrameRate, animationSeparator, startAtCurrentTime, frameLength); | ||
bool startAtCurrentTime = true, double? frameLength = null, Vector2? maxSize = null) | ||
=> source.GetAnimation(componentName, default, default, animatable, looping, applyConfigFrameRate, animationSeparator, startAtCurrentTime, frameLength, maxSize); | ||
|
||
public static Drawable? GetAnimation(this ISkin? source, string componentName, WrapMode wrapModeS, WrapMode wrapModeT, bool animatable, bool looping, bool applyConfigFrameRate = false, | ||
string animationSeparator = "-", bool startAtCurrentTime = true, double? frameLength = null) | ||
string animationSeparator = "-", bool startAtCurrentTime = true, double? frameLength = null, Vector2? maxSize = null) | ||
{ | ||
if (source == null) | ||
return null; | ||
|
||
var textures = GetTextures(source, componentName, wrapModeS, wrapModeT, animatable, animationSeparator, out var retrievalSource); | ||
var textures = GetTextures(source, componentName, wrapModeS, wrapModeT, animatable, animationSeparator, maxSize, out var retrievalSource); | ||
|
||
switch (textures.Length) | ||
{ | ||
|
@@ -53,7 +54,7 @@ public static partial class LegacySkinExtensions | |
} | ||
} | ||
|
||
public static Texture[] GetTextures(this ISkin? source, string componentName, WrapMode wrapModeS, WrapMode wrapModeT, bool animatable, string animationSeparator, out ISkin? retrievalSource) | ||
public static Texture[] GetTextures(this ISkin? source, string componentName, WrapMode wrapModeS, WrapMode wrapModeT, bool animatable, string animationSeparator, Vector2? maxSize, out ISkin? retrievalSource) | ||
{ | ||
retrievalSource = null; | ||
|
||
|
@@ -80,6 +81,9 @@ public static Texture[] GetTextures(this ISkin? source, string componentName, Wr | |
// if an animation was not allowed or not found, fall back to a sprite retrieval. | ||
var singleTexture = retrievalSource.GetTexture(componentName, wrapModeS, wrapModeT); | ||
|
||
if (singleTexture != null && maxSize != null) | ||
singleTexture = singleTexture.WithMaximumSize(maxSize.Value); | ||
|
||
return singleTexture != null | ||
? new[] { singleTexture } | ||
: Array.Empty<Texture>(); | ||
|
@@ -88,18 +92,31 @@ IEnumerable<Texture> getTextures(ISkin skin) | |
{ | ||
for (int i = 0; true; i++) | ||
{ | ||
Texture? texture; | ||
var texture = skin.GetTexture(getFrameName(i), wrapModeS, wrapModeT); | ||
|
||
if ((texture = skin.GetTexture(getFrameName(i), wrapModeS, wrapModeT)) == null) | ||
if (texture == null) | ||
break; | ||
|
||
if (maxSize != null) | ||
texture = texture.WithMaximumSize(maxSize.Value); | ||
|
||
yield return texture; | ||
} | ||
} | ||
|
||
string getFrameName(int frameIndex) => $"{componentName}{animationSeparator}{frameIndex}"; | ||
} | ||
|
||
public static Texture WithMaximumSize(this Texture texture, Vector2 maxSize) | ||
{ | ||
if (texture.DisplayWidth <= maxSize.X && texture.DisplayHeight <= maxSize.Y) | ||
return texture; | ||
|
||
// use scale adjust property for downscaling the texture in order to meet the specified maximum dimensions. | ||
texture.ScaleAdjust *= Math.Max(texture.DisplayWidth / maxSize.X, texture.DisplayHeight / maxSize.Y); | ||
Comment on lines
+115
to
+116
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We're going to have to be very conscious about this, to ensure we don't have any local usages overriding I've checked all usages in the project right now and it seems okay. For now. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. On a second thought, an alternative way to achieve this could be to have something like a "maximum texture size" property in That might be better as to avoid tinkering around with low properties like |
||
return texture; | ||
} | ||
|
||
public static bool HasFont(this ISkin source, LegacyFont font) | ||
{ | ||
return source.GetTexture($"{source.GetFontPrefix(font)}-0") != null; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where does 100 come from? The template skin is 82 x 103 (
@1x
).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not to say it's correct, but I think I picked 100 as it's closest to 103...I'll change to 82x103