Fix storyboard commands occurring before the earliest point of visibility delaying gameplay #11990
Conversation
…lity delaying gameplay In osu-stable, storyboard intros start from the first command, but in the case of storyboard drawables which have an initial hidden state, all commands before the time at which they become visible (ie. the first command where `Alpha` increases to a non-zero value) are ignored. This brings lazer in line with that behaviour. It also removes several unnecessary LINQ calls. Note that the alpha check being done in its own pass is important, as it must be the "minimum present alpha across all command groups, including loops". This is what makes the implementation slightly complex. Closes ppy#11981.
| { | ||
| var first = Alpha.Commands.FirstOrDefault(); | ||
|
|
||
| return first?.StartValue == 0 ? first.StartTime : (double?)null; |
There was a problem hiding this comment.
Pretty sure this check isn't needed, since a starting alpha command with other start values will still trigger an intro on stable.
In other words, if the starting alpha command starts with say, 1, this will behave as if there are no alpha-affecting commands, when there clearly is at least one.
There was a problem hiding this comment.
I also tested when a fade command has both start and end values of 0 before any other fade commands, and it still causes an intro on stable.
There was a problem hiding this comment.
To confirm, an alpha command starting at 1 makes the sprite invisible before the specified time?
There was a problem hiding this comment.
No, an alpha command starting at 1 will make the sprite initially visible (starting from first of any of the sprite's commands), 0 to 1 is vice versa.
If it's 0 to 0, it'll be invisible but still affect intro time.
There was a problem hiding this comment.
I misread the question, updated above comment. Answer is still no.
There was a problem hiding this comment.
Okay sure, but I think this example will still work correctly with my implementation (and going forward, be more correct?).
Is that fail case you mention actually in any beatmap? If not, that's stable implemented incorrectly and should be considered fine to have "fail".
There was a problem hiding this comment.
I guess any map with such patterns would simply have more of their storyboarded content visible. Alright then, seems correct.
There was a problem hiding this comment.
One thing that jumped out to me in this thread, about that
If it's 0 to 0, it'll be invisible but still affect intro time.
case: are we talking about something like this?
Sprite,Background,Centre,"SB\leaf.png",320,240
F,0,-1000,-500,0,0 // fake fade that doesn't do anything
F,0,-500,0,0,0.5
In that case I'd expect the intro to start at -500 and not -1000, since the first command doesn't actually display the sprite, while it'd be -1000 using the current implementation... Is that something that storyboards rely on to start displaying earlier?
There was a problem hiding this comment.
Is that something that storyboards rely on to start displaying earlier?
I ran a couple greps for "fades from 0 to 0" in my song folder and found nothing of the sort (among about 530 sets with storyboards). I can't remember any such cases either.
Note that the only use for this would be to extend the intro time earlier than any storyboard sprites are visible, which would only cause a black background at the start (or just the diff background if it wasn't removed in the storyboard). That was basically the exact issue with the storyboard mentioned in #11981 in lazer.
If we're going by the logic of "only visible sprites can affect intro time" as peppy seems to suggest, then yes, that case would contradict that. The question is if it needs to be fixed in this implementation, since I'd wager that nobody uses it based on my sample size (or would need/want to use it).
On another note, checking for instances of the other cases I pointed out earlier would probably take more effort than some greps. Although I could check this programmatically on all the maps I have, I don't find it worth it if we don't care about implementing such hypothetical storyboards 1-to-1 with stable anyway.
There was a problem hiding this comment.
It's an interesting case for sure, but I'd like to hope that's out of scope for the time being (until it arises that it was actually necessary for a ranked beatmap to work as expected).
One would hope that noop transforms like that would be removed by the osu! editor, or whatever scripting program the storyboarder is using.
| { | ||
| var first = Alpha.Commands.FirstOrDefault(); | ||
|
|
||
| return first?.StartValue == 0 ? first.StartTime : (double?)null; |
There was a problem hiding this comment.
One thing that jumped out to me in this thread, about that
If it's 0 to 0, it'll be invisible but still affect intro time.
case: are we talking about something like this?
Sprite,Background,Centre,"SB\leaf.png",320,240
F,0,-1000,-500,0,0 // fake fade that doesn't do anything
F,0,-500,0,0,0.5
In that case I'd expect the intro to start at -500 and not -1000, since the first command doesn't actually display the sprite, while it'd be -1000 using the current implementation... Is that something that storyboards rely on to start displaying earlier?
Co-authored-by: Bartłomiej Dach <dach.bartlomiej@gmail.com>
In osu-stable, storyboard intros start from the first command, but in the case of storyboard drawables which have an initial hidden state, all commands before the time at which they become visible (ie. the first command where
Alphaincreases to a non-zero value) are ignored.This brings lazer in line with that behaviour. It also removes several unnecessary LINQ calls.
Note that the alpha check being done in its own pass is important, as it must be the "minimum present alpha across all command groups, including loops". This is what makes the implementation slightly complex.
Closes #11981.