Editor not selecting correct object when Objects are stacked

[Eeveelution]

Describe the bug: Ideally in osu!stable, when objects are stacked, the latest object gets selected. in lazer however it defaults to the first object that is stacked.

Screenshots or videos showing encountered issue: Expected Behaviour: https://streamable.com/gw5nqm Current Behaviour: https://streamable.com/ai8pbv

osu!lazer version: 2020.1128.0

Logs: runtime.log updater.log database.log performance.log

To reproduce, stack 2 sliders or objects on top of each other and try selecting.

(i hope this hasnt been reported before and that its a new issue. im really sorry if i post 2 duplicates in 1 day)

[peppy]

Even in the case where the user is clicking on a lower object in the stack (on the circle's border, let's say) would you still expect the lowest object in the stack to be selected?

[Eeveelution]

On the current stable version when clicking on the circle border, it selects the circle of which the border belongs to. so i think it'd be ideal to have the same behaviour in lazer too

[Grrum]

In osu/osu.Game/Screens/Edit/Compose/Components/BlueprintContainer.cs, I found this function that I think controls the selection referenced in the issue.

private bool beginClickSelection(MouseButtonEvent e)
{
    // Iterate from the top of the input stack (blueprints closest to the front of the screen first).
    foreach (SelectionBlueprint blueprint in SelectionBlueprints.AliveChildren.Reverse())
    {
        if (!blueprint.IsHovered) continue;

        return clickSelectionBegan = SelectionHandler.HandleSelectionRequested(blueprint, e);
    }

    return false;
}

The comment seems to confirm the behavior referenced in this issue.

I can write a comparison function that takes two blueprints and says which was one is the latest (based on EditorClock.CurrentTime and blueprint.HitObject.StartTime). Then AliveChildren can be sorted based on the latest object. Iterating off that list instead of AliveChildren.Reverse() should create the expected behavior.

[peppy]

I'd probably check whether that approach is actually any different from just dropping the .Reverse() and calling it a day.

Importantly, make sure to test the case of circle stacks, not just the one in the video (to make sure it matches osu-stable behaviour at very least).

[Grrum]

Dropping the .Reverse() leaves the objects in the opposite order (the objects later in time are first in the list) which is not the behavior we want. The circle stack situation works very similar to sliders that this isn't something to make a specific test case for.

The current osu-stable behavior is to take the closest (edit: temporally) hit object regardless if it's before or after the current time. This also considers slider/spinner ends. That's different from what I interpreted from Eevee's comment "the latest object gets selected" which would mean the closest object before the current time, but not after. I'm fine with implementing the current osu-stable version over the proposed one, but I'd figure I should notify @Eeveelution to get their input on whether the mapper should want the latest object or the closest object.

I'm also trying to learn how to write tests in this codebase. Would you recommend both a unit test around the compare function and a UI level test like the ones in TestSceneSliderPlacementBlueprint.cs? I'm guessing these tests would go in some new file like osu.Game.Tests/Editing/TestSceneBlueprintContainer.cs?

[bdach]

i think the most intuitive UX here is, quoting above:

On the current stable version when clicking on the circle border, it selects the circle of which the border belongs to.

isn't it?

The current osu-stable behavior is to take the closest hit object regardless

not sure i'm interpreting correctly. to clarify - closer temporally or spatially?

Would you recommend both a unit test around the compare function and a UI level test

just a normal test scene is fine. no reason to go overboard with it. not sure about the new file name, something like TestSceneStackSelection or similar would probably fit better.

[Grrum]

not sure i'm interpreting correctly. to clarify - closer temporally or spatially?

Ah sorry yeah, I meant closer temporally. So the object that is the least amount of time away from the current time in either direction is the one selected if both are hovered.