Skip to content

Commit 78084e3

Browse files
authored
Merge pull request #30411 from frenzibyte/editor-slider-touch-support-2
Fix placing objects via touch in editor not working sometimes
2 parents c25215d + 0e65655 commit 78084e3

File tree

10 files changed

+222
-63
lines changed

10 files changed

+222
-63
lines changed

Diff for: osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderDrawing.cs

+73-10
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
using osu.Framework.Input;
99
using osu.Framework.Testing;
1010
using osu.Game.Beatmaps;
11+
using osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders;
1112
using osu.Game.Rulesets.Osu.Objects;
1213
using osu.Game.Rulesets.UI;
1314
using osu.Game.Screens.Edit.Components.RadioButtons;
@@ -24,38 +25,93 @@ public partial class TestSceneSliderDrawing : TestSceneOsuEditor
2425
protected override IBeatmap CreateBeatmap(RulesetInfo ruleset) => new TestBeatmap(ruleset, false);
2526

2627
[Test]
27-
public void TestTouchInputAfterTouchingComposeArea()
28+
public void TestTouchInputPlaceHitCircleDirectly()
29+
{
30+
AddStep("tap circle", () => tap(this.ChildrenOfType<EditorRadioButton>().Single(b => b.Button.Label == "HitCircle")));
31+
32+
AddStep("tap to place circle", () => tap(this.ChildrenOfType<Playfield>().Single()));
33+
AddAssert("circle placed correctly", () =>
34+
{
35+
var circle = (HitCircle)EditorBeatmap.HitObjects.Single(h => h.StartTime == EditorClock.CurrentTimeAccurate);
36+
Assert.Multiple(() =>
37+
{
38+
Assert.That(circle.Position.X, Is.EqualTo(256f).Within(0.01f));
39+
Assert.That(circle.Position.Y, Is.EqualTo(192f).Within(0.01f));
40+
});
41+
42+
return true;
43+
});
44+
}
45+
46+
[Test]
47+
public void TestTouchInputPlaceCircleAfterTouchingComposeArea()
2848
{
2949
AddStep("tap circle", () => tap(this.ChildrenOfType<EditorRadioButton>().Single(b => b.Button.Label == "HitCircle")));
3050

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

34-
AddStep("move current time", () => InputManager.Key(Key.Right));
54+
AddStep("move forward", () => InputManager.Key(Key.Right));
3555

36-
AddStep("tap to place circle", () => tap(this.ChildrenOfType<Playfield>().Single().ToScreenSpace(new Vector2(10, 10))));
56+
AddStep("tap to place circle", () => tap(this.ChildrenOfType<Playfield>().Single()));
3757
AddAssert("circle placed correctly", () =>
3858
{
3959
var circle = (HitCircle)EditorBeatmap.HitObjects.Single(h => h.StartTime == EditorClock.CurrentTimeAccurate);
4060
Assert.Multiple(() =>
4161
{
42-
Assert.That(circle.Position.X, Is.EqualTo(10f).Within(0.01f));
43-
Assert.That(circle.Position.Y, Is.EqualTo(10f).Within(0.01f));
62+
Assert.That(circle.Position.X, Is.EqualTo(256f).Within(0.01f));
63+
Assert.That(circle.Position.Y, Is.EqualTo(192f).Within(0.01f));
64+
});
65+
66+
return true;
67+
});
68+
}
69+
70+
[Test]
71+
public void TestTouchInputPlaceSliderDirectly()
72+
{
73+
AddStep("tap slider", () => tap(this.ChildrenOfType<EditorRadioButton>().Single(b => b.Button.Label == "Slider")));
74+
75+
AddStep("hold to draw slider", () => InputManager.BeginTouch(new Touch(TouchSource.Touch1, this.ChildrenOfType<Playfield>().Single().ToScreenSpace(new Vector2(50, 20)))));
76+
AddStep("drag to draw", () => InputManager.MoveTouchTo(new Touch(TouchSource.Touch1, this.ChildrenOfType<Playfield>().Single().ToScreenSpace(new Vector2(200, 50)))));
77+
AddAssert("selection not initiated", () => this.ChildrenOfType<DragBox>().All(d => d.State == Visibility.Hidden));
78+
AddAssert("blueprint visible", () => this.ChildrenOfType<SliderPlacementBlueprint>().Single().Alpha > 0);
79+
AddStep("end", () => InputManager.EndTouch(new Touch(TouchSource.Touch1, InputManager.CurrentState.Touch.GetTouchPosition(TouchSource.Touch1)!.Value)));
80+
AddAssert("slider placed correctly", () =>
81+
{
82+
var slider = (Slider)EditorBeatmap.HitObjects.Single(h => h.StartTime == EditorClock.CurrentTimeAccurate);
83+
Assert.Multiple(() =>
84+
{
85+
Assert.That(slider.Position.X, Is.EqualTo(50f).Within(0.01f));
86+
Assert.That(slider.Position.Y, Is.EqualTo(20f).Within(0.01f));
87+
Assert.That(slider.Path.ControlPoints.Count, Is.EqualTo(2));
88+
Assert.That(slider.Path.ControlPoints[0].Position, Is.EqualTo(Vector2.Zero));
89+
90+
// the final position may be slightly off from the mouse position when drawing, account for that.
91+
Assert.That(slider.Path.ControlPoints[1].Position.X, Is.EqualTo(150).Within(5));
92+
Assert.That(slider.Path.ControlPoints[1].Position.Y, Is.EqualTo(30).Within(5));
4493
});
4594

4695
return true;
4796
});
97+
}
4898

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

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

