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

Show distance snap at current point in time (and add ability to set as usable value) #20854

Merged
merged 10 commits into from
Oct 26, 2022

Conversation

peppy
Copy link
Sponsor Member

@peppy peppy commented Oct 21, 2022

RFC, working with what I have for the UI.

osu!stable shows "prev" and "next" values (based on selection), but this time i've tried showing the current instead.

Copy link
Collaborator

@bdach bdach left a comment

Choose a reason for hiding this comment

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

What is in this pull seems to be working pretty alright, small quibbles aside. Not sure what is up with the TODOs, though.

osu.Game/Rulesets/Edit/ExpandableButton.cs Outdated Show resolved Hide resolved
osu.Game/Rulesets/Edit/ExpandableButton.cs Outdated Show resolved Hide resolved
@@ -112,6 +112,14 @@ public override string ConvertSelectionToString()

private RectangularPositionSnapGrid rectangularPositionSnapGrid;

protected override double ReadCurrentDistanceSnap(HitObject before, HitObject after)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Spinners throw a bit of a wrench into this, the function proposes distance spacings for spinners that are a bit nonsense (likely because spinners technically have a "position" in the middle of the playfield). May want to prevent proposing a spacing if one of the objects is a spinner.

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

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

I think I'd probably leave this for now as "working as intended", based on the gameplay direction where we want to allow non-centered spinners.

osu.Game/Rulesets/Edit/DistancedHitObjectComposer.cs Outdated Show resolved Hide resolved
Comment on lines +83 to +91
// osu!catch's distance snap implementation is limited, in that a custom spacing cannot be specified.
// Therefore this functionality is not currently used.
//
// The implementation below is probably correct but should be checked if/when exposed via controls.

float expectedDistance = DurationToDistance(before, after.StartTime - before.GetEndTime());
float actualDistance = Math.Abs(((CatchHitObject)before).EffectiveX - ((CatchHitObject)after).EffectiveX);

return actualDistance / expectedDistance;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this is going to only be correct for single fruits, and be wrong for juice streams (where before.EndX should be used) and even more wrong for banana showers (god knows what should be used there, X of last banana in shower?). But given there's no way to test as is anyway I think I'm going to let that slide as out of scope for the time being...

@bdach bdach enabled auto-merge October 26, 2022 21:06
@bdach bdach merged commit 90a6888 into ppy:master Oct 26, 2022
@peppy peppy deleted the read-current-distance-snap branch October 27, 2022 01:48
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.

None yet

2 participants