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

Exiting the manage collections screen while editing an existing collection leaves the low-pass filter active #24100

Closed
Bruno5430 opened this issue Jul 1, 2023 · 15 comments · Fixed by #27390

Comments

@Bruno5430
Copy link
Contributor

Bruno5430 commented Jul 1, 2023

Type

Game behaviour

Bug description

  • Go to the beatmap carousel
  • Manage the collections by either right-clicking a beatmap or the options tab
  • Select the title of any existing collection (it's not necessary to be modified)
  • Exit the manage collections screen without unselecting the textbox by either clicking the X icon or outside of the screen
  • The low-pass filter is left active (even in gameplay)
  • Entering the manage collection screen again and exiting normally clears the filter

Screenshots or videos

2023-07-01.16-37-48.webm

Version

2023.621.0

Logs

@Joehuu
Copy link
Member

Joehuu commented Jul 2, 2023

@Bruno5430 can you reliably reproduce this with the steps you provided? Can't seem to reproduce well. I only hit this once after 5 minutes of trying.

Something has gone really wrong with the PopOut() call (FadeOut() and ScaleTo() animations don't happen in the video too):

protected override void PopOut()
{
base.PopOut();
lowPassFilter.CutoffTo(AudioFilter.MAX_LOWPASS_CUTOFF, 100, Easing.InCubic);
this.FadeOut(exit_duration, Easing.OutQuint);
this.ScaleTo(0.9f, exit_duration);
// Ensure that textboxes commit
GetContainingInputManager()?.TriggerFocusContention(this);
}

@Bruno5430
Copy link
Contributor Author

@Bruno5430 can you reliably reproduce this with the steps you provided? Can't seem to reproduce well. I only hit this once after 5 minutes of trying.

Yeah i can, but i should clarify some conditions

  • Exit the manage collections screen by either pressing Esc or clicking outside of the screen

Pressing Esc unselects the textbox and doesn't leave the filter active, i will edit it in op

  • Select the title of any existing collection (it's not necessary to be modified)

It has to be an existing collection (It's not necessary to have any beatmaps), It doesn't work if you select the "Create new collection" textbox

Also I'm in linux, so idk if It's platform specific or locale specific (I't's on es_AR)

@Bruno5430
Copy link
Contributor Author

Ok, it isn't entirely consistent but it's still very common and the conditions i noted before still apply
2023-07-02 15-58-02.webm

This time submitting clean logs from the session shown in the video
legacy-ipc.log
performance-draw.log
performance-update.log
performance-audio.log
performance-input.log
database.log
input.log
performance.log
network.log
runtime.log

@peppy
Copy link
Member

peppy commented Jul 3, 2023

I have no idea how this can happen.

A starting point is to figure out what state the overlay is in when in this state. Somehow it is not visible but skipped all state change logic (so State.Value must either still be visible, or it got dismissed so brutally that it couldn't execute the callbacks / animations).

I can't seem to repro locally, so if someone manages it, use the draw visualiser to examine the ManageCollectionsDialog's state.

@Bruno5430
Copy link
Contributor Author

Bruno5430 commented Jul 4, 2023

Apparently the overlay does hide, but the animations do this:

  • FadeOut and ScaleTo happen instantly
  • CutoffTo freezes indefinitely, and only unfreezes when another animation plays
2023-07-04.17-12-17.webm

@peppy
Copy link
Member

peppy commented Feb 19, 2024

@Bruno5430 please re-test this in the next release (coming today or tomorrow) thanks!

@Bruno5430
Copy link
Contributor Author

Still happens

2024-02-19.19-25-23.webm

@frenzibyte
Copy link
Member

frenzibyte commented Feb 19, 2024

The key to this happening is probably in the sudden spike you're getting while trying to exit the manage collections screen.

@frenzibyte frenzibyte self-assigned this Feb 19, 2024
@frenzibyte
Copy link
Member

Thanks to the information in #24100 (comment), and in addition to what I observed above, it's very likely that the issue is happening from the following scenario:

  1. ManageCollectionsDialog.PopOut adds transformation for alpha, scale, and AudioFilter transforming cutoff
  2. Game gets frozen for whatever reason (as can be noticed in all of the videos attached above).
  3. Game regains control, and on the next iteration of ManageCollectionsDialog.UpdateSubTree, both alpha and scale transforms are processed to final value immediately (since time has passed already)
  4. ManageCollectionsDialog becomes non-present after processing the alpha transform, so it doesn't update its children due to this specific conditional, leaving AudioFilter's transform stuck indefinitely until the dialog is visible again.

I can replicate this as well by putting the update thread to ~300ms sleep in PopOut after all transforms are added.

This questions the friendliness between the AudioFilter component and the rest of the drawable system, especially due to the conditional I linked in point 4. CompositeDrawable assumes that if the parent drawable already became non-present, then there's no point to process children anymore since it doesn't matter visually (everything will be hidden), but AudioFilter is an audible component, therefore being an exception to that assumption.

I'm not quite sure what's the best approach here, but this undoubtedly shows a flaw in the nature of AudioFilter with the rest of the drawable system. I had thought of a separate system where audio filters are managed by a general component and the cutoff values are managed by a bindable that's transformed by the drawable class that controls its presence (i.e. ManageCollectionsDialog doing the bindable transform using this.TransformBindableTo), but as I try to pursue through this direction, I feel further unsure as to whether it's a good direction or not, as it comes with its own levels of complexity, and I'm frankly not that good at designing a reliable system.

For the time being, we can easily work around this issue by either making ManageCollectionsDialog always present using AlwaysPresent = true, or override the IsPresent boolean to be true when the filter has a non-default cutoff.

@bdach
Copy link
Collaborator

bdach commented Feb 20, 2024

I'd probably do something like override RequiresChildrenUpdate on the collections dialog and call it a day.

@frenzibyte
Copy link
Member

If we continue to use AudioFilter in more places, we're very likely to hit multiple kinds of this issue in those places.

@bdach
Copy link
Collaborator

bdach commented Feb 20, 2024

We work around presence weirdness in so many places now that I consider that argument to be mostly moot.

@frenzibyte
Copy link
Member

frenzibyte commented Feb 20, 2024

It's not about workarounds, it's about predicting how the transform can fail to be processed to avoid that from occurring with the user, because again, AudioFilter is an audible component. If presence is taken away, audio will not return back to normal (unlike normal drawables where loss of presence means nothing displayed)

@bdach
Copy link
Collaborator

bdach commented Feb 20, 2024

No I find that presence is the core issue here and don't see this as any different to any e.g. scheduling issue caused by presence wonkiness. The only difference that the breakage is audible and not visible.

@arialp
Copy link

arialp commented Feb 26, 2024

I experienced the same issue today for build 2024.221.0-lazer and although it looks like you found potential fixes, I would like to add my findings.

While creating a new collection via right click on beatmap > Collections > Manage and then clicking off the menu to go back, the game occasionally stutters, causing the beatmap music to stay muffled everywhere in-game. I was able to reproduce this issue at times: 0:14, 1:06, and 1:26 in the uploaded video. I also noticed that longer and more complex collection names cause higher probability of stutters upon leaving the menu, and subsequently, higher reproducibility of the bug. The temporary fix is to simply open the aforementioned menu again and close without doing anything.

I installed osu!lazer on a spinning HDD and thought this was the main cause as I experience frequent stutters in song select already because of a slow disk.

Logs:
compressed-logs.zip

Osu.2024.02.26.-.13.44.16.03.mp4

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

Successfully merging a pull request may close this issue.

6 participants