godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.14k stars 93 forks source link

Add a new connection flag to only a receive a signal once per frame #3245

Open colugomusic opened 3 years ago

colugomusic commented 3 years ago

Describe the project you are working on

A DAW with lots of complex controls which interact with each other in horrible, convoluted ways.

Describe the problem or limitation you are having in your project

Sometimes objects must do a non-trivial amount of work in response to a signal. For example an item is added to a list:

func add_item(new_item):
    items.append(new_item)
    emit_signal("item_added")
func _on_list_item_added():
    reconfigure_the_object_in_some_fancy_way() # some expensive function

This works fine if it's just one item but perhaps the items are being loaded from a file or generated or something:

for i in range(big_number):
    list.add_item(i)

Now the expensive function gets called multiple times, even though only the last call is the one that actually matters.

Obviously you can restructure this to something like:

for i in range(big_number):
    list.add_item(i)

list.reconfigure()

I don't like this because it introduces stronger coupling between the two objects which the signal/slot connection system generally helps us avoid. The structure of the interaction is often far more complex than the example given here and the objects involved may be many levels of abstraction apart. In large, complex projects it is nice to set things up in such a way that you can simply emit a signal to notify that something has happened, and trust that the rest of the system will respond appropriately without having to explicitly tell it to.

Another approach is to move the work into _draw() and trigger an update by calling update(). I don't like this approach for multiple reasons:

Describe the feature / enhancement and how it helps to overcome the problem or limitation

A new connection flag, something like CONNECT_ONCE_PER_FRAME, which causes the connected method to only trigger at most once per frame. When combined with the existing CONNECT_DEFERRED flag this gives us a more flexible/generic version of the existing update() and _draw() synergy. i.e. If the signal is emitted multiple times in the same frame then the connected method will be triggered only once during idle time.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

connect("signal", object, "method", [], CONNECT_DEFERRED | CONNECT_ONCE_PER_FRAME)

...

# Signal is emitted multiple times in the same frame:
emit_signal("signal")
emit_signal("signal") # ignored by the receiving object
emit_signal("signal") # ignored by the receiving object
emit_signal("signal") # ignored by the receiving object

# the connected method will be called once during idle time.

If this enhancement will not be used often, can it be worked around with a few lines of script?

Not really, but you can achieve an ugly version by doing something like this: https://github.com/colugomusic/gdnutil/blob/master/include/gdnutil/once_per_frame.hpp

Another alternative which I discussed above is to abuse the update() / _draw() behaviour e.g.


var dirty_1 := false
var dirty_2 := false

func _on_signal_emitted():
    dirty_1 = true
    update()

func _on_different_signal_emitted():
    dirty_2 = true
    update()

func _draw():
    if dirty_1:
        dirty_1 = false
        do_some_work()

    if dirty_2:
        dirty_2 = false
        do_different_work()

Is there a reason why this should be core and not an add-on in the asset library?

Requires changes to the core I assume.

KoBeWi commented 3 years ago

You can achieve the same effect as using _draw() with a custom function. E.g.

var dirty = false
func on_signal_received():
    my_custom_update()

func my_custom_update():
    dirty = true
    call_deferred("deferred_update")

func deferred_update():
    if not dirty:
        return
    dirty = false
    stuff...
colugomusic commented 3 years ago

Exactly, this is the idiom encapsulated by the gdnative class I linked above. So I now do something like this:

class Test : public godot::Object
{
    GODOT_CLASS(Test, godot::Object);

    ....

    Ref<gdn::OncePerFrame> dirty_work_;

    void _init()
    {
        dirty_work_.instance();
        dirty_work->set_task([this]()
        {
            // Do expensive things
        });
    }

    void something()
    {
        // trigger as many times as you like, it only happens once per frame
        dirty_work_->trigger();
        dirty_work_->trigger();
        dirty_work_->trigger();
        dirty_work_->trigger();
        dirty_work_->trigger();
    }
};

Which looks weird but it's less error prone than writing it in full every time, especially when you have a lot of different things going on. Do you agree a new connection flag makes sense here or am I the only one who really has this problem?

KoBeWi commented 3 years ago

Well, personally I didn't have such problem. When I needed the dirty flag, I was calling the method manually, not using signals. That flag sounds like a rather specific use-case and you still need some workarounds if you don't use signals. But maybe the issue is more common for other people, the proposal was open for just one day.