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 configuration option to disable Taiko notes flying after hit #31665

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

aedoq
Copy link

@aedoq aedoq commented Jan 24, 2025

In osu!stable, the "flying away" animation is shown only when the HUD is active. This adds a configuration option to restore this behaviour or to disable the animation altogether.

Discussed previously: #23656

@aedoq
Copy link
Author

aedoq commented Jan 24, 2025

Short demo:

taiko_demo.mp4

@LittleLilyBun
Copy link

Just a personal opinion as a taiko player, I like this but it feels like the fade effect takes too long, I feel like I'm hitting late because the note disappears late. For comparison, stable faded the notes instantly.

@aedoq aedoq force-pushed the hit-flying-config branch from ae0515c to ef45c1d Compare January 24, 2025 22:05
@aedoq
Copy link
Author

aedoq commented Jan 24, 2025

@LittleLilyBun You're right, I changed this to be the same as in stable, should be less jarring:

2025-01-24.23-07-48.mp4

@OpenSauce04
Copy link
Contributor

As an aside, I made a somewhat related pull request about a year ago addressing the animation of the flying notes not snapping to the center of the judgement area like they do in stable: #26926

Perhaps these two features could be combined together into a single setting as they both relate to the appearance of the flying notes?

Would wait on the thoughts of a maintainer before making any changes.

@smoogipoo
Copy link
Contributor

I'm not going to make a call on this one because this is something that would've been addressed by ruleset skinning. Will defer to @peppy.

@smoogipoo smoogipoo requested a review from peppy January 27, 2025 05:11
@aedoq
Copy link
Author

aedoq commented Feb 7, 2025

this is something that would've been addressed by ruleset skinning.

Skimming through that PR, it seems that these are completely separate issues? The "flying away" animation is not an object subject to skinning in any case, it is simply the animation path of Taiko hit objects. It is more in line with configurable circle snapping, which is currently locked behind the classic mod.

Also, I might add that if I understood the code correctly, the animation anyway has a discontinuous second derivative which would need fixing. (If an object moves gravity_travel_height up in gravity_time, then it will move 2*gravity_travel_height down in sqrt(2)*gravity_time under the same acceleration, not 2*gravity_time.)

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.

I'm not against this happening, but this PR's implementation is a bit haphazard.

Special attention is going to be required to get the visible state of the HUD – or more specifically, the health bar. Not too sure how this should be handled.

@@ -18,11 +18,13 @@ protected override void InitialiseDefaults()
base.InitialiseDefaults();

SetDefault(TaikoRulesetSetting.TouchControlScheme, TaikoTouchControlScheme.KDDK);
SetDefault(TaikoRulesetSetting.HitFlyingEnable, TaikoHitFlyingEnable.Always);
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be sometimes to match stable?

Copy link
Author

Choose a reason for hiding this comment

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

I had assumed the behaviour in lazer is intentional, since the animation is also no longer related to the health bar. So I just added a ClassicDefault that matches stable. Should the lazer default be sometimes as well?

{
rulesetConfig.BindWith(TaikoRulesetSetting.HitFlyingEnable, hitFlyingEnableSetting);
hitFlyingEnableSetting.BindValueChanged(_ => updateHitFlying());
osuConfig.BindWith(OsuSetting.HUDVisibilityMode, hudVisible);
Copy link
Member

Choose a reason for hiding this comment

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

This will look wrong during replays and when the hold-for-hud key is held.

Copy link
Author

@aedoq aedoq Feb 18, 2025

Choose a reason for hiding this comment

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

Hmm you're right. Ideally we could bind to HUDOverlay.ShowHud here, but I'm not familiar enough with the framework to get the DI right. It gives me a DependencyNotRegisteredException if I try to [Resolved] it.

On a related note: Is it fine if, like now, the decision to show a flying note is only taken once, such that enabling the HUD will not show the animation for notes that have already passed?

@@ -39,6 +42,9 @@ public TaikoAction? HitAction
private double? lastPressHandleTime;

private readonly Bindable<HitType> type = new Bindable<HitType>();
private readonly Bindable<TaikoHitFlyingEnable> hitFlyingEnableSetting = new Bindable<TaikoHitFlyingEnable>();
private readonly Bindable<HUDVisibilityMode> hudVisible = new Bindable<HUDVisibilityMode>();
protected bool ShowFlyingHits;
Copy link
Member

Choose a reason for hiding this comment

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

This is flimsy. If this isn't in the right value before LoadComplete, it won't be handled correctly in DrawableFlyingHit.

Copy link
Author

Choose a reason for hiding this comment

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

This should be fine now, since DrawableFlyingHit has no logic on this anymore?

@@ -26,6 +26,11 @@ protected override void LoadComplete()
{
base.LoadComplete();
ApplyMaxResult();

if (!ShowFlyingHits)
Copy link
Member

Choose a reason for hiding this comment

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

Nothing will ever unhide this.

Copy link
Author

@aedoq aedoq Feb 18, 2025

Choose a reason for hiding this comment

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

This wasn't necessary it turns out. I removed this.

In osu!stable, the "flying away" animation is shown only when the HUD
is active. This adds a configuration option to restore this behaviour
or to disable the animation altogether.
@aedoq aedoq force-pushed the hit-flying-config branch from ef45c1d to 9431df3 Compare February 18, 2025 18:26
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.

5 participants