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

Fix autoplay button wrongly being active after a Ctrl-Enter play #2207

Merged
merged 6 commits into from Mar 16, 2018

Conversation

3 participants
@FreezyLemon
Member

FreezyLemon commented Mar 12, 2018

The multiplier and everything were correct before, just the button was still active.

deselect autoplay button after ctrl-enter play
before, the mod was removed, but the button was still active
@@ -100,6 +100,7 @@ protected override void OnResuming(Screen last)
{
var autoType = Ruleset.Value.CreateInstance().GetAutoplayMod().GetType();
SelectedMods.Value = SelectedMods.Value.Where(m => m.GetType() != autoType).ToArray();

This comment has been minimized.

@Aergwyn

Aergwyn Mar 12, 2018

Member

Wouldn't this line be unnecessary if you deselect right afterwards anyways?

@peppy

This comment has been minimized.

Member

peppy commented Mar 12, 2018

I'd like to figure out why the button was in the wrong state, because this is basically a local fix. If something elsewhere in the game modifies the mods Bindable directly the same thing is going to happen.

@FreezyLemon

This comment has been minimized.

Member

FreezyLemon commented Mar 13, 2018

Note that this is only a bug when removing the mod. Adding the mod works fine (observe by pressing Ctrl-Enter with the mod selection still open. you can see the button being selected properly).

This might have been an oversight, but the current code only tries to select new mods without deselecting the old ones:
Image

(where SelectTypes does not deselect any buttons)

@peppy

This comment has been minimized.

Member

peppy commented Mar 13, 2018

Yeah, that's the part which should be fixed. You can probably add a fix for that in this PR as well.

@peppy peppy added this to the Candidate Issues milestone Mar 13, 2018

@FreezyLemon

This comment has been minimized.

Member

FreezyLemon commented Mar 13, 2018

Looking at this a bit more, there's some inconsistencies I'd like to address too:

  1. There's SelectTypes and DeselectTypes even though the SelectTypes method takes the mod instances, and not their types, as an argument. I'd like to make this more consistent. Should the methods take types, or mod instances as arguments?
  2. Should SelectTypes maintain its functionality (just add new mods), or should it set (add + remove) the buttons to some 'selection state' for a lack of a better term?

FreezyLemon and others added some commits Mar 14, 2018

make SelectTypes set mods instead of only adding new ones
also made the method actually take types as parameter to make it consistent
@peppy

peppy approved these changes Mar 16, 2018

@peppy peppy merged commit 1b2e4bb into ppy:master Mar 16, 2018

1 check passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@FreezyLemon FreezyLemon deleted the FreezyLemon:fix-autoplay-button-deselect branch Mar 16, 2018

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