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

The forge-provided Slider does not stop when releasing the cursor outside the slider #8485

Closed
PlatinPython opened this issue Feb 26, 2022 · 4 comments · Fixed by #8496
Closed
Labels
1.16 1.18 Bug This request reports or fixes a new or existing bug.

Comments

@PlatinPython
Copy link
Contributor

Minecraft Version: 1.16.5, 1.18.1

Forge Version: 36.2.29, 39.0.88

Steps to Reproduce:

  1. Create a screen with a slider using the forge-provided Slider class (net.minecraftforge.(fml.)client.gui.widget.Slider)
  2. Open said screen, move the slider and keep holding
  3. Move the cursor outside the slider box
  4. Release the cursor

Small mods for ease of reproduction

Description of issue:
When one releases the cursor outside of a slider's box the slider does not stop changing it's value, to stop it from moving one has to again click on the slider. This also allows moving multiple sliders at once.
Video

@PlatinPython PlatinPython added the Triage This request requires the active attention of the Triage Team. Requires labelling or reviews. label Feb 26, 2022
@sciwhiz12 sciwhiz12 added 1.16 1.18 Bug This request reports or fixes a new or existing bug. labels Feb 26, 2022
@desht
Copy link
Contributor

desht commented Mar 3, 2022

The problem here is that the Slider widget is tracking the dragged state itself, which is unnecessary (and wrong), although it may well have been valid when the widget was created (Minecraft gui handling has changed a lot since then).

The vanilla AbstractSliderButton widget is an example of how to do this properly; simply let the owning ContainerEventHandler track the dragging state (which it does anyway) - it automatically calls AbstractWidget#onDrag() on the slider even if the mouse pointer is moved off the slider, as long as the mouse button is held down.

I can try to do a PR when I get a little time, but the changes required are:

  • Delete the dragging field and all code which references it from Slider
  • Delete the now-unnecessary onRelease() method
  • Add an onDrag() method:
    @Override
    protected void onDrag(double mouseX, double mouseY, double dragX, double dragY) {
        this.sliderValue = (mouseX - (this.x + 4)) / (float)(this.width - 8);
        updateSlider();
    }
  • Probably also get rid of that "String concatenation in loop" warning... string concatenation in loops is bad, m'kay?

@desht
Copy link
Contributor

desht commented Mar 3, 2022

I should also note that removing dragging is technically a breaking change, since the field is public, but there's a lot of public stuff in that widget which really shouldn't be.

@PlatinPython
Copy link
Contributor Author

I am currently working on a PR to "fix" this. I created a completely new Slider class and deprecated this one. The new class includes some QoL features which allows for better control of the slider. Will link the PR once done (still gotta do some docs).

@PlatinPython
Copy link
Contributor Author

Will be "fixed" once #8496 is merged.

@autoforge autoforge bot removed the Triage This request requires the active attention of the Triage Team. Requires labelling or reviews. label Feb 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.16 1.18 Bug This request reports or fixes a new or existing bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants