Skip to content

Conversation

@bdach
Copy link
Collaborator

@bdach bdach commented Aug 12, 2025

This indicator allows the player to either rewind to an earlier part of the replay, or to proceed to results. It also plays a shortened variant of the failure animation SFX.

2025-08-12.14-35-51.mov

In addition to the main piece, this goes on a few side missions as well:

Refactor fail management to be more centralised

Before this commit, there was AllowFailAnimation (used by multiplayer) and OnFail() (used by score submitting implementations of player to ensure a failed play submits).

The former is replaced by PerformFail() which allows for arbitrary operations on failure, which replay player leverages here for the main piece to work. The latter would ideally be replaced by nothing, but it's placed in a very awkward place behind a schedule, so by force of necessity to avoid code duplication it's replaced by ConcludeFailedScore() which is overridden to submit the score in all submitting players --- except for multiplayer, which is never supposed to be calling it, so in that case it just throws an exception.

The reason I did this is not to be a hypocrite after I complained about the fail flows last time.

Add rudimentary support of rewinding failed state to health processor

As indicated in the inline comment this is very best effort, just to make the HP bar not very obviously stuck in a very obviously incorrect state after the replay is rewound from a failure. There's likely to be a bunch of replay accuracy issues related to this, but I'm just making the minimum effort to get this to work semi-acceptably for now.

If this is too much then I'll drop this change with no contest.

bdach added 4 commits August 12, 2025 14:18
Before this commit, there was `AllowFailAnimation` (used by multiplayer)
and `OnFail()` (used by score submitting implementations of player to
ensure a failed play submits).

The former is replaced by `PerformFail()` which allows for arbitrary
operations on failure, which replay player shall leverage in subsequent
commits. The latter would ideally be replaced by nothing, but it's
placed in a very awkward place behind a schedule, so by force of
necessity to avoid code duplication it's replaced by
`ConcludeFailedScore()` which is overridden to submit the score in all
submitting players --- except for multiplayer, which is never supposed
to be calling it, so in that case it just throws an exception.
This indicator allows the player to either rewind to an earlier part of
the replay, or to proceed to results. It also plays a shortened variant
of the failure animation SFX.
As indicated in the inline comment this is very best effort, just to
make the HP bar not very obviously stuck in a very obviously incorrect
state after the replay is rewound from a failure. There's likely to be a
bunch of replay accuracy issues related to this, but I'm just making the
minimum effort to get this to work semi-acceptably for now.
@bdach bdach self-assigned this Aug 12, 2025
@peppy peppy self-requested a review August 15, 2025 05:59
/// <summary>
/// "Go to results"
/// </summary>
public static LocalisableString GoToResults => new TranslatableString(getKey(@"go_to_results"), @"Go to results");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was thinking "Show results" might fit better but given it's shifting between game screens I think this is probably best left as-is.

Copy link
Member

@peppy peppy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to work pretty well. Feels more than good enough for a first iteration.

@peppy peppy merged commit a14d7a2 into ppy:master Aug 15, 2025
7 of 9 checks passed
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.

2 participants