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.

Sign up for GitHub

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

Jump to bottom

Prevent custom divisor ranges from halting divisor preset cycling #26689

Merged
merged 2 commits into from Jan 25, 2024
Merged

Prevent custom divisor ranges from halting divisor preset cycling #26689

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9e1c242
Prevent custom divisor ranges from halting preset cycling
myQwil Jan 24, 2024
a264ac9
Change name and description of `force` parameter in `SetArbitraryDivi…
myQwil Jan 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions osu.Game.Tests/Visual/Editing/TestSceneBeatDivisorControl.cs
View file
Open in desktop
Expand Up @@ -210,6 +210,13 @@ public void TestBeatPresetNavigation()
switchPresets(-1);
assertPreset(BeatDivisorType.Custom, 15);
assertBeatSnap(15);

setDivisorViaInput(24);
assertPreset(BeatDivisorType.Custom, 24);
switchPresets(1);
assertPreset(BeatDivisorType.Common);
switchPresets(-2);
assertPreset(BeatDivisorType.Triplets);
}

private void switchBeatSnap(int direction) => AddRepeatStep($"move snap {(direction > 0 ? "forward" : "backward")}", () =>
Expand Down
5 changes: 3 additions & 2 deletions osu.Game/Screens/Edit/BindableBeatDivisor.cs
View file
Open in desktop
Expand Up @@ -29,10 +29,11 @@ public BindableBeatDivisor(int value = 1)
/// Set a divisor, updating the valid divisor range appropriately.
/// </summary>
/// <param name="divisor">The intended divisor.</param>
public void SetArbitraryDivisor(int divisor)
/// <param name="force">Ignores the current valid divisor range when true.</param>
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

This reads wrong. Don't you mean "forces changing the valid divisors to a common preset"? I'd probably name this variable preferKnownPresets or experiment with extracting the logic out to a second method for this, called something like SetPresetsForDivisor(int divisor).

public void SetArbitraryDivisor(int divisor, bool force = false)
{
// If the current valid divisor range doesn't contain the proposed value, attempt to find one which does.
if (!ValidDivisors.Value.Presets.Contains(divisor))
if (force || !ValidDivisors.Value.Presets.Contains(divisor))
{
if (BeatDivisorPresetCollection.COMMON.Presets.Contains(divisor))
ValidDivisors.Value = BeatDivisorPresetCollection.COMMON;
Expand Down
4 changes: 2 additions & 2 deletions osu.Game/Screens/Edit/Compose/Components/BeatDivisorControl.cs
View file
Open in desktop
Expand Up @@ -208,11 +208,11 @@ private void cycleDivisorType(int direction)
switch (currentType)
{
case BeatDivisorType.Common:
beatDivisor.SetArbitraryDivisor(4);
beatDivisor.SetArbitraryDivisor(4, true);
break;

case BeatDivisorType.Triplets:
beatDivisor.SetArbitraryDivisor(6);
beatDivisor.SetArbitraryDivisor(6, true);
break;

case BeatDivisorType.Custom:
Expand Down