Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,55 @@ public void TestDragHoldNoteTail()
AddAssert("tail blueprint positioned correctly", () => this.ChildrenOfType<EditHoldNoteEndPiece>().ElementAt(1).DrawPosition == drawableHoldNote.Tail.DrawPosition);
}

[Test]
public void TestDragHoldNoteTailOutsidePlayfield()
{
setScrollStep(ScrollingDirection.Down);

HoldNote holdNote = null;
AddStep("setup beatmap", () =>
{
composer.EditorBeatmap.Clear();
composer.EditorBeatmap.Add(holdNote = new HoldNote
{
Column = 1,
StartTime = 250,
EndTime = 750,
});
});

DrawableHoldNote drawableHoldNote = null;
EditHoldNoteEndPiece tailPiece = null;

AddStep("select blueprint", () =>
{
drawableHoldNote = this.ChildrenOfType<DrawableHoldNote>().Single();
InputManager.MoveMouseTo(drawableHoldNote);
InputManager.Click(MouseButton.Left);
});
AddStep("grab hold note tail", () =>
{
tailPiece = this.ChildrenOfType<EditHoldNoteEndPiece>().Last();
InputManager.MoveMouseTo(tailPiece);
InputManager.PressButton(MouseButton.Left);
});

AddStep("drag tail upwards", () =>
{
InputManager.MoveMouseTo(tailPiece, new Vector2(-500, -120));
InputManager.ReleaseButton(MouseButton.Left);
});

AddAssert("start time unchanged", () => holdNote!.StartTime, () => Is.EqualTo(250));
AddAssert("end time is snapped", () => holdNote.EndTime % 125, () => Is.Zero);

AddAssert("head note positioned correctly", () => Precision.AlmostEquals(drawableHoldNote.ScreenSpaceDrawQuad.BottomLeft, drawableHoldNote.Head.ScreenSpaceDrawQuad.BottomLeft));
AddAssert("tail note positioned correctly", () => Precision.AlmostEquals(drawableHoldNote.ScreenSpaceDrawQuad.TopLeft, drawableHoldNote.Tail.ScreenSpaceDrawQuad.BottomLeft));

AddAssert("head blueprint positioned correctly", () => this.ChildrenOfType<EditHoldNoteEndPiece>().ElementAt(0).DrawPosition == drawableHoldNote.Head.DrawPosition);
AddAssert("tail blueprint positioned correctly", () => this.ChildrenOfType<EditHoldNoteEndPiece>().ElementAt(1).DrawPosition == drawableHoldNote.Tail.DrawPosition);
}

private void setScrollStep(ScrollingDirection direction)
=> AddStep($"set scroll direction = {direction}", () => ((Bindable<ScrollingDirection>)composer.Composer.ScrollingInfo.Direction).Value = direction);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

namespace osu.Game.Rulesets.Mania.Tests.Editor
{
public partial class TestSceneHoldNoteTailDrag : EditorTestScene
public partial class TestSceneTimelineHoldNote : EditorTestScene
{
protected override Ruleset CreateEditorRuleset() => new ManiaRuleset();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@ protected override bool OnMouseDown(MouseDownEvent e)
if (e.Button != MouseButton.Left)
return false;

if (!IsValidForPlacement)
return false;

if (Column == null)
return false;

Expand Down
2 changes: 1 addition & 1 deletion osu.Game.Rulesets.Mania/Edit/ManiaHitObjectComposer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public ManiaHitObjectComposer(Ruleset ruleset)
public IScrollingInfo ScrollingInfo => drawableRuleset.ScrollingInfo;

protected override Playfield PlayfieldAtScreenSpacePosition(Vector2 screenSpacePosition) =>
Playfield.GetColumnByPosition(screenSpacePosition);
Playfield.GetClosestColumnByPosition(screenSpacePosition.X);

protected override DrawableRuleset<ManiaHitObject> CreateDrawableRuleset(Ruleset ruleset, IBeatmap beatmap, IReadOnlyList<Mod> mods) =>
drawableRuleset = new DrawableManiaEditorRuleset(ruleset, beatmap, mods);
Expand Down
2 changes: 1 addition & 1 deletion osu.Game.Rulesets.Mania/Edit/ManiaSelectionHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ private void performColumnMovement(int lastColumn, MoveSelectionEvent<HitObject>
{
var maniaPlayfield = ((ManiaHitObjectComposer)composer).Playfield;

var currentColumn = maniaPlayfield.GetColumnByPosition(moveEvent.Blueprint.ScreenSpaceSelectionPoint + moveEvent.ScreenSpaceDelta);
var currentColumn = maniaPlayfield.GetClosestColumnByPosition((moveEvent.Blueprint.ScreenSpaceSelectionPoint + moveEvent.ScreenSpaceDelta).X);
if (currentColumn == null)
return;

Expand Down
30 changes: 8 additions & 22 deletions osu.Game.Rulesets.Mania/UI/ManiaPlayfield.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using osu.Framework.Graphics.Containers;
using System;
using System.Collections.Generic;
using System.Linq;
using JetBrains.Annotations;
using osu.Framework.Allocation;
using osu.Framework.Graphics.Primitives;
Expand Down Expand Up @@ -97,30 +98,15 @@ public ManiaPlayfield(List<StageDefinition> stageDefinitions)
public void Add(BarLine barLine) => stages.ForEach(s => s.Add(barLine));

/// <summary>
/// Retrieves a column from a screen-space position.
/// Find the closest column to the proposed screen space position.
/// </summary>
/// <param name="screenSpacePosition">The screen-space position.</param>
/// <returns>The column which the <paramref name="screenSpacePosition"/> lies in.</returns>
public Column GetColumnByPosition(Vector2 screenSpacePosition)
/// <param name="screenSpaceX">The screen-space X coordinate.</param>
/// <returns>The column which the <paramref name="screenSpaceX"/> is closest to.</returns>
public Column GetClosestColumnByPosition(float screenSpaceX)
{
Column found = null;

foreach (var stage in stages)
{
foreach (var column in stage.Columns)
{
if (column.ReceivePositionalInputAt(new Vector2(screenSpacePosition.X, column.ScreenSpaceDrawQuad.Centre.Y)))
{
found = column;
break;
}
}

if (found != null)
break;
}

return found;
return stages
.SelectMany(s => s.Columns.Select((Column column, float distance) (c) => (c, Math.Abs(screenSpaceX - c.ScreenSpaceDrawQuad.Centre.X))))
.MinBy(c => c.distance).column;
}

/// <summary>
Expand Down
2 changes: 1 addition & 1 deletion osu.Game/Rulesets/Edit/HitObjectPlacementBlueprint.cs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public abstract partial class HitObjectPlacementBlueprint : PlacementBlueprint

private HitObject? getPreviousHitObject() => beatmap.HitObjects.TakeWhile(h => h.StartTime <= startTimeBindable.Value).LastOrDefault();

protected override bool IsValidForPlacement => HitObject.StartTime >= beatmap.ControlPointInfo.TimingPoints.FirstOrDefault()?.Time;
protected override bool IsValidForPlacement => base.IsValidForPlacement && HitObject.StartTime >= beatmap.ControlPointInfo.TimingPoints.FirstOrDefault()?.Time;

[Resolved]
private IPlacementHandler placementHandler { get; set; } = null!;
Expand Down
9 changes: 8 additions & 1 deletion osu.Game/Rulesets/Edit/PlacementBlueprint.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// 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.

using osu.Framework.Allocation;
using osu.Framework.Graphics;
using osu.Framework.Graphics.Containers;
using osu.Framework.Input.Bindings;
Expand Down Expand Up @@ -29,13 +30,16 @@ public abstract partial class PlacementBlueprint : VisibilityContainer, IKeyBind
/// Override this with any preconditions that should be double-checked on committing.
/// If <c>false</c> is returned and a commit is attempted, the blueprint will be destroyed instead.
/// </remarks>
protected virtual bool IsValidForPlacement => true;
protected virtual bool IsValidForPlacement => PlacementActive != PlacementState.Waiting || hitObjectComposer.CursorInPlacementArea;
Copy link
Collaborator

@bdach bdach Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On a superficial check this change looks potentially dicey. The conditions look right but I dunno that enforcing them at this deep a level (in terms of inheritance hierarchy, I mean) is correct.

The rest of the commit seems fine. Not sure whether you expected me to run this, all I did was an ocular patdown of the diff.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, pretty much fine with that.


// 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;

[Resolved]
private HitObjectComposer hitObjectComposer { get; set; } = null!;

protected PlacementBlueprint()
{
RelativeSizeAxes = Axes.Both;
Expand Down Expand Up @@ -69,6 +73,9 @@ public virtual void EndPlacement(bool commit)

case PlacementState.Waiting:
// ensure placement was started before ending to make state handling simpler.
if (!IsValidForPlacement)
return;

BeginPlacement();
break;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ public abstract partial class EditorBlueprintContainer : BlueprintContainer<HitO
[Resolved]
protected EditorBeatmap Beatmap { get; private set; }

[Cached]
protected readonly HitObjectComposer Composer;

private HitObjectUsageEventBuffer usageEventBuffer;
Expand Down
Loading