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 object duration checks #13874

Merged
merged 4 commits into from
Jul 13, 2021
Merged

Add object duration checks #13874

merged 4 commits into from
Jul 13, 2021

Conversation

Naxesss
Copy link
Contributor

@Naxesss Naxesss commented Jul 13, 2021

Makes more progress on #12091

A little uncertain if we want CheckTooShortSliders and CheckTooShortSpinners to be named so similarly considering one is a spread-dependent check and the other applies to any difficulty. We'll eventually want another too short spinner check that is specifically for spread (making sure beginners have enough time to spin and recover), so I could see this getting a bit confusing, but I'm not sure what else to call them at the moment.

KG6kMU2iaG.mp4

The verify tab can now detect when sliders in easy difficulties are extremely short, when spinners are so short that auto can't achieve 1000 on them, and when objects somehow have a duration of 0 or less.

Copy link
Sponsor Member

@peppy peppy left a comment

Choose a reason for hiding this comment

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

Looks fine otherwise.

/// </summary>
private const double leniency = 0.5d;

public CheckMetadata Metadata { get; } = new CheckMetadata(CheckCategory.Compose, "Zero-length hitobjects");
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Suggested change
public CheckMetadata Metadata { get; } = new CheckMetadata(CheckCategory.Compose, "Zero-length hitobjects");
public CheckMetadata Metadata { get; } = new CheckMetadata(CheckCategory.Compose, "Zero-length objects");

We don't use the term "hitobject" anywhere in messages exposed to the user, as far as I can see.

Copy link
Contributor Author

@Naxesss Naxesss Jul 13, 2021

Choose a reason for hiding this comment

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

There are a few other checks where this term is used, like "unsnapped hitobjects", "concurrent hitobjects", "offscreen hitobjects", etc (these are meant to be exposed to the user at some point). I'd say if we want to change that it'll probably be best handled in that separate pass you mentioned on discord, as it will affect more than just this PR.

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Can probably happen when we add localisation support for all this stuff, if not sooner.

@peppy peppy merged commit 2436ebb into ppy:master Jul 13, 2021
@Naxesss Naxesss deleted the short-object-checks branch November 3, 2021 16: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.

2 participants