Skip to content

Commit

Permalink
Merge pull request #19457 from peppy/fix-summary-kiai-ranges
Browse files Browse the repository at this point in the history
Fix editor summary timeline not responding to kiai changes correctly
  • Loading branch information
frenzibyte authored Jul 29, 2022
2 parents f07a416 + 2500e5c commit f79d749
Showing 1 changed file with 52 additions and 30 deletions.
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// 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.

#nullable disable

using System.Linq;
using osu.Framework.Allocation;
using osu.Framework.Bindables;
Expand All @@ -18,13 +16,13 @@ namespace osu.Game.Screens.Edit.Components.Timelines.Summary.Parts
public class EffectPointVisualisation : CompositeDrawable, IControlPointVisualisation
{
private readonly EffectControlPoint effect;
private Bindable<bool> kiai;
private Bindable<bool> kiai = null!;

[Resolved]
private EditorBeatmap beatmap { get; set; }
private EditorBeatmap beatmap { get; set; } = null!;

[Resolved]
private OsuColour colours { get; set; }
private OsuColour colours { get; set; } = null!;

public EffectPointVisualisation(EffectControlPoint point)
{
Expand All @@ -38,37 +36,61 @@ public EffectPointVisualisation(EffectControlPoint point)
private void load()
{
kiai = effect.KiaiModeBindable.GetBoundCopy();
kiai.BindValueChanged(_ =>
{
ClearInternal();
kiai.BindValueChanged(_ => refreshDisplay(), true);
}

AddInternal(new ControlPointVisualisation(effect));
private EffectControlPoint? nextControlPoint;

if (!kiai.Value)
return;
protected override void LoadComplete()
{
base.LoadComplete();

var endControlPoint = beatmap.ControlPointInfo.EffectPoints.FirstOrDefault(c => c.Time > effect.Time && !c.KiaiMode);
// Due to the limitations of ControlPointInfo, it's impossible to know via event flow when the next kiai point has changed.
// This is due to the fact that an EffectPoint can be added to an existing group. We would need to bind to ItemAdded on *every*
// future group to track this.
//
// I foresee this being a potential performance issue on beatmaps with many control points, so let's limit how often we check
// for changes. ControlPointInfo needs a refactor to make this flow better, but it should do for now.
Scheduler.AddDelayed(() =>
{
var next = beatmap.ControlPointInfo.EffectPoints.FirstOrDefault(c => c.Time > effect.Time);

// handle kiai duration
// eventually this will be simpler when we have control points with durations.
if (endControlPoint != null)
if (!ReferenceEquals(nextControlPoint, next))
{
RelativeSizeAxes = Axes.Both;
Origin = Anchor.TopLeft;

Width = (float)(endControlPoint.Time - effect.Time);

AddInternal(new PointVisualisation
{
RelativeSizeAxes = Axes.Both,
Origin = Anchor.TopLeft,
Width = 1,
Height = 0.25f,
Depth = float.MaxValue,
Colour = effect.GetRepresentingColour(colours).Darken(0.5f),
});
nextControlPoint = next;
refreshDisplay();
}
}, true);
}, 100, true);
}

private void refreshDisplay()
{
ClearInternal();

AddInternal(new ControlPointVisualisation(effect));

if (!kiai.Value)
return;

// handle kiai duration
// eventually this will be simpler when we have control points with durations.
if (nextControlPoint != null)
{
RelativeSizeAxes = Axes.Both;
Origin = Anchor.TopLeft;

Width = (float)(nextControlPoint.Time - effect.Time);

AddInternal(new PointVisualisation
{
RelativeSizeAxes = Axes.Both,
Origin = Anchor.TopLeft,
Width = 1,
Height = 0.25f,
Depth = float.MaxValue,
Colour = effect.GetRepresentingColour(colours).Darken(0.5f),
});
}
}

// kiai sections display duration, so are required to be visualised.
Expand Down

0 comments on commit f79d749

Please sign in to comment.