54-
AddStep("move current time", () => InputManager.Key(Key.Right));
109+
AddStep("move forward", () => InputManager.Key(Key.Right));
55110

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

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

81137
private void tap(Vector2 position)
82138
{
83-
InputManager.BeginTouch(new Touch(TouchSource.Touch1, position));
139+
hold(position);
84140
InputManager.EndTouch(new Touch(TouchSource.Touch1, position));
85141
}
142+
143+
private void hold(Drawable drawable, Vector2 offset = default) => hold(drawable.ToScreenSpace(drawable.LayoutRectangle.Centre + offset));
144+
145+
private void hold(Vector2 position)
146+
{
147+
InputManager.BeginTouch(new Touch(TouchSource.Touch1, position));
148+
}
86149
}
87150
}

Diff for: osu.Game.Rulesets.Osu/Edit/Blueprints/GridPlacementBlueprint.cs

+19-6
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// See the LICENCE file in the repository root for full licence text.
33

44
using osu.Framework.Allocation;
5+
using osu.Framework.Graphics.Containers;
56
using osu.Framework.Input.Events;
67
using osu.Game.Rulesets.Edit;
78
using osuTK;
@@ -31,12 +32,7 @@ private void load(OsuGridToolboxGroup gridToolboxGroup)
3132
public override void EndPlacement(bool commit)
3233
{
3334
if (!commit && PlacementActive != PlacementState.Finished)
34-
{
35-
gridToolboxGroup.StartPosition.Value = originalOrigin;
36-
gridToolboxGroup.Spacing.Value = originalSpacing;
37-
if (!gridToolboxGroup.GridLinesRotation.Disabled)
38-
gridToolboxGroup.GridLinesRotation.Value = originalRotation;
39-
}
35+
resetGridState();
4036

4137
base.EndPlacement(commit);
4238

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

104100
public override void UpdateTimeAndPosition(SnapResult result)
105101
{
102+
if (State.Value == Visibility.Hidden)
103+
return;
104+
106105
var pos = ToLocalSpace(result.ScreenSpacePosition);
107106

108107
if (PlacementActive != PlacementState.Active)
@@ -122,5 +121,19 @@ public override void UpdateTimeAndPosition(SnapResult result)
122121
}
123122
}
124123
}
124+
125+
protected override void PopOut()
126+
{
127+
base.PopOut();
128+
resetGridState();
129+
}
130+
131+
private void resetGridState()
132+
{
133+
gridToolboxGroup.StartPosition.Value = originalOrigin;
134+
gridToolboxGroup.Spacing.Value = originalSpacing;
135+
if (!gridToolboxGroup.GridLinesRotation.Disabled)
136+
gridToolboxGroup.GridLinesRotation.Value = originalRotation;
137+
}
125138
}
126139
}

Diff for: osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderPlacementBlueprint.cs

+7
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,13 @@ protected override bool OnMouseDown(MouseDownEvent e)
156156
return true;
157157
}
158158

159+
// this allows sliders to be drawn outside compose area (after starting from a point within the compose area).
160+
public override bool ReceivePositionalInputAt(Vector2 screenSpacePos) => base.ReceivePositionalInputAt(screenSpacePos) || PlacementActive == PlacementState.Active;
161+
162+
// ReceivePositionalInputAtSubTree generally always returns true when masking is disabled, but we don't want that,
163+
// otherwise a slider path tooltip will be displayed anywhere in the editor (outside compose area).
164+
protected override bool ReceivePositionalInputAtSubTree(Vector2 screenSpacePos) => ReceivePositionalInputAt(screenSpacePos);
165+
159166
private void beginNewSegment(PathControlPoint lastPoint)
160167
{
161168
segmentStart = lastPoint;

Diff for: osu.Game.Tests/Visual/Editing/TestSceneHitObjectComposer.cs

+40
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
using osu.Game.Rulesets.Osu;
2020
using osu.Game.Rulesets.Osu.Edit;
2121
using osu.Game.Rulesets.Osu.Objects;
22+
using osu.Game.Rulesets.UI;
2223
using osu.Game.Screens.Edit;
2324
using osu.Game.Screens.Edit.Components.RadioButtons;
2425
using osu.Game.Screens.Edit.Components.TernaryButtons;
@@ -82,6 +83,45 @@ public void SetUpSteps()
8283
});
8384
}
8485

86+
[Test]
87+
public void TestPlacementOutsideComposeScreen()
88+
{
89+
AddStep("clear all control points and hitobjects", () =>
90+
{
91+
editorBeatmap.ControlPointInfo.Clear();
92+
editorBeatmap.Clear();
93+
});
94+
95+
AddStep("Add timing point", () => editorBeatmap.ControlPointInfo.Add(0, new TimingControlPoint()));
96+
AddStep("select circle", () => hitObjectComposer.ChildrenOfType<EditorRadioButton>().First(d => d.Button.Label == "HitCircle").TriggerClick());
97+
AddStep("move mouse to compose", () => InputManager.MoveMouseTo(hitObjectComposer.ChildrenOfType<HitObjectContainer>().Single()));
98+
AddStep("click", () => InputManager.Click(MouseButton.Left));
99+
AddAssert("circle placed", () => editorBeatmap.HitObjects.Count == 1);
100+
101+
AddStep("move mouse outside compose", () => InputManager.MoveMouseTo(hitObjectComposer.ChildrenOfType<HitObjectContainer>().Single().ScreenSpaceDrawQuad.TopLeft - new Vector2(0f, 20f)));
102+
AddStep("click", () => InputManager.Click(MouseButton.Left));
103+
AddAssert("no circle placed", () => editorBeatmap.HitObjects.Count == 1);
104+
}
105+
106+
[Test]
107+
public void TestDragSliderOutsideComposeScreen()
108+
{
109+
AddStep("clear all control points and hitobjects", () =>
110+
{
111+
editorBeatmap.ControlPointInfo.Clear();
112+
editorBeatmap.Clear();
113+
});
114+
115+
AddStep("Add timing point", () => editorBeatmap.ControlPointInfo.Add(0, new TimingControlPoint()));
116+
AddStep("select slider", () => hitObjectComposer.ChildrenOfType<EditorRadioButton>().First(d => d.Button.Label == "Slider").TriggerClick());
117+
118+
AddStep("move mouse to compose", () => InputManager.MoveMouseTo(hitObjectComposer.ChildrenOfType<HitObjectContainer>().Single()));
119+
AddStep("hold", () => InputManager.PressButton(MouseButton.Left));
120+
AddStep("move mouse outside compose", () => InputManager.MoveMouseTo(hitObjectComposer.ChildrenOfType<HitObjectContainer>().Single().ScreenSpaceDrawQuad.TopLeft - new Vector2(0f, 80f)));
121+
AddStep("release", () => InputManager.ReleaseButton(MouseButton.Left));
122+
AddAssert("slider placed", () => editorBeatmap.HitObjects.Count == 1);
123+
}
124+
85125
[Test]
86126
public void TestPlacementOnlyWorksWithTiming()
87127
{

Diff for: osu.Game/Rulesets/Edit/HitObjectComposer.cs

+9-8
Original file line numberDiff line numberDiff line change
@@ -537,22 +537,23 @@ private void toolSelected(CompositionTool tool)
537537

538538
#region IPlacementHandler
539539

540-
public void BeginPlacement(HitObject hitObject)
540+
public void ShowPlacement(HitObject hitObject)
541541
{
542542
EditorBeatmap.PlacementObject.Value = hitObject;
543543
}
544544

545-
public void EndPlacement(HitObject hitObject, bool commit)
545+
public void HidePlacement()
546546
{
547547
EditorBeatmap.PlacementObject.Value = null;
548+
}
548549

549-
if (commit)
550-
{
551-
EditorBeatmap.Add(hitObject);
550+
public void CommitPlacement(HitObject hitObject)
551+
{
552+
EditorBeatmap.PlacementObject.Value = null;
553+
EditorBeatmap.Add(hitObject);
552554

553-
if (autoSeekOnPlacement.Value && EditorClock.CurrentTime < hitObject.StartTime)
554-
EditorClock.SeekSmoothlyTo(hitObject.StartTime);
555-
}
555+
if (autoSeekOnPlacement.Value && EditorClock.CurrentTime < hitObject.StartTime)
556+
EditorClock.SeekSmoothlyTo(hitObject.StartTime);
556557
}
557558

558559
public void Delete(HitObject hitObject) => EditorBeatmap.Remove(hitObject);

Diff for: osu.Game/Rulesets/Edit/HitObjectPlacementBlueprint.cs

+25-2
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using System.Threading;
66
using osu.Framework.Allocation;
77
using osu.Framework.Bindables;
8+
using osu.Framework.Graphics.Containers;
89
using osu.Game.Audio;
910
using osu.Game.Beatmaps;
1011
using osu.Game.Beatmaps.ControlPoints;
@@ -63,18 +64,26 @@ private void load()
6364
startTimeBindable.BindValueChanged(_ => ApplyDefaultsToHitObject(), true);
6465
}
6566

67+
private bool placementBegun;
68+
6669
protected override void BeginPlacement(bool commitStart = false)
6770
{
6871
base.BeginPlacement(commitStart);
6972

70-
placementHandler.BeginPlacement(HitObject);
73+
if (State.Value == Visibility.Visible)
74+
placementHandler.ShowPlacement(HitObject);
75+
76+
placementBegun = true;
7177
}
7278

7379
public override void EndPlacement(bool commit)
7480
{
7581
base.EndPlacement(commit);
7682

77-
placementHandler.EndPlacement(HitObject, IsValidForPlacement && commit);
83+
if (IsValidForPlacement && commit)
84+
placementHandler.CommitPlacement(HitObject);
85+
else
86+
placementHandler.HidePlacement();
7887
}
7988

8089
/// <summary>
@@ -127,5 +136,19 @@ public override void UpdateTimeAndPosition(SnapResult result)
127136
/// refreshing <see cref="Objects.HitObject.NestedHitObjects"/> and parameters for the <see cref="HitObject"/>.
128137
/// </summary>
129138
protected void ApplyDefaultsToHitObject() => HitObject.ApplyDefaults(beatmap.ControlPointInfo, beatmap.Difficulty);
139+
140+
protected override void PopIn()
141+
{
142+
base.PopIn();
143+
144+
if (placementBegun)
145+
placementHandler.ShowPlacement(HitObject);
146+
}
147+
148+
protected override void PopOut()
149+
{
150+
base.PopOut();
151+
placementHandler.HidePlacement();
152+
}
130153
}
131154
}

Diff for: osu.Game/Rulesets/Edit/PlacementBlueprint.cs

+11-6
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,14 @@
77
using osu.Framework.Input.Events;
88
using osu.Game.Input.Bindings;
99
using osu.Game.Rulesets.Objects;
10-
using osuTK;
1110
using osuTK.Input;
1211

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

33+
// the blueprint should still be considered for input even if it is hidden,
34+
// especially when such input is the reason for making the blueprint become visible.
35+
public override bool PropagatePositionalInputSubTree => true;
36+
public override bool PropagateNonPositionalInputSubTree => true;
37+
3438
protected PlacementBlueprint()
3539
{
3640
RelativeSizeAxes = Axes.Both;
3741

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

@@ -104,8 +108,6 @@ public void OnReleased(KeyBindingReleaseEvent<GlobalAction> e)
104108
{
105109
}
106110

107-
public override bool ReceivePositionalInputAt(Vector2 screenSpacePos) => true;
108-
109111
protected override bool Handle(UIEvent e)
110112
{
111113
base.Handle(e);
@@ -127,6 +129,9 @@ protected override bool Handle(UIEvent e)
127129
}
128130
}
129131

132+
protected override void PopIn() => this.FadeIn();
133+
protected override void PopOut() => this.FadeOut();
134+
130135
public enum PlacementState
131136
{
132137
Waiting,

0 commit comments

Comments
 (0)