1
0
mirror of https://github.com/ppy/osu.git synced 2024-12-18 05:06:14 +08:00
Commit Graph

1347 Commits

Author SHA1 Message Date
Bartłomiej Dach
89509ea49e
Fix DrawableOsuHitObject not properly cleaning up dim application callbacks
Should fix https://github.com/ppy/osu/issues/28629.

First of all, to support the claim that this does fix the issue -
reproduction is rather difficult, but I believe I found a way to
maximise the chances of it reproducing by performing the following
steps:

1. Apply the following diff:

diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs
index eacd2b3e75..4c00da031a 100644
--- a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs
+++ b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs
@@ -6,6 +6,7 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
+using System.Threading;
 using JetBrains.Annotations;
 using osu.Framework.Allocation;
 using osu.Framework.Bindables;
@@ -95,6 +96,8 @@ public DrawableSlider([CanBeNull] Slider s = null)
         [BackgroundDependencyLoader]
         private void load()
         {
+            Thread.Sleep(100);
+
             tailContainer = new Container<DrawableSliderTail> { RelativeSizeAxes = Axes.Both };

             AddRangeInternal(new Drawable[]

2. Download https://osu.ppy.sh/beatmapsets/1470790#osu/3023028 and open
   it in the editor.

3. Select all objects using Ctrl-A. Yes, it'll take a while, especially
   so with the patch above.

4. Rotate the selection by any amount using the right toolbox.

5. Press undo. The game should crash. If it doesn't spam redo and undo
   until it does.

Now to explain what the fix is.

In the issue thread I spent a considerable time hemming and hawing about
which of the dimmable pieces was null, which was a complete miss and a
failure at reading. Let's see the stack trace again:

	2024-06-27 02:15:20 [error]: at osu.Game.Rulesets.Osu.Objects.Drawables.DrawableOsuHitObject.<UpdateInitialTransforms>g__applyDim|15_0(Drawable piece) in /home/runner/work/osu-auth-client/osu-auth-client/osu/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableOsuHitObject.cs:line 101

Line 101, you say? What could be null here?

	bd8addfb5f/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableOsuHitObject.cs (L101)

Okay, what's `InitialLifetimeOffset`, then?

	bd8addfb5f/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableOsuHitObject.cs (L108)

Yes, that's right. It's `HitObject` that is null here.

Now why does *that* happen? First, let's note that all stacks where this
died went through `UpdateState()`, which means that the problematic
`applyDim()` calls had to be `ApplyCustomUpdateState` event callbacks.
Meaning that the pieces where `HitObject` was null were DHOs themselves.

Recall that parent DHOs and child DHOs are pooled separately. Therefore,
there is no guarantee that any parent and child DHOs will remain
associated with each other for the entire duration of a gameplay
session; it is quite the contrary, and nobody should rely on that.
Unfortunately for us, adding a `applyDimToDrawableHitObject` callback to
a child object's `ApplyCustomUpdateState` *implicitly creates* such an
association, because it ends up allocating a closure that captures
`this` (meaning the parent in this context).

Therefore, this now creates a situation where a child DHO can attempt to
read state from a former parent DHO which can be in an indeterminate
state, and in fact, when this crashes, the former parent DHO is most
likely not even in use - hence the null `HitObject`.

Thus, the fix is to clear the association by unsubscribing from the
event when nested objects are cleared.

My hypothesis why the reproduction scenario is like it is, is that both
the sleep and the increased pressure on the pool (by way of selecting
all objects and therefore forcing the DHOs to be materialised beyond
pool capacity) increases the likelihood of getting a crosslink.
When pool pressure is low, it is much more likely that a parent DHO
*will* get the same child DHO again on re-application, even though
that is not guaranteed.

Just as an additional detail, note that the sentry issue for this lists
the "first seen" version as 2024.312.0, which is the release that
included https://github.com/ppy/osu/pull/27401 which would be directly
responsible for this mess.
2024-09-19 14:30:02 +02:00
Dean Herbert
0c6bee4033
Merge branch 'master' into free-sliders 2024-08-19 14:27:54 +09:00
Bartłomiej Dach
088e8ad0a2
Respect pre-empt time when auto-generating breaks
Closes https://github.com/ppy/osu/issues/28703.
2024-07-23 13:30:13 +02:00
Dan Balasescu
a7e110f669
Don't rely on single-use properties 2024-07-18 19:07:02 +09:00
OliBomby
89f106d14f Merge remote-tracking branch 'upstream/master' into free-sliders 2024-07-03 20:40:25 +02:00
Bartłomiej Dach
b339d6a00c
Fix editor performance regression with hitmarkers active 2024-06-26 16:26:32 +02:00
Dean Herbert
dc817b62cc
Fix editor performance dropping over time when hit markers are enabled
There's probably a better solution but let's hotfix this for now.
2024-06-26 01:06:37 +09:00
Bartłomiej Dach
2cb18820ea
Fix incorrect slider judgement positions when classic mod is active
Regressed in https://github.com/ppy/osu/pull/27977.

Bit ad-hoc but don't see how to fix without just reverting the change.
2024-06-25 10:07:58 +02:00
Dean Herbert
df43a1c6cc
Add note about every-frame-transforms 2024-06-23 03:31:40 +08:00
Bartłomiej Dach
ad2cd0ba8f
Adjust behaviour of hit animations toggle to match user expectations 2024-06-20 13:42:32 +02:00
OliBomby
d7fee53d67 Merge remote-tracking branch 'upstream/master' into free-sliders 2024-06-19 23:15:41 +02:00
Bartłomiej Dach
86b13074a0
Merge branch 'master' into edit-nodesample 2024-06-06 13:42:25 +02:00
OliBomby
34c4ee7de8 add CanBeNull attribute to LastRepeat 2024-06-03 13:38:42 +02:00
Bartłomiej Dach
8916f08f86
Only take initial judgement position from object instead of following
Looks less bad with mods like depth active.

Co-authored-by: Dean Herbert <pe@ppy.sh>
2024-05-30 09:03:02 +02:00
Bartłomiej Dach
a6c776dac8
Use hopefully safer implementation of anchoring judgements to objects 2024-05-29 11:11:43 +02:00
Bartłomiej Dach
22a2adb5e6
Revert unrelated changes 2024-05-29 10:57:30 +02:00
Bartłomiej Dach
9fb7cc5900
Merge branch 'master' into magnetised-judgements 2024-05-29 10:56:53 +02:00
Aurelian
b74f66e335 SliderBall's rotation updates based on CurvePositionAt 2024-05-28 19:38:33 +02:00
OliBomby
2261d65b01 Merge remote-tracking branch 'upstream/master' into free-sliders 2024-05-28 18:42:34 +02:00
DavidBeh
8c2a4eb78a Fix formatting 2024-04-24 12:40:23 +02:00
DavidBeh
16190a4ed7 Made judgements follow DrawableOsuHitObjects. Enabled judgements for depth mod 2024-04-24 00:23:45 +02:00
DavidBeh
3a914b9337 Fixed judgements with MG mod without causing side effects 2024-04-23 23:24:51 +02:00
DavidBeh
331f1f31b0 Attempt to position DrawableOsuJudgement based on its DrawableOsuHitObject instead of DrawableOsuHitObject.HitObject 2024-04-23 19:19:11 +02:00
DavidBeh
beee76d64a enabled and fixed judgements for the magnetised mod 2024-04-22 20:25:43 +02:00
Dan Balasescu
4806ea54f1
Only optimise Catmull segments in osu ruleset 2024-04-01 17:22:50 +09:00
Bartłomiej Dach
c9bbeeb49a
Merge branch 'master' into heatmap-misses 2024-03-18 14:43:24 +01:00
Dean Herbert
fcc35a6acc
Fix cross-talk between pooled DrawableSliderRepeat usage causing incorrect rotation 2024-03-08 11:37:13 +08:00
Dean Herbert
646edb239a
Merge pull request #27429 from bdach/rewind-slider-tracking
Fix slider tracking state not restoring correctly in all cases on rewind
2024-03-07 22:41:12 +08:00
Dean Herbert
b53b752e54
Update usage of MathUtils 2024-03-06 12:13:12 +08:00
Bartłomiej Dach
876b806423
Store tracking history to slider judgement result instead 2024-02-29 12:11:50 +01:00
Bartłomiej Dach
1d1db951f0
Reset slider input manager state completely on new object application
Kind of scary this wasn't happening already.

Mirrors `SpinnerRotationTracker`.
2024-02-29 11:59:19 +01:00
Bartłomiej Dach
d05b31933f
Fix slider tracking state not restoring correctly in all cases on rewind 2024-02-29 11:59:15 +01:00
Bartłomiej Dach
b5ce2642aa
Fix subscribing to ApplyCustomUpdateState too much 2024-02-28 13:20:41 +01:00
Bartłomiej Dach
bbdd85020c
Fix slider tails sometimes not dimming correctly
Originally noticed during review of another change:
https://github.com/ppy/osu/pull/27369#issuecomment-1966140198.

`DrawableOsuHitObject` tries to solve the initial dimming of objects
by applying transform to a list of dimmable parts. For plain drawables
this is safe, but if one of the parts is a DHO, it is not safe,
because drawable transforms can be cleared at will.

In particular, on first use of a drawable slider,
`UpdateInitialTransforms()` would fire via `LoadComplete()` on the
`DrawableSlider`, but *then*, also via `LoadComplete()`,
the `DrawableSliderTail` would update its own state and by doing so
inadvertently clear the dim transform just added by the slider.

To fix, ensure dim transforms are applied to DHOs
via `ApplyCustomUpdateState`.
2024-02-27 11:45:03 +01:00
Bartłomiej Dach
087a2a741a
Merge pull request #27369 from Detze/incorrect-slider-head-dim
Fix slider head being incorrectly dimmed twice
2024-02-27 11:17:34 +01:00
Detze
91d7bd1026 Don't dim slider head in DrawableSlider 2024-02-24 21:56:44 +01:00
Andrei Zavatski
1fb19e7129 Reduce allocations in DrawableSpinner 2024-02-24 20:18:30 +03:00
Andrei Zavatski
572f693eec Fix failing tests related to slider ticks 2024-02-17 23:28:35 +03:00
Dean Herbert
0df6e8f595
Remove list allocations in UpdateNestedSamples 2024-02-18 03:23:34 +08:00
Dean Herbert
0714a4fc1e
Revert sample lookup logic that was not allocating anything 2024-02-18 03:18:50 +08:00
Andrei Zavatski
22f5a66c02 Reduce allocations during beatmap selection 2024-02-17 15:46:38 +03:00
Dan Balasescu
1f13124b38
Always process position as long as it's hittable
For example... If a hitobject is pressed but the result is a shake, this
will stop processing hits.
2024-02-07 03:52:56 +09:00
Dan Balasescu
e2867986c5
Add xmldocs 2024-02-07 03:52:14 +09:00
Dan Balasescu
38f7913b31
Fix inverted condition 2024-02-07 03:50:56 +09:00
Dan Balasescu
9b8f206486
Enable NRT for DrawableHitCircle to clean up 2024-02-07 03:38:07 +09:00
Dan Balasescu
5850d6a578
Show near-misses on the results-screen heatmap 2024-02-06 22:30:22 +09:00
Dan Balasescu
c18cd65081
Merge pull request #26694 from chandler14362/hit-object-result-allocations
Avoid closure allocations when applying hit object results
2024-02-06 17:49:26 +09:00
Bartłomiej Dach
3e29a9c334
Merge pull request #26877 from smoogipoo/adjust-default-minresult
Adjust default min result of `SliderTailHit`, remove override
2024-02-05 14:15:38 +01:00
Bartłomiej Dach
fb80d76b4a
Apply further changes to remove remaining weirdness 2024-02-05 13:37:38 +01:00
Bartłomiej Dach
39fe078984
Merge branch 'master' into hit-object-result-allocations 2024-02-05 13:24:10 +01:00