Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reverse arrow points in the wrong direction #30843

Open
424ever opened this issue Nov 22, 2024 · 3 comments
Open

Reverse arrow points in the wrong direction #30843

424ever opened this issue Nov 22, 2024 · 3 comments

Comments

@424ever
Copy link
Contributor

424ever commented Nov 22, 2024

Type

Cosmetic

Bug description

On this map the reverse arrow of 01:28:091 (1) - points normal to the slider body, instead of inwards. Screenshots are taken in the editor for convenience, but it also shows up like this during gameplay and with different skins.

Screenshots or videos

Lazer
osu_2024-11-22_18-18-22

Stable
screenshot150

Version

2024.1115.3

Logs

compressed-logs.zip

@frenzibyte
Copy link
Member

I attempted to investigate this one but there's just too much logic going on. Here's a test scene covering this issue for whoever is brave enough to figure this out:

// 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 System.Linq;
using NUnit.Framework;
using osu.Framework.Graphics;
using osu.Game.Beatmaps;
using osu.Game.Beatmaps.ControlPoints;
using osu.Game.Graphics;
using osu.Game.Graphics.Sprites;
using osu.Game.Rulesets.Judgements;
using osu.Game.Rulesets.Mods;
using osu.Game.Rulesets.Objects;
using osu.Game.Rulesets.Objects.Drawables;
using osu.Game.Rulesets.Objects.Types;
using osu.Game.Rulesets.Osu.Objects;
using osu.Game.Rulesets.Osu.Objects.Drawables;
using osuTK;
using osuTK.Graphics;

namespace osu.Game.Rulesets.Osu.Tests
{
    public partial class TestSceneSliderRepeat : OsuSkinnableTestScene
    {
        [Test]
        public void TestWeirdSlider()
        {
            AddStep("Slider with wrong direction", () => SetContents(_ => wrongDirectionSlider()));
        }

        private const double time_offset = 1500;

        /// <summary>
        /// A <see cref="Slider"/> specification which displays one of the repeat arrows pointing at a completely wrong direction.
        /// </summary>
        private Drawable wrongDirectionSlider()
        {
            var slider = new Slider
            {
                SliderVelocityMultiplier = 0.75,
                StartTime = Time.Current + time_offset,
                Position = new Vector2(0, -100),
                Path = new SliderPath(PathType.PERFECT_CURVE, new[]
                {
                    Vector2.Zero,
                    new Vector2(-32, 176),
                    new Vector2(-64, 360),
                }, 360),
                RepeatCount = 5,
            };

            return createDrawable(slider, 2);
        }

        private Drawable createDrawable(Slider slider, float circleSize)
        {
            slider.ApplyDefaults(new ControlPointInfo(), new BeatmapDifficulty
            {
                CircleSize = circleSize,
                SliderTickRate = 3
            });

            var drawable = CreateDrawableSlider(slider);

            foreach (var mod in SelectedMods.Value.OfType<IApplicableToDrawableHitObject>())
                mod.ApplyToDrawableHitObject(drawable);

            drawable.OnNewResult += onNewResult;

            return drawable;
        }

        private int depthIndex;

        protected virtual DrawableSlider CreateDrawableSlider(Slider slider) => new DrawableSlider(slider)
        {
            Anchor = Anchor.Centre,
            Depth = depthIndex++
        };

        private float judgementOffsetDirection = 1;

        private void onNewResult(DrawableHitObject judgedObject, JudgementResult result)
        {
            if (!(judgedObject is DrawableOsuHitObject osuObject))
                return;

            OsuSpriteText text;
            Add(text = new OsuSpriteText
            {
                Anchor = Anchor.Centre,
                Origin = Anchor.Centre,
                Text = result.IsHit ? "Hit!" : "Miss!",
                Colour = result.IsHit ? Color4.Green : Color4.Red,
                Font = OsuFont.GetFont(size: 30),
                Position = osuObject.HitObject.StackedEndPosition + judgementOffsetDirection * new Vector2(0, 45)
            });

            text.Delay(150)
                .Then().FadeOut(200)
                .Then().Expire();

            judgementOffsetDirection *= -1;
        }
    }
}

@frenzibyte frenzibyte removed their assignment Nov 23, 2024
@peppy
Copy link
Member

peppy commented Nov 26, 2024

Had a brief look into this and it's likely something directly in the trig call (everything around it looks sane enough, ie. the reference points should in my eyes give a valid value, and the while code is not relevant here). I don't have the knowledge to quickly act on this (@bdach might be able to smell the smell within seconds though).

@bdach bdach self-assigned this Nov 26, 2024
@bdach
Copy link
Collaborator

bdach commented Nov 26, 2024

ie. the reference points should in my eyes give a valid value

I don't think so, upon looking at it.

The situation here is as follows:

  • The repeats that look wrong are near the head, i.e. path position (0,0).

  • The slider curve used for the repeat angle calculations is:

    [0] = {Vector2} (0, 0)
    [1] = {Vector2} (0, 0)
    [2] = {Vector2} (0.001953125, 0.00048828125)
    [3] = {Vector2} (-32.689453, 179.87793)
    [4] = {Vector2} (-63.032513, 354.43427)
    [5] = {Vector2} (-63.032513, 354.43427)
    
  • For the head repeats, points (0) and (2) are used as the reference. The segment through them is in the completely wrong direction compared to the rest of the slider:
    1732611395

  • Now the reason why that point is produced where the trouble is. The slider path type here is perfect curve, and the three anchors of the slider are almost (but not really) collinear. This doesn't fail any of our existing protections, but does likely incur some numerical instability, that leads all of the points of the approximated curve to not exactly go through the slider anchors. In particular the first anchor, the (0.001953125, 0.00048828125) thing is the issue here. It fails this check:

    // Skip the first vertex if it is the same as the last vertex from the previous segment
    bool skipFirst = calculatedPath.Count > 0 && subPath.Count > 0 && calculatedPath.Last() == subPath[0];

    A possible escape hatch would be something like

    diff --git a/osu.Game/Rulesets/Objects/SliderPath.cs b/osu.Game/Rulesets/Objects/SliderPath.cs
    index 5550815370..d173f05968 100644
    --- a/osu.Game/Rulesets/Objects/SliderPath.cs
    +++ b/osu.Game/Rulesets/Objects/SliderPath.cs
    @@ -301,7 +301,7 @@ private void calculatePath()
                         List<Vector2> subPath = calculateSubPath(segmentVertices, segmentType);
     
                         // Skip the first vertex if it is the same as the last vertex from the previous segment
    -                    bool skipFirst = calculatedPath.Count > 0 && subPath.Count > 0 && calculatedPath.Last() == subPath[0];
    +                    bool skipFirst = calculatedPath.Count > 0 && subPath.Count > 0 && Precision.AlmostEquals(calculatedPath.Last(), subPath[0], 0.5f);
     
                         for (int j = skipFirst ? 1 : 0; j < subPath.Count; j++)
                             calculatedPath.Add(subPath[j]);
    

    but the precision choice is arbitrary to make-work, and this probably has diffcalc implications. Not sure how to proceed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants