godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
91.35k stars 21.25k forks source link

set_block_signals for slider no effect #99615

Open CsloudX opened 14 hours ago

CsloudX commented 14 hours ago

Tested versions

v4.3.stable.mono.official [77dcf97d8]

System information

Godot v4.3.stable.mono - Windows 10.0.17763 - Vulkan (Forward+) - dedicated NVIDIA GeForce RTX 2060 (NVIDIA; 31.0.15.2849) - AMD Ryzen 9 5950X 16-Core Processor (32 Threads)

Issue description

Image

Image

Steps to reproduce

just test the MRP

Minimal reproduction project (MRP)

ValueChangeTest.zip

AThousandShips commented 10 hours ago

This is because Slider resets this value internally: https://github.com/godotengine/godot/blob/0c45ace151f25de2ca54fe7a46b6f077be32ba6f/scene/gui/slider.cpp#L73-L79

We could improve this by first saving the value, and restoring it, instead of resetting it, like so:

bool was_blocking = is_blocking_signals();
set_block_signals(true);
if (orientation == VERTICAL) {
    set_as_ratio(1 - (((double)grab.pos - (grab_height / 2.0)) / max));
} else {
    set_as_ratio(((double)grab.pos - (grab_width / 2.0)) / max);
}
set_block_signals(was_blocking);
AThousandShips commented 10 hours ago

Marking this as a good first issue but keep in mind that this is something to discuss first, so if someone opens a fix please be prepared that the solution might change or this might not be considered a bug

MrHPotter commented 5 hours ago

i opened a PR with a very simple fix as https://github.com/godotengine/godot/pull/99630/commits :)