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

Add AlwaysPresent for AudioContainer and fix AlwaysPresent not being taken into consideration when computing isMaskedAway #6359

Closed
wants to merge 3 commits into from

Conversation

Drison64
Copy link

used for ppy/osu#29215

@smoogipoo
Copy link
Contributor

smoogipoo commented Aug 14, 2024

Presence has nothing to do with being masked away or being alive. I'm fine with the property, if that's all that you need to get things working. Otherwise, we'll need to think of something else.

@Drison64
Copy link
Author

Drison64 commented Aug 14, 2024

I did not notice I pushed the ShouldBeAlive (was from when I was testing things out), i removed it and it still works.

I added alwaysPresent check to ComputeIsMaskedAway, because when it was masked away (in this case, off the screen) a check in CompositeDrawable line 917 does not pass. I am open to creating a different property just for this case if needed.

@smoogipoo
Copy link
Contributor

Yeah, that check is supposed to not pass. It has caused problems in the past but I don't think presence is the way to fix this, or at least I'm not ready to do it yet.

There are two solutions I can mention without having looked at the osu!-side code:

  1. Make the container not be masked away in general.
  2. Override UpdateSubTreeMasking() and return false.

@peppy
Copy link
Sponsor Member

peppy commented Aug 15, 2024

Also agree here.

If anything, rather than exposing this as a setting it should be implicitly applied to AudioContainer without the user having control.

@smoogipoo
Copy link
Contributor

I'm not sure about that... Means if at some point the entire game is wrapped in an AudioContainer no masking calcs would ever take place. Would prefer it to be applied to a local container where the intent is explicit, for now.

@peppy peppy closed this Aug 15, 2024
@Drison64 Drison64 changed the title Add AlwaysPresent for AudioContainer and fix AlwaysPresent not being taken into consideration when computing isMaskedAway or ShouldBeAlive Add AlwaysPresent for AudioContainer and fix AlwaysPresent not being taken into consideration when computing isMaskedAway Aug 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants