ppy / osu

rhythm is just a *click* away!
https://osu.ppy.sh
MIT License
14.91k stars 2.2k forks source link

Sliders can have zero length #10959

Open Pennek opened 3 years ago

Pennek commented 3 years ago

Sliders and spinners can be modified to have zero length. Sliders by using transformation tools, and spinners by dragging in the timeline. It looks like spinners can also be dragged into negative values (as seen on the video where only the number is left)

video: https://youtu.be/ypFaliLLgi8 osu!lazer version: 2020.1121.0

vmaggioli commented 3 years ago

Any updates on a potential solution? I'm debugging through just to see if I can find what's causing it and it seems like the following snippet should handle this case (for spinners at least) https://github.com/ppy/osu/blob/fd45d8c95a644b3c45d0cbe9ccad034957b269ce/osu.Game/Screens/Edit/Compose/Components/Timeline/TimelineHitObjectBlueprint.cs#L390-L391

bdach commented 3 years ago

@vmaggioli At the point at which you're already debugging this, you should be looking to find out an answer yourself. That said, as a hint, the conditional that you pointed to does not do what you think it does. The spinner can - expectedly - get a zero duration when snappedTime == hitObject.StartTime, because that edge case is currently not checked.

Pennek commented 3 years ago

I can still replicate this. https://youtu.be/BKNTfaXuF_4

peppy commented 3 years ago

Can you explain the repro case? Is copy-pasting the slider enough to trigger this?

bdach commented 3 years ago

I can reproduce. As far as I can tell it needs to be an existing slider from a map that wasn't created using the lazer editor, for whatever reason. So any plain ol' map imported from direct can be used to repro.

Pennek commented 3 years ago

Sliders can be modified to zero length if another slider with length >minimum length is copypasted on top of it. (apparently only applies to sliders from existing maps)

I also found another way of creating zero length sliders which abuses the perfect curve algorithm and the transformation box. Since the transformation box interacts with all mouse buttons, I can left-click hold and drag the middle controlpoint of a three-point slider out to the transformation box and then right-click hold to modify the slider in both ways at once

It also works the other way around by dragging the box and then the middle anchor point

peppy commented 3 years ago

@Pennek Going to need a video showing what you mean for the first reported case of this. I have no idea how to reproduce.

peppy commented 3 years ago

I also can't reproduce your multi-mouse-button example. Does this still occur on the latest release?

As an update, I have been able to reproduce something similar without the case you mention in either of your reproductions. It looks to involve the logic flow when the type of curve changes as a result of exceeding the gamefield bounding box (PathControlPointPiece.updatePathType) which is run after the path version changes, likely too late for the rollback logic to kick in.

peppy commented 3 years ago

Further investigation shows that we aren't reverting the path control point Types after they are modified by the aforementioned function. In addition, just storing and reverting them is not enough, as the updatePathType code is run on every invalidation, meaning if you revert in a way that doesn't satisfy it, the path type will again be changed.

I think this is where we need to rewrite the "attempt mutation" operation to be more resilient...

Trung0246 commented 1 year ago

Looks like I found another way to place zero-length slider by dragging slider end across different bpm. This attempt can also cause crash when attempt to drag the end node.

https://user-images.githubusercontent.com/11626920/209462750-f50d22f2-3638-4020-9e07-739c793ca313.mp4

After further testing I think the bpm thing probably unnecessary but no idea. This requires to move the mouse by a very small distance.

https://user-images.githubusercontent.com/11626920/209463027-c31b8290-819c-4644-8c00-a50c313c95bc.mp4

database.log input.log legacy-ipc.log network.log performance.log performance-audio.log performance-draw.log performance-input.log performance-update.log runtime.log updater.log

Version: 2022.1208.0

waaipe commented 5 months ago

I recently found a way to create a zero length slider with the use of distance snap, although this is not very consistent and I was only able to reproduce it on several maps, only under specific conditions.

Steps used to reproduce:

  1. After finding a valid spot in timeline, start placing a slider (only the first point) while holding LAlt
  2. Move cursor so you can see a valid slider (if you were to place another point)
  3. Move close enough to the first point so that you can not create a valid slider anymore
  4. Cancel the slider placement

video example https://github.com/ppy/osu/assets/155621949/3dfce88b-4dc5-48b0-b736-076965deb161

My video example here includes this set: https://osu.ppy.sh/beatmapsets/1139462#osu/2380150

If I export the result for compatibility and look at the .osu file, the slider is represented by this line: 453,305,105079,2,0,L|429:309,1,2.5040692435140954E-06,0|0,1:0|1:0,2:0:0:0:

The "valid spot in a timeline" conditions are unknown to me. From my testing, even editing the objects before the red (1) circle changed the ability to reproduce this issue.

2024.221.0-lazer