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

Fix placing objects via touch in editor not working sometimes #30411

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
83 changes: 73 additions & 10 deletions osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderDrawing.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using osu.Framework.Input;
using osu.Framework.Testing;
using osu.Game.Beatmaps;
using osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders;
using osu.Game.Rulesets.Osu.Objects;
using osu.Game.Rulesets.UI;
using osu.Game.Screens.Edit.Components.RadioButtons;
Expand All @@ -24,38 +25,93 @@ public partial class TestSceneSliderDrawing : TestSceneOsuEditor
protected override IBeatmap CreateBeatmap(RulesetInfo ruleset) => new TestBeatmap(ruleset, false);

[Test]
public void TestTouchInputAfterTouchingComposeArea()
public void TestTouchInputPlaceHitCircleDirectly()
{
AddStep("tap circle", () => tap(this.ChildrenOfType<EditorRadioButton>().Single(b => b.Button.Label == "HitCircle")));

AddStep("tap to place circle", () => tap(this.ChildrenOfType<Playfield>().Single()));
AddAssert("circle placed correctly", () =>
{
var circle = (HitCircle)EditorBeatmap.HitObjects.Single(h => h.StartTime == EditorClock.CurrentTimeAccurate);
Assert.Multiple(() =>
{
Assert.That(circle.Position.X, Is.EqualTo(256f).Within(0.01f));
Assert.That(circle.Position.Y, Is.EqualTo(192f).Within(0.01f));
});

return true;
});
}

[Test]
public void TestTouchInputPlaceCircleAfterTouchingComposeArea()
{
AddStep("tap circle", () => tap(this.ChildrenOfType<EditorRadioButton>().Single(b => b.Button.Label == "HitCircle")));

// this input is just for interacting with compose area
AddStep("tap playfield", () => tap(this.ChildrenOfType<Playfield>().Single()));
AddAssert("circle placed", () => EditorBeatmap.HitObjects.Single(h => h.StartTime == EditorClock.CurrentTimeAccurate) is HitCircle);

AddStep("move current time", () => InputManager.Key(Key.Right));
AddStep("move forward", () => InputManager.Key(Key.Right));

AddStep("tap to place circle", () => tap(this.ChildrenOfType<Playfield>().Single().ToScreenSpace(new Vector2(10, 10))));
AddStep("tap to place circle", () => tap(this.ChildrenOfType<Playfield>().Single()));
AddAssert("circle placed correctly", () =>
{
var circle = (HitCircle)EditorBeatmap.HitObjects.Single(h => h.StartTime == EditorClock.CurrentTimeAccurate);
Assert.Multiple(() =>
{
Assert.That(circle.Position.X, Is.EqualTo(10f).Within(0.01f));
Assert.That(circle.Position.Y, Is.EqualTo(10f).Within(0.01f));
Assert.That(circle.Position.X, Is.EqualTo(256f).Within(0.01f));
Assert.That(circle.Position.Y, Is.EqualTo(192f).Within(0.01f));
});

return true;
});
}

[Test]
public void TestTouchInputPlaceSliderDirectly()
{
AddStep("tap slider", () => tap(this.ChildrenOfType<EditorRadioButton>().Single(b => b.Button.Label == "Slider")));

AddStep("hold to draw slider", () => InputManager.BeginTouch(new Touch(TouchSource.Touch1, this.ChildrenOfType<Playfield>().Single().ToScreenSpace(new Vector2(50, 20)))));
AddStep("drag to draw", () => InputManager.MoveTouchTo(new Touch(TouchSource.Touch1, this.ChildrenOfType<Playfield>().Single().ToScreenSpace(new Vector2(200, 50)))));
AddAssert("selection not initiated", () => this.ChildrenOfType<DragBox>().All(d => d.State == Visibility.Hidden));
AddAssert("blueprint visible", () => this.ChildrenOfType<SliderPlacementBlueprint>().Single().Alpha > 0);
AddStep("end", () => InputManager.EndTouch(new Touch(TouchSource.Touch1, InputManager.CurrentState.Touch.GetTouchPosition(TouchSource.Touch1)!.Value)));
AddAssert("slider placed correctly", () =>
{
var slider = (Slider)EditorBeatmap.HitObjects.Single(h => h.StartTime == EditorClock.CurrentTimeAccurate);
Assert.Multiple(() =>
{
Assert.That(slider.Position.X, Is.EqualTo(50f).Within(0.01f));
Assert.That(slider.Position.Y, Is.EqualTo(20f).Within(0.01f));
Assert.That(slider.Path.ControlPoints.Count, Is.EqualTo(2));
Assert.That(slider.Path.ControlPoints[0].Position, Is.EqualTo(Vector2.Zero));

// the final position may be slightly off from the mouse position when drawing, account for that.
Assert.That(slider.Path.ControlPoints[1].Position.X, Is.EqualTo(150).Within(5));
Assert.That(slider.Path.ControlPoints[1].Position.Y, Is.EqualTo(30).Within(5));
});

return true;
});
}

