-
-
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 16 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 |
---|---|---|
|
@@ -4,6 +4,7 @@ | |
using System; | ||
using osu.Framework.Bindables; | ||
using osu.Framework.Graphics; | ||
using osu.Game.Rulesets.Osu.Objects; | ||
using osu.Game.Skinning; | ||
using osuTK; | ||
|
||
|
@@ -20,7 +21,7 @@ public class OsuLegacySkinTransformer : LegacySkinTransformer | |
/// Their hittable area is 128px, but the actual circle portion is 118px. | ||
/// We must account for some gameplay elements such as slider bodies, where this padding is not present. | ||
/// </summary> | ||
public const float LEGACY_CIRCLE_RADIUS = 64 - 5; | ||
public const float LEGACY_CIRCLE_RADIUS = OsuHitObject.OBJECT_RADIUS - 5; | ||
|
||
public OsuLegacySkinTransformer(ISkin skin) | ||
: base(skin) | ||
|
@@ -41,14 +42,14 @@ public OsuLegacySkinTransformer(ISkin skin) | |
return this.GetAnimation("sliderscorepoint", false, false); | ||
|
||
case OsuSkinComponents.SliderFollowCircle: | ||
var followCircleContent = this.GetAnimation("sliderfollowcircle", true, true, true); | ||
var followCircleContent = this.GetAnimation("sliderfollowcircle", true, true, true, maxSize: new Vector2(308f)); | ||
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. Where does this 308 come from? 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. Wiki, maximum effective follow circle size (https://osu.ppy.sh/wiki/en/Skinning/osu%21#slider, see |
||
if (followCircleContent != null) | ||
return new LegacyFollowCircle(followCircleContent); | ||
|
||
return null; | ||
|
||
case OsuSkinComponents.SliderBall: | ||
var sliderBallContent = this.GetAnimation("sliderb", true, true, animationSeparator: ""); | ||
var sliderBallContent = this.GetAnimation("sliderb", true, true, animationSeparator: "", maxSize: new Vector2(OsuHitObject.OBJECT_RADIUS * 2)); | ||
|
||
// todo: slider ball has a custom frame delay based on velocity | ||
// Math.Max((150 / Velocity) * GameBase.SIXTY_FRAME_TIME, GameBase.SIXTY_FRAME_TIME); | ||
|
@@ -138,7 +139,7 @@ public OsuLegacySkinTransformer(ISkin skin) | |
if (!this.HasFont(LegacyFont.HitCircle)) | ||
return null; | ||
|
||
return new LegacySpriteText(LegacyFont.HitCircle) | ||
return new LegacySpriteText(LegacyFont.HitCircle, new Vector2(OsuHitObject.OBJECT_RADIUS * 2)) | ||
{ | ||
// stable applies a blanket 0.8x scale to hitcircle fonts | ||
Scale = new Vector2(0.8f), | ||
|
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 }; | ||
} | ||
} | ||
} |
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