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

Storyboard section with loops rendered incorrectly #29675

Open
NoffyNoffeh opened this issue Sep 1, 2024 · 4 comments
Open

Storyboard section with loops rendered incorrectly #29675

NoffyNoffeh opened this issue Sep 1, 2024 · 4 comments

Comments

@NoffyNoffeh
Copy link

NoffyNoffeh commented Sep 1, 2024

Type

Game Behavior

Bug description

When playing the daily challenge for the Hide and Seek set https://osu.ppy.sh/beatmapsets/972932#osu/2036905 , I noticed my storyboard is broken in lazer at 01:02:005 - through 01:07:895 - and 01:55:011 -

Some elements expected to scale repeatedly using a loop command stop scaling after the first instance of the loop command is completed, and creates a different undesired visual when compared to stable.

Screenshots or videos

A video showing how it currently plays in stable vs. lazer:
https://www.youtube.com/watch?v=qjzQ3kqhi6Y

Here is the osb script for the two related sprites, if it's useful to reference:

bottom sprite
image

top sprite with looping scale command
image

Version

2024.817.0

Logs

compressed-logs.zip

@bdach
Copy link
Collaborator

bdach commented Sep 13, 2024

Looking at what the storyboard classes do, the minimal example is

        [Test]
        public void TestTest() => AddStep("do stuff", () =>
        {
            Box box;

            Child = box = new MyBox
            {
                Size = new Vector2(100),
                Anchor = Anchor.Centre,
                Origin = Anchor.Centre,
            };

            using (box.BeginDelayedSequence(500))
                box.ScaleTo(Vector2.Zero).Then().ScaleTo(new Vector2(0.8f), 250, Easing.OutElastic).Loop(500, 10);

            using (box.BeginDelayedSequence(1000))
                box.ScaleTo(Vector2.Zero).Then().ScaleTo(Vector2.Zero).Loop(500, 10);
        });


        private partial class MyBox : Box
        {
            public override bool RemoveCompletedTransforms => false;
        }

(MyBox is there just to be able to see the transforms in post, although it being present does match the storyboard case.)

I'm not super sure where to take this next, because you could argue that it's a framework bug in transform handling, or it's a bug in the storyboard implementation because this code is not valid.

@smoogipoo mind taking a look here and chipping in as to whether you'd expect the code as written above to visually behave as is expected? (see video in OP)

@peppy
Copy link
Sponsor Member

peppy commented Sep 27, 2024

In the repro you provided, is the issue that the two loops are overlapping each other?

@bdach
Copy link
Collaborator

bdach commented Sep 27, 2024

Something like that. Video for reference:

2024-09-27.10-54-11.mp4

I haven't looked in detail what is happening precisely but it appears the two loops interfere with each other in a way that affects the correct readout of initial values of the transform. Hence the question as to whether this is even considered valid usage from the framework's standpoint.

@Wieku
Copy link
Contributor

Wieku commented Sep 27, 2024

@bdach Not a solution, but some insights.

Hence the question as to whether this is even considered valid usage from the framework's standpoint

Pretty sure not. Transform system keeps track of the latest transform and discards older ones:
https://github.com/ppy/osu-framework/blob/e4cbe949e87e6d72fbbcfca07a545abe8a1c1a37/osu.Framework/Graphics/Transforms/TargetGroupingTransformTracker.cs#L121-L151

Transforms are ordered by start time and then by transform id, so if both loop elements have same absolute start times then the one with bigger id (added later) takes precedence.

If we add instant commands first then it executes as expected (for some reason sharex freaks out when recording tests):

2024-09-28_00-22-16_teBBR.mp4
using (box.BeginDelayedSequence(500))
    box.ScaleTo(Vector2.Zero).Then().ScaleTo(new Vector2(0.8f), 552, Easing.OutBack).Loop(184, 10);

using (box.BeginDelayedSequence(1236))
    box.ScaleTo(Vector2.Zero).Then().ScaleTo(Vector2.Zero).Loop(736, 10);

So if changing the framework is out of the question, storyboard parser would have to recognize instant commands at the end of the loop and insert them before the rest.

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

No branches or pull requests

5 participants