[Test]
public void TestTouchInputPlaceSliderAfterTouchingComposeArea()
{
AddStep("tap slider", () => tap(this.ChildrenOfType<EditorRadioButton>().Single(b => b.Button.Label == "Slider")));

// this input is just for interacting with compose area
AddStep("tap playfield", () => tap(this.ChildrenOfType<Playfield>().Single()));
AddStep("tap and hold another spot", () => hold(this.ChildrenOfType<Playfield>().Single(), new Vector2(50, 0)));
AddUntilStep("wait for slider placement", () => EditorBeatmap.HitObjects.SingleOrDefault(h => h.StartTime == EditorClock.CurrentTimeAccurate) is Slider);
AddStep("end", () => InputManager.EndTouch(new Touch(TouchSource.Touch1, InputManager.CurrentState.Touch.GetTouchPosition(TouchSource.Touch1)!.Value)));

AddStep("move current time", () => InputManager.Key(Key.Right));
AddStep("move forward", () => InputManager.Key(Key.Right));

AddStep("hold to draw slider", () => InputManager.BeginTouch(new Touch(TouchSource.Touch1, this.ChildrenOfType<Playfield>().Single().ToScreenSpace(new Vector2(50, 20)))));
AddStep("drag to draw", () => InputManager.MoveTouchTo(new Touch(TouchSource.Touch1, this.ChildrenOfType<Playfield>().Single().ToScreenSpace(new Vector2(200, 50)))));
AddAssert("selection not initiated", () => this.ChildrenOfType<DragBox>().All(d => d.State == Visibility.Hidden));
AddAssert("blueprint visible", () => this.ChildrenOfType<SliderPlacementBlueprint>().Single().IsPresent);
AddStep("end", () => InputManager.EndTouch(new Touch(TouchSource.Touch1, InputManager.CurrentState.Touch.GetTouchPosition(TouchSource.Touch1)!.Value)));
AddAssert("slider placed correctly", () =>
{
Expand All @@ -76,12 +132,19 @@ public void TestTouchInputAfterTouchingComposeArea()
});
}

private void tap(Drawable drawable) => tap(drawable.ScreenSpaceDrawQuad.Centre);
private void tap(Drawable drawable, Vector2 offset = default) => tap(drawable.ToScreenSpace(drawable.LayoutRectangle.Centre + offset));

private void tap(Vector2 position)
{
InputManager.BeginTouch(new Touch(TouchSource.Touch1, position));
hold(position);
InputManager.EndTouch(new Touch(TouchSource.Touch1, position));
}

private void hold(Drawable drawable, Vector2 offset = default) => hold(drawable.ToScreenSpace(drawable.LayoutRectangle.Centre + offset));

private void hold(Vector2 position)
{
InputManager.BeginTouch(new Touch(TouchSource.Touch1, position));
}
}
}
25 changes: 19 additions & 6 deletions osu.Game.Rulesets.Osu/Edit/Blueprints/GridPlacementBlueprint.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// See the LICENCE file in the repository root for full licence text.

using osu.Framework.Allocation;
using osu.Framework.Graphics.Containers;
using osu.Framework.Input.Events;
using osu.Game.Rulesets.Edit;
using osuTK;
Expand Down Expand Up @@ -31,12 +32,7 @@ private void load(OsuGridToolboxGroup gridToolboxGroup)
public override void EndPlacement(bool commit)
{
if (!commit && PlacementActive != PlacementState.Finished)
{
gridToolboxGroup.StartPosition.Value = originalOrigin;
gridToolboxGroup.Spacing.Value = originalSpacing;
if (!gridToolboxGroup.GridLinesRotation.Disabled)
gridToolboxGroup.GridLinesRotation.Value = originalRotation;
}
resetGridState();

base.EndPlacement(commit);

Expand Down Expand Up @@ -103,6 +99,9 @@ protected override void OnDragEnd(DragEndEvent e)

public override void UpdateTimeAndPosition(SnapResult result)
{
if (State.Value == Visibility.Hidden)
return;

var pos = ToLocalSpace(result.ScreenSpacePosition);

if (PlacementActive != PlacementState.Active)
Expand All @@ -122,5 +121,19 @@ public override void UpdateTimeAndPosition(SnapResult result)
}
}
}

protected override void PopOut()
{
base.PopOut();
resetGridState();
}

