Legacy Slider end support

[MaxKruse]

closes https://github.com/llllllllll/slider/issues/104

First implementation, It did seem to behave ok, but some values, esp. for multi-curve sliders should be checked over again.

Namely, on Tatoe as a test map, it gave a slider-end of x=262 which is nowhere near the x=271 that the sliderball ingame at the specified timestamp renders. This is probably a bug somewhere in this code, but i cant quite figure out where it is.

To sort-of compensate for this, i've added a multiplier for the test that takes into account the sliderball-leniency that exists. This seems to make the test pass. A review would be appreciated, i'll try and get somebody 3rd party to review this.

[MaxKruse]

Just in case a mention for visibility @llllllllll

[tybug]

I think keeping this new behavior purely local to Slider is better than a new parameter to Beatmap#hit_objects. This is made easier by not adjusting the end time of sliders as mentioned above.

I've pushed a commit which adds a new true_tick_points property to Slider (not heavily tested), and removes the slider end time adjustment. Let me know if you have any concerns with my changes.

[MaxKruse]

I went over the changes and fixed the tests to be accurate. All looking good from here! Only concern might be a last linting pass, flake8 breaks for me :/

[MaxKruse]

As you suggested, i double-checked with lazer's code on the position of the legacy slider ticks. This image is for the first slider in the Tatoe Map, which maches (except rounding down) the values i got from the visual editor inspection - so at least that was rather precise.

(Lazer only calculates the positional offset with the PositionAt() function, thats why the addition is there)

For the more complex slider (as in, the second test slider at 20153ms):

from these 2 spot-checks, i think we can conclude that the calculations here are correct.

Some missmatches will happen, as lazer calculates the length of the slider based on the decimal value it should go on for - while this lib does not have access to that and simply goes off of the end - start times, which in themselve carry a small error due to missmatch between doubles (in lazer) and ints (in this lib).

It is usually uninteresting if the legacy slider end is off by 1-2 ingame pixels (depending on buildup of rounding error). My conclusion to that is that the bigged_allowed_gap can be set at 2, or a small refactor to allow more precise duration calculation is needed.

[tybug]

Thanks for checking against lazer. The current implementation seems correct then.

I've refactored the existing test, which in part removes this assert:

    # make sure the timing is still right
    assert (
        abs(
            slider2.true_tick_points[-1].offset -
            timedelta(milliseconds=20425)
        ) <= timedelta(milliseconds=1)
    )

This was only being checked against one of the sliders and honestly I'm not sure what it was checking. Can add it back if you think it's important and elaborate on what's being checked.

The only remaining question to me is whether we want to move this to tick_points or keep it as a new method. I don't use tick_points myself in any dependent code, so I don't really know whether moving this to tick_points would be a breaking change or fixing behavior consumers were always hoping to get / relying on anyway (and therefore not really breaking). @llllllllll any thoughts?

[tybug]

I do think it would be fine to move this change into tick_points, but let's get this in as a separate method for now. We can always move it into tick_points later and leave an alias behind, if desired.