godotengine / godot

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

Range value_changed signal stuttering and scroll wheel skipping #67459

Open Koyper opened 2 years ago

Koyper commented 2 years ago

Godot version

4.0.beta3

System information

MacOS arm64

Issue description

The Range value_changed signal stutters, which is consistant with Godot 3.5x. I worked around it using the Range changed signal in 3.5x, which does not stutter, but the changed signal is no longer emitted in Godot 4 when the range value changes.

Additionally, when scrolling with the scroll wheel, the value_changed signal is intermittent.

Here are videos of a SectionContainer with sticky header in both 3.5.1 and 4.0.beta3:

https://user-images.githubusercontent.com/33969780/196005002-172be292-bf9e-4c82-8a4e-736277b20cb0.mov

https://user-images.githubusercontent.com/33969780/196005015-96832dcb-0fc0-4762-b535-9ba76fe4092c.mov

Steps to reproduce

Run the project below and scroll up and down.

Minimal reproduction project

range_changed_signal_and_scroll_bugs.zip

Rindbee commented 2 years ago

Well, I think being able to modify the position of the child nodes in the BoxContainer seems like a bug too.

Koyper commented 2 years ago

That’s a good point, although it has worked without complaint through 3.5x. I will see about putting the header in a MarginContainer or making the ContainerSection a Control instead to see if that takes care of the stuttering issue.

On Oct 15, 2022, at 10:02 PM, 风青山 @.***> wrote:

Well, I think being able to modify the position of the child nodes in the BoxContainer seems like a bug too.

— Reply to this email directly, view it on GitHub https://github.com/godotengine/godot/issues/67459#issuecomment-1279879390, or unsubscribe https://github.com/notifications/unsubscribe-auth/AIDFM5BW3JGLZ6I5EZZXE53WDNV3NANCNFSM6AAAAAARGBHFPE. You are receiving this because you authored the thread.

Koyper commented 2 years ago

I tried commenting out the changes in range.cpp and recompiling the engine so that it emits the changed signal like 3.5x and the problem still persits. It also behaves the same using the now working ScrollBar scrolling signal.

I also testing putting the header inside a MarginContainer so that the postion inside the VBoxContainer is not being modified and it behaves exactly the same.

My guess is that there is a race condition between the signal emit and ScrollContainer rearranging its children that's causing the stuttering.

This is an important bug to fix, as there is no other option for getting any clean signal that scrolling is happening - the scroll_started and scroll_ended signals still do not work.

I wonder if a scrolling signal should be emitted by ScrollContainer after it's handled scroll changes?

Rindbee commented 2 years ago

This is an important bug to fix, as there is no other option for getting any clean signal that scrolling is happening - the scroll_started and scroll_ended signals still do not work.

The reason for the jitter is probably that these positions are not updated when these signals are sent. So your code uses the expired position to calculate the new position.

I wonder if a scrolling signal should be emitted by ScrollContainer after it's handled scroll changes?

The scrolling signal is emitted when the ScrollBar (not ScrollContainer) tries to scroll, this signal does not mean the value has actually changed.

Koyper commented 2 years ago

Thanks! Okay, your comments sent me off in the correct direction and the code below gets rid of the jitter. Here's the key difference: (parent below is the ScrollContainer)

parent.get_child(0).connect("item_rect_changed", Callable(self, "position_header"), CONNECT_DEFERRED)

Notibly, it STILL jittered when scrolling with the mouse wheel without the CONNECT_DEFFERED.

What I recommend is that a convenience signal like scroll_updated be emitted by ScrollContainer using the approach above, because Range.value_changed and ScrollBar.scrolling can't be used for functions like this topic, and the solution is not obvious. I think a lot of people would find this useful.

class ContainerSection extends VBoxContainer:
    var parent: SectionContainer
    var header: Control = null
    var intersects_state := false

    signal section_visibility_changed(container_section, is_visible)

    func _init(p_parent: SectionContainer):
        parent = p_parent
        add_theme_constant_override("separation", 0)
        size_flags_vertical = SIZE_EXPAND_FILL

    func _ready():
        if get_child_count() == 2:
            header = get_child(0)
            parent.get_child(0).connect("item_rect_changed", Callable(self, "position_header"), CONNECT_DEFERRED)

    func get_section() -> Node:
        return get_child(1 if get_child_count() == 2 else 0)

    func position_header():
        var overlap_rect := Rect2(parent.global_position, Vector2(1.0, size.y))
        var test_point := Vector2(global_position.x, global_position.y + size.y)
        if header != null and overlap_rect.has_point(test_point):
            var overlap: float = (parent.global_position.y + position.y) - global_position.y
            var remaining_gap: float = size.y - overlap + position.y
            var header_offset: float = overlap - position.y
            if remaining_gap < header.size.y:
                var pushup_amount: float = header.size.y - remaining_gap
                header_offset -= pushup_amount
            header.position.y = header_offset
        else:
            header.position.y = 0.0
        if Rect2(parent.global_position, parent.size).intersects(Rect2(global_position, size)):
            if intersects_state == false:
                emit_signal("section_visibility_changed", self, true)
                intersects_state = true
        else:
            if intersects_state == true:
                emit_signal("section_visibility_changed", self, false)
                intersects_state = false

https://user-images.githubusercontent.com/33969780/196969070-1935484e-8e70-4901-a038-c85474fdb682.mov

RadiantUwU commented 1 month ago

Scroll containers have been modified, is this still reproducible?