1
0
mirror of https://github.com/ppy/osu.git synced 2024-11-14 16:37:26 +08:00

Fix DrawableJudgement not always animating correctly on skin change

If the skin is changed before gameplay has started (at the loading
screen) it is possible for a sequence of events to occur which results
in the animation not being played:

- `SkinReloadableDrawable` runs its BDL load (and calls `OnSkinChanged`
  once)
- User changes skin, triggering `DrawableJudgement`'s skin change
  handling (binding directly on the `SkinSource` locally)
- This will call `PrepareDrawables` and reinitialise the
  `SkinnableDrawable` child hierarchy, then immediately apply the
animations to it.
- The new `SkinnableDrawable` will then get the `SkinChanged` event and
  schedule a handler for it, which will run on its first Update call.
- Any added animations will be lost as a result.

Fixed by binding directly to the `SkinnableDrawable`'s `OnSkinChanged`.
This has the added bonus of not needing to reinitialise the child
hierarchy on skin change (which felt a bit weird in the first place).
This commit is contained in:
Dean Herbert 2020-11-24 15:19:51 +09:00
parent 060acb9010
commit ee33f62809

View File

@ -32,9 +32,6 @@ namespace osu.Game.Rulesets.Judgements
private readonly Container aboveHitObjectsContent; private readonly Container aboveHitObjectsContent;
[Resolved]
private ISkinSource skinSource { get; set; }
/// <summary> /// <summary>
/// Duration of initial fade in. /// Duration of initial fade in.
/// </summary> /// </summary>
@ -78,29 +75,6 @@ namespace osu.Game.Rulesets.Judgements
public Drawable GetProxyAboveHitObjectsContent() => aboveHitObjectsContent.CreateProxy(); public Drawable GetProxyAboveHitObjectsContent() => aboveHitObjectsContent.CreateProxy();
protected override void LoadComplete()
{
base.LoadComplete();
skinSource.SourceChanged += onSkinChanged;
}
private void onSkinChanged()
{
// on a skin change, the child component will update but not get correctly triggered to play its animation.
// we need to trigger a reinitialisation to make things right.
currentDrawableType = null;
PrepareForUse();
}
protected override void Dispose(bool isDisposing)
{
base.Dispose(isDisposing);
if (skinSource != null)
skinSource.SourceChanged -= onSkinChanged;
}
/// <summary> /// <summary>
/// Apply top-level animations to the current judgement when successfully hit. /// Apply top-level animations to the current judgement when successfully hit.
/// If displaying components which require lifetime extensions, manually adjusting <see cref="Drawable.LifetimeEnd"/> is required. /// If displaying components which require lifetime extensions, manually adjusting <see cref="Drawable.LifetimeEnd"/> is required.
@ -142,16 +116,17 @@ namespace osu.Game.Rulesets.Judgements
Debug.Assert(Result != null); Debug.Assert(Result != null);
prepareDrawables();
runAnimation(); runAnimation();
} }
private void runAnimation() private void runAnimation()
{ {
// is a no-op if the drawables are already in a correct state.
prepareDrawables();
// undo any transforms applies in ApplyMissAnimations/ApplyHitAnimations to get a sane initial state. // undo any transforms applies in ApplyMissAnimations/ApplyHitAnimations to get a sane initial state.
ApplyTransformsAt(double.MinValue, true); // ApplyTransformsAt(double.MinValue, true);
ClearTransforms(true); // ClearTransforms(true);
LifetimeStart = Result.TimeAbsolute; LifetimeStart = Result.TimeAbsolute;
@ -193,6 +168,8 @@ namespace osu.Game.Rulesets.Judgements
private void prepareDrawables() private void prepareDrawables()
{ {
Logger.Log("prepareDrawables on DrawableJudgement");
var type = Result?.Type ?? HitResult.Perfect; //TODO: better default type from ruleset var type = Result?.Type ?? HitResult.Perfect; //TODO: better default type from ruleset
// todo: this should be removed once judgements are always pooled. // todo: this should be removed once judgements are always pooled.
@ -203,7 +180,6 @@ namespace osu.Game.Rulesets.Judgements
if (JudgementBody != null) if (JudgementBody != null)
RemoveInternal(JudgementBody); RemoveInternal(JudgementBody);
aboveHitObjectsContent.Clear();
AddInternal(JudgementBody = new SkinnableDrawable(new GameplaySkinComponent<HitResult>(type), _ => AddInternal(JudgementBody = new SkinnableDrawable(new GameplaySkinComponent<HitResult>(type), _ =>
CreateDefaultJudgement(type), confineMode: ConfineMode.NoScaling) CreateDefaultJudgement(type), confineMode: ConfineMode.NoScaling)
{ {
@ -211,14 +187,29 @@ namespace osu.Game.Rulesets.Judgements
Origin = Anchor.Centre, Origin = Anchor.Centre,
}); });
JudgementBody.OnSkinChanged += () =>
{
// on a skin change, the child component will update but not get correctly triggered to play its animation (or proxy the newly created content).
// we need to trigger a reinitialisation to make things right.
proxyContent();
runAnimation();
};
proxyContent();
currentDrawableType = type;
void proxyContent()
{
aboveHitObjectsContent.Clear();
if (JudgementBody.Drawable is IAnimatableJudgement animatable) if (JudgementBody.Drawable is IAnimatableJudgement animatable)
{ {
var proxiedContent = animatable.GetAboveHitObjectsProxiedContent(); var proxiedContent = animatable.GetAboveHitObjectsProxiedContent();
if (proxiedContent != null) if (proxiedContent != null)
aboveHitObjectsContent.Add(proxiedContent); aboveHitObjectsContent.Add(proxiedContent);
} }
}
currentDrawableType = type;
} }
protected virtual Drawable CreateDefaultJudgement(HitResult result) => new DefaultJudgementPiece(result); protected virtual Drawable CreateDefaultJudgement(HitResult result) => new DefaultJudgementPiece(result);