Deferred Engine Events

Last update would make more sense.

2 Likes

A few things broke because of this, but itā€™s not too bad. Basically, I relied on things immediately changing when an event such as :GetPropertyChangedSignal("Value") is fired. Now, they fire in a certain unknown order and values are not set in the same way as they were before.

2 Likes

Upon enabling deferred signal behavior, it immediately revealed a very sneaky race condition in my codebase which decreased the readability of my code as a result. Furthermore, it forced me to make the necessary corrections which in turn resulted in my code becoming incredibly more maintainable and easier to make sense of the behavior before runtime. Being able to know exactly what my code does merely by reading it will surely keep the amount of surprises throughout development to a minimum.

:smiling_face_with_three_hearts: :star_struck:

5 Likes

So far I see how some of my code runs but the default Roblox Animate script is broken now I get this error

Invalid animation id ā€˜<error: unknown AssetId protocol>ā€™:

It seems the player animation I had set for my own roblox character from the shop caused the default animate script to throw this error when on deferred. Using default animations doesnā€™t throw any errors.

Did you even read the error? This has nothing to do with deferred signals. It clearly states that you input the animation incorrectly, it must follow this format: rbxassetid://ID HERE.

Yes, on Immediate this doesnā€™t cause an error but on deferred it does but I figured out why and it was because Iā€™m changing the walk animation ID to one I was testing and accidently removed in a batch clean-up - I would eventually audit my code and remove unnecessary comments/code after Iā€™ve finished but on Immediate it did not throw an error for this but on Deferred it does and Iā€™m glad it caught the error which was bypassed on Immediate signal strangely enough.

local testAnimation = "12719310108"
local animateScript = character:WaitForChild("Animate")
animateScript.walk.WalkAnim.AnimationId = testAnimation

completely bypassed on Immediate signal, caught on deferred :slight_smile:

1 Like

Will there be any option in the future that separate Signal Behavior from Plugin and Main Game? currently The Building Tools (By F3X) will break when I try to use it with deferred mode I need to use Immediate mode when building and switch to deferred mode when play testing (moon animator also return error sometime)
Screenshot 2023-04-10 080151

5 Likes

Can you give an example about the benefit of switching to deferred signal? Is it helpful for signal debounding?

1 Like

I tried this and it broke my game. Deferred wonā€™t let me hook into ChatServiceRunner.ChatService on startup as the events happen before it can start up the service in studio. I donā€™t know if this forces me to switch to the new text chat serviceā€¦

2 Likes

Turn the feature on to see if it causes bugs. Then fix those bugs before the update.

1 Like

Simple API inclusion that would make it much easier to switch:

PLEASE add an Instance:IsDestroyed() method to query if an instance is destroyed (and distinguish it from ancestry changes)

There are many classes of bugs and off-by-one-frame errors that are caused by scripts and effects not cleaning up in time after an instance was destroyed.

I will need to test the effect of deferred signals on my game, but the classes of bugs that come to mind for me:

  • When deferred signals were released, there was a bug that calling Connection:Disconnect() would still allow the connectionā€™s event handler to still potentially be called next frame. This was a primary reason for my hesitation. Was this fixed?

  • When an instance is destroyed, do ALL associated events for that instance (other than Destroy) get cancelled/ignored the next frame?

  • Does Destroyed get priotitized over other events to fire first?

2 Likes

While this change does impact ordering the order itself is still predictable and works in a similar way to immediate mode. When an event is triggered, the handler is put into a queue. That queue is later resumed in order until it is empty. If additional events are triggered while a handler is running those events will be added to the back of the queue and resumed after all pending work has finished.

3 Likes

I can see how IsDestroyed would be useful when using deferred events. Iā€™ll reach out to the team that implemented the Destroying event to see what we can do about this. Thanks for the suggestion!

Regarding your other points:

  1. We decided not to do this as the event itself was triggered before disconnect was called. I believe the most common case for this was when listening to the first instance of an event, for that we introduced Once. For other use-cases it is possible to check the ā€˜Connectedā€™ field of RBXScriptConnection.
  2. When an instance is destroyed, events should get disconnected as they would if you explicitly called Disconnect. If there were any pending invocations, the event handlers would still run.
  3. Not that I am aware but there might be some special logic in place to ensure it always runs before event handlers disconnect.

As an aside, we discussed adding a method similar to Disconnect that would immediately disconnect the event and cancel all pending invocations that were queued but decided against adding it for now. If you think this would be better than checking the ā€˜Connectedā€™ field then I would recommend submitting a feature request for it. That would allow me to better understand your use-case and give other users an opportunity to share theirs.

4 Likes

One important use case that isnā€™t covered by Event:Once() is waiting on events that may potentially need to fire multiple times. For example:

local connection
connection = instance.ChildAdded:Connect(function(child)
    if child.Name == "Seat" then
        connection:Disconnect()
        -- Run handler with a child named "Seat"
        onSeat(child)
    end
end)

The above code could be problematic in a number of cases that forget to check connection.Connected

Even more use cases that are extremely important are pretty diffuse.
An example:

function MyClass:SubscribeToState(key: string, cb: () -> ())
    local conn = instance:GetAttributeChangedSignal(key):Connect(cb)
    
    -- Return "unsubscribe" function
    return function()
        conn:Disconnect()
    end
end

The above code seems like it should work, but with Deferred signals leads to undefined behavior where the callback could potentially fire even after unsubscribe was called.

To fix the above code, you would need to do this:

function MyClass:SubscribeToState(key: string, cb: () -> ())
    local conn
    conn = instance:GetAttributeChangedSignal(key)
        :Connect(function()
            if conn.Connected then
                cb()
            end
        end)
    
    -- Return "unsubscribe" function
    return function()
        conn:Disconnect()
    end
end

The above form code and similar is present in a lot of existing code across many libraries, third party or otherwise. Having conventional checks for conn.Connected feels like a hacky workaround or a code smell, and this is a primary reason why I was reluctant to adopt deferred events.

Iā€™m not convinced that adding connection.Connected checks is a great solution. That a connection is connected is built in to the current assumptions about connections themselves, and large amounts of code has already been written with this assumption.

If thereā€™s a performance reason for this being a limitation, I might understand. But you are changing the behavior of connections to work differently from long standing inherent assumptions of how they work.

10 Likes

Hi, could you tell us whatā€™s preventing you from adopting the new TextChatService?

Iā€™m not sure I agree since I canā€™t think of a case where anyone would expect to still receive an event after itā€™s been disconnected, whether it happens multiple times in the same frame or not.

It should be the other way round where if player expect to receive every events in the frame then they should make sure to disconnect at the end of the frame instead themselves.

Not receiving any more events after disconnection is intuitive and should be the default behaviour. This is also in line with the behaviours before if the aim is for people to migrate more easily.

2 Likes

Old things eventually break, thats just how it is.

Just time, I built a custom chat for my lobby. But even fixing that in one of my simpler places, deferred events breaks the place, because thereā€™s a very specific order of events that needs to happen to copy across player GUI.

Hi, weā€™re no longer actively maintaining the legacy chat system. We instead encourage you to migrate to the new TextChatService (Migrating from Legacy Chat to In-Experience Text Chat | Roblox Creator Documentation). If you need any assistance or have any questions/feedback, please let us know in this post: TextChatService is now the default for new experiences!. Thank you!

As someone who is interested in security, I am curious as to what set of vulnerabilities that this eliminates.