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

Add repel mod to the osu ruleset #18607

Merged
merged 23 commits into from
Jul 12, 2022
Merged

Add repel mod to the osu ruleset #18607

merged 23 commits into from
Jul 12, 2022

Conversation

ggliv
Copy link
Contributor

@ggliv ggliv commented Jun 7, 2022

Initially brought up in #18294.

This mod effectively does the opposite of the existing magnetised mod, pushing hitobjects away from your cursor instead of pulling them toward it. The gradual hitobject repositioning shared between the two mods is implemented in a new abstract class, OsuEaseHitObjectPositionsMod (I am not 100% sure on the naming convention I should use here, please let me know if it should be changed). (No longer abstracted as of 2fe34f1)

OsuHitObjectGenerationUtils.calculatePossibleMovementBounds is exposed to allow the mod to ensure sliders do not move outside of the bounds of the playfield, and a new parameter (defaulted to true) is added to determine whether the returned RectangleF should account for the radius of the slider. (Reverted as of 6e883a6) The slider clamping logic is reused from clampSliderToPlayfield.

repel.mp4

@mk56-spn
Copy link
Contributor

mk56-spn commented Jun 7, 2022

Ive played it a bit and it feels very akward, which i guess its fair enough but the maximum repel strength needs to be cut down to whats currently .75 strength, anything above that is just not really reasonable

@apollo-dw
Copy link
Contributor

I'd personally expect repel to be a checkbox / switch in Magnetised, rather than a whole separate mod

@frenzibyte
Copy link
Member

Having it a separate mod is consistent with Grow/Deflate, WindUp/WindDown, etc. I think that's fine for now.

@ggliv ggliv requested a review from bdach June 27, 2022 21:33
@peppy
Copy link
Member

peppy commented Jul 1, 2022

Is it intended that the repulsion gets strong the further away objects are to the cursor? This to be seems counter-intuitive, and hasn't been mentioned anywhere, so I wanted to clarify before going any further.

@ggliv
Copy link
Contributor Author

ggliv commented Jul 2, 2022

I didn't explicitly intend for that to be the case while putting this together, but I think it works well enough gameplay-feel-wise. Using a constant strength makes repulsion while the cursor is close to a hitobject feel jerky, in my opinion, though I could just be used to the original feel at this point.

Here's the diff for constant strength if you want to give it a try.
diff --git a/osu.Game.Rulesets.Osu/Mods/OsuModRepel.cs b/osu.Game.Rulesets.Osu/Mods/OsuModRepel.cs
index 052c7128f6..371e2edfd0 100644
--- a/osu.Game.Rulesets.Osu/Mods/OsuModRepel.cs
+++ b/osu.Game.Rulesets.Osu/Mods/OsuModRepel.cs
@@ -51,24 +51,20 @@ public void Update(Playfield playfield)
 
             foreach (var drawable in playfield.HitObjectContainer.AliveObjects)
             {
-                float x = Math.Clamp(2 * drawable.X - cursorPos.X, 0, OsuPlayfield.BASE_SIZE.X);
-                float y = Math.Clamp(2 * drawable.Y - cursorPos.Y, 0, OsuPlayfield.BASE_SIZE.Y);
+                var destination = drawable.Position - 100 * Vector2.NormalizeFast(cursorPos - drawable.Position);
+                destination = Vector2.Clamp(destination, Vector2.Zero, OsuPlayfield.BASE_SIZE);
 
                 if (drawable.HitObject is Slider thisSlider)
                 {
                     var possibleMovementBounds = OsuHitObjectGenerationUtils.CalculatePossibleMovementBounds(thisSlider);
 
-                    x = possibleMovementBounds.Width < 0
-                        ? x
-                        : Math.Clamp(x, possibleMovementBounds.Left, possibleMovementBounds.Right);
-
-                    y = possibleMovementBounds.Height < 0
-                        ? y
-                        : Math.Clamp(y, possibleMovementBounds.Top, possibleMovementBounds.Bottom);
+                    destination = Vector2.Clamp(
+                        destination,
+                        new Vector2(possibleMovementBounds.Left, possibleMovementBounds.Top),
+                        new Vector2(possibleMovementBounds.Right, possibleMovementBounds.Bottom)
+                    );
                 }
 
-                var destination = new Vector2(x, y);
-
                 switch (drawable)
                 {
                     case DrawableHitCircle circle:

@peppy
Copy link
Member

peppy commented Jul 4, 2022

To make it clear, I was implying it should (if going by the physically correct definition) be opposite of what it is right now, not constant.

@ggliv
Copy link
Contributor Author

ggliv commented Jul 6, 2022

I've pushed some commits that correct the repulsion strength. I'm still not 100% sure on the feel of it though, would appreciate some input on that front if you don't think it's currently up to snuff.

@mk56-spn
Copy link
Contributor

mk56-spn commented Jul 6, 2022

I'll give it a play

nagi-desuuu
nagi-desuuu previously approved these changes Jul 7, 2022
@mk56-spn
Copy link
Contributor

mk56-spn commented Jul 8, 2022

sorry for the late response, feels really nice now, its smooth and it doesn't sort of get annoying to hit when you get close to the circle. predictable movement, well done. gameplay wise i feel it's good for a merge now

@ppy ppy deleted a comment from nagi-desuuu Jul 8, 2022
the destination vector is clamped within playfield borders, we want dampLength to be based on distance from the cursor.
// 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.

#nullable disable
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would be preferable to have all new code have NRT enabled. The problem here is going to be gameplayClock, but I think the way to go here is to annotate it as nullable and then add a Debug.Assert(gameplayClock != null) in easeTo().

public override ModType Type => ModType.Fun;
public override string Description => "Hit objects run away!";
public override double ScoreMultiplier => 1;
public override Type[] IncompatibleMods => new[] { typeof(OsuModAutopilot), typeof(OsuModWiggle), typeof(OsuModTransform), typeof(ModAutoplay), typeof(OsuModMagnetised) };
Copy link
Collaborator

Choose a reason for hiding this comment

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

This mod is declaring incompatibility with 5 others, but 4 of those 5 are not declaring incompatibility with OsuModRepel themselves in the other direction (OsuModMagnetised being the exception).

Copy link
Member

Choose a reason for hiding this comment

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

You still missed autoplay..

Copy link
Member

Choose a reason for hiding this comment

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

and relax.

Copy link
Contributor Author

@ggliv ggliv Jul 12, 2022

Choose a reason for hiding this comment

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

Yikes.

Relax was intentionally excluded though, #17319 isn't an issue when hitobjects are being pushed away. I've opened #19084 to fix.

@ggliv ggliv requested a review from bdach July 11, 2022 21:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants