Notifications can kick players from live games

[ZenoG2]

Type

Game behaviour

Bug description

Notifications can be sent to the game during breaks. Clicking the notification will take you to the menu with no warning, cancelling your current play.

Screenshots or videos

https://youtu.be/jiyayK8jd9s

Version

2024.727.0

[ZenoG2]

It seems like update notifications are not the only kind of notification that cause this issue...

[peppy]

This is kiiinda how notifications work. Player only blocks the first exit request, but the restart notification is very persistent due to using PerformFromScreen:

https://github.com/ppy/osu/blob/12a148d1084b1a6f11525ff16c223ecb6ddd6927/osu.Desktop/Updater/SquirrelUpdateManager.cs#L150-L155

https://github.com/ppy/osu/blob/f068b7a521c8ce8b29da9161f7ef4c7ab92429ec/osu.Game/OsuGame.cs#L857-L861

I can think of two options:

• Player should block at the pause screen regardless of further exit requests until the user chooses the on-screen "Exit" button. This would make it similar to the editor when it has changes pending.
• The flow to exit on update application should be less persistent. This would either mean making an exception for "user in gameplay" (simple) or changing the PerformFromScreen implementation to allow no-retry mode.

Open to opinion on this one @ppy/team-client.

[bdach]

I don't think update notifications should ever be shown during gameplay to begin with. I'm not sure how feasible that is to address, however.

Player should block at the pause screen regardless of further exit requests until the user chooses the on-screen "Exit" button. This would make it similar to the editor when it has changes pending.

I feel like this would get sorta confusing if it was just using the standard pause screen without specific messaging that the game will restart on exit.

The flow to exit on update application should be less persistent. This would either mean making an exception for "user in gameplay" (simple) or changing the PerformFromScreen implementation to allow no-retry mode.

Again not sure on this one due to messaging. If the notification says "click to restart" and then the restart either doesn't happen or is indefinitely delayed until player exit then that does not sound good.

[peppy]

I'll see if I can stop it showing during gameplay.

[peppy]

@bdach which of these would you prefer?

// 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 osu.Framework.Allocation;
using osu.Framework.Bindables;

namespace osu.Game.Screens.Play
{
    [Cached]
    public interface ILocalUserPlayInfo
    {
        /// <summary>
        /// Whether the local user is currently playing.
        /// </summary>
        /// <remarks>
        /// Importantly, this will become <c>false</c> during break time (or when paused).
        /// </remarks>
        IBindable<bool> IsPlaying { get; }

        /// <summary>
        /// Whether the local user is in a gameplay session (ie. at the <see cref="Player"/> screen).
        /// </summary>
        /// <remarks>
        /// This will still be <c>true</c> if the user is paused or in break time.
        /// </remarks>
        IBindable<bool> IsInGameplay { get; }
    }
}

// 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 osu.Framework.Allocation;
using osu.Framework.Bindables;

namespace osu.Game.Screens.Play
{
    [Cached]
    public interface ILocalUserPlayInfo
    {
        /// <summary>
        /// Whether the local user is currently playing.
        /// </summary>
        IBindable<PlayingState> PlayingState { get; }
    }

    public enum PlayingState
    {
        /// <summary>
        /// The local player is not current in gameplay
        /// </summary>
        NotPlaying,

        /// <summary>
        /// In break time or paused.
        /// </summary>
        Break,

        /// <summary>
        /// The local user is in active gameplay.
        /// </summary>
        Playing,
    }
}

"neither" is also an option. but if we're looking for a less hacky solution, I'd rather not make a new tracking class and just modify this one to work for the cases we require.

[bdach]

Probably the enum variant.