private void resetGridState()
{
gridToolboxGroup.StartPosition.Value = originalOrigin;
gridToolboxGroup.Spacing.Value = originalSpacing;
if (!gridToolboxGroup.GridLinesRotation.Disabled)
gridToolboxGroup.GridLinesRotation.Value = originalRotation;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,13 @@ protected override bool OnMouseDown(MouseDownEvent e)
return true;
}

// this allows sliders to be drawn outside compose area (after starting from a point within the compose area).
public override bool ReceivePositionalInputAt(Vector2 screenSpacePos) => base.ReceivePositionalInputAt(screenSpacePos) || PlacementActive == PlacementState.Active;

// ReceivePositionalInputAtSubTree generally always returns true when masking is disabled, but we don't want that,
// otherwise a slider path tooltip will be displayed anywhere in the editor (outside compose area).
protected override bool ReceivePositionalInputAtSubTree(Vector2 screenSpacePos) => ReceivePositionalInputAt(screenSpacePos);

private void beginNewSegment(PathControlPoint lastPoint)
{
segmentStart = lastPoint;
Expand Down
40 changes: 40 additions & 0 deletions osu.Game.Tests/Visual/Editing/TestSceneHitObjectComposer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
using osu.Game.Rulesets.Osu;
using osu.Game.Rulesets.Osu.Edit;
using osu.Game.Rulesets.Osu.Objects;
using osu.Game.Rulesets.UI;
using osu.Game.Screens.Edit;
using osu.Game.Screens.Edit.Components.RadioButtons;
using osu.Game.Screens.Edit.Components.TernaryButtons;
Expand Down Expand Up @@ -82,6 +83,45 @@ public void SetUpSteps()
});
}

[Test]
public void TestPlacementOutsideComposeScreen()
{
AddStep("clear all control points and hitobjects", () =>
{
editorBeatmap.ControlPointInfo.Clear();
editorBeatmap.Clear();
});

AddStep("Add timing point", () => editorBeatmap.ControlPointInfo.Add(0, new TimingControlPoint()));
AddStep("select circle", () => hitObjectComposer.ChildrenOfType<EditorRadioButton>().First(d => d.Button.Label == "HitCircle").TriggerClick());
AddStep("move mouse to compose", () => InputManager.MoveMouseTo(hitObjectComposer.ChildrenOfType<HitObjectContainer>().Single()));
AddStep("click", () => InputManager.Click(MouseButton.Left));
AddAssert("circle placed", () => editorBeatmap.HitObjects.Count == 1);

AddStep("move mouse outside compose", () => InputManager.MoveMouseTo(hitObjectComposer.ChildrenOfType<HitObjectContainer>().Single().ScreenSpaceDrawQuad.TopLeft - new Vector2(0f, 20f)));
AddStep("click", () => InputManager.Click(MouseButton.Left));
AddAssert("no circle placed", () => editorBeatmap.HitObjects.Count == 1);
}

[Test]
public void TestDragSliderOutsideComposeScreen()
{
AddStep("clear all control points and hitobjects", () =>
{
editorBeatmap.ControlPointInfo.Clear();
editorBeatmap.Clear();
});

AddStep("Add timing point", () => editorBeatmap.ControlPointInfo.Add(0, new TimingControlPoint()));
AddStep("select slider", () => hitObjectComposer.ChildrenOfType<EditorRadioButton>().First(d => d.Button.Label == "Slider").TriggerClick());

AddStep("move mouse to compose", () => InputManager.MoveMouseTo(hitObjectComposer.ChildrenOfType<HitObjectContainer>().Single()));
AddStep("hold", () => InputManager.PressButton(MouseButton.Left));
AddStep("move mouse outside compose", () => InputManager.MoveMouseTo(hitObjectComposer.ChildrenOfType<HitObjectContainer>().Single().ScreenSpaceDrawQuad.TopLeft - new Vector2(0f, 80f)));
AddStep("release", () => InputManager.ReleaseButton(MouseButton.Left));
AddAssert("slider placed", () => editorBeatmap.HitObjects.Count == 1);
}

[Test]
public void TestPlacementOnlyWorksWithTiming()
{
Expand Down
17 changes: 9 additions & 8 deletions osu.Game/Rulesets/Edit/HitObjectComposer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -537,22 +537,23 @@ private void toolSelected(CompositionTool tool)

#region IPlacementHandler

public void BeginPlacement(HitObject hitObject)
public void ShowPlacement(HitObject hitObject)
{
EditorBeatmap.PlacementObject.Value = hitObject;
}

public void EndPlacement(HitObject hitObject, bool commit)
public void HidePlacement()
{
EditorBeatmap.PlacementObject.Value = null;
}

if (commit)
{
EditorBeatmap.Add(hitObject);
public void CommitPlacement(HitObject hitObject)
{
EditorBeatmap.PlacementObject.Value = null;
EditorBeatmap.Add(hitObject);

if (autoSeekOnPlacement.Value && EditorClock.CurrentTime < hitObject.StartTime)
EditorClock.SeekSmoothlyTo(hitObject.StartTime);
}
if (autoSeekOnPlacement.Value && EditorClock.CurrentTime < hitObject.StartTime)
EditorClock.SeekSmoothlyTo(hitObject.StartTime);
}

public void Delete(HitObject hitObject) => EditorBeatmap.Remove(hitObject);
Expand Down
27 changes: 25 additions & 2 deletions osu.Game/Rulesets/Edit/HitObjectPlacementBlueprint.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Threading;
using osu.Framework.Allocation;
using osu.Framework.Bindables;
using osu.Framework.Graphics.Containers;
using osu.Game.Audio;
using osu.Game.Beatmaps;
using osu.Game.Beatmaps.ControlPoints;
Expand Down Expand Up @@ -63,18 +64,26 @@ private void load()
startTimeBindable.BindValueChanged(_ => ApplyDefaultsToHitObject(), true);
}

private bool placementBegun;

protected override void BeginPlacement(bool commitStart = false)
{
base.BeginPlacement(commitStart);

placementHandler.BeginPlacement(HitObject);
if (State.Value == Visibility.Visible)
placementHandler.ShowPlacement(HitObject);

placementBegun = true;
}

public override void EndPlacement(bool commit)
{
base.EndPlacement(commit);

placementHandler.EndPlacement(HitObject, IsValidForPlacement && commit);
if (IsValidForPlacement && commit)
placementHandler.CommitPlacement(HitObject);
else
placementHandler.HidePlacement();
}

/// <summary>
Expand Down Expand Up @@ -127,5 +136,19 @@ public override void UpdateTimeAndPosition(SnapResult result)
/// refreshing <see cref="Objects.HitObject.NestedHitObjects"/> and parameters for the <see cref="HitObject"/>.
/// </summary>
protected void ApplyDefaultsToHitObject() => HitObject.ApplyDefaults(beatmap.ControlPointInfo, beatmap.Difficulty);

protected override void PopIn()
{
base.PopIn();

if (placementBegun)
placementHandler.ShowPlacement(HitObject);
}

protected override void PopOut()
{
base.PopOut();
placementHandler.HidePlacement();
}
}
}
17 changes: 11 additions & 6 deletions osu.Game/Rulesets/Edit/PlacementBlueprint.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,14 @@
using osu.Framework.Input.Events;
using osu.Game.Input.Bindings;
using osu.Game.Rulesets.Objects;
using osuTK;
using osuTK.Input;

namespace osu.Game.Rulesets.Edit
{
/// <summary>
/// A blueprint which governs the placement of something.
/// </summary>
public abstract partial class PlacementBlueprint : CompositeDrawable, IKeyBindingHandler<GlobalAction>
public abstract partial class PlacementBlueprint : VisibilityContainer, IKeyBindingHandler<GlobalAction>
{
/// <summary>
/// Whether the <see cref="HitObject"/> is currently mid-placement, but has not necessarily finished being placed.
Expand All @@ -31,12 +30,17 @@ public abstract partial class PlacementBlueprint : CompositeDrawable, IKeyBindin
/// </remarks>
protected virtual bool IsValidForPlacement => true;

// the blueprint should still be considered for input even if it is hidden,
// especially when such input is the reason for making the blueprint become visible.
public override bool PropagatePositionalInputSubTree => true;
public override bool PropagateNonPositionalInputSubTree => true;

protected PlacementBlueprint()
{
RelativeSizeAxes = Axes.Both;

// This is required to allow the blueprint's position to be updated via OnMouseMove/Handle
// on the same frame it is made visible via a PlacementState change.
// the blueprint should still be considered for input even if it is hidden,
// especially when such input is the reason for making the blueprint become visible.
AlwaysPresent = true;
}

Expand Down Expand Up @@ -104,8 +108,6 @@ public void OnReleased(KeyBindingReleaseEvent<GlobalAction> e)
{
}

public override bool ReceivePositionalInputAt(Vector2 screenSpacePos) => true;

protected override bool Handle(UIEvent e)
{
base.Handle(e);
Expand All @@ -127,6 +129,9 @@ protected override bool Handle(UIEvent e)
}
}

protected override void PopIn() => this.FadeIn();
protected override void PopOut() => this.FadeOut();

public enum PlacementState
{
Waiting,
Expand Down
Loading
Loading