Turn the feature on to see if it causes bugs. Then fix those bugs before the update.
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?
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.
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:
- 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.
- 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.
- 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.
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.
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.
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.
I managed to implement the new textchatservice and didnt take me that long once i worked out how to do system messages.
But unfortunately deferred events still breaks my experience because it holds on to events that need to happen before other events in other scripts for the gui to be copied across from replicated storage.
So fixing this is going to take a lot of effort (so will be using immediate events for now).
Events can be disconnected in multiple ways. The most obvious way is by calling the disconnect method on a connection object. They can also be disconnected indirectly such as by destroying an instance. The desired behavior depends on how the event is being disconnected. We distinguish these two behaviors as follows:
- Hard - Disconnect from the event immediately and drop all pending events associated with the connection.
- Soft - Disconnect from the event immediately but run the associated event handler for any events that are still pending.
In the case where you explicitly call disconnect you most probably want a hard disconnect and in the case of destroy, a soft disconnect. While we considered adding two separate methods to reflect these we decided that making all disconnects a soft-disconnect would be better and if a hard disconnect is still desired it can be achieved by reading the Connected field.
That all said, we are very open to this feedback so I will re-open this discussion internally and update the thread accordingly with the outcome.
Oh aight! Thanks! You’re a life saver!
Hello, I’m having some issues converting over to deferred. on immediate sending inventory weight data back to client through remote event work fine but it just doesn’t update properly on deferred. It even shows the right weight from the server which is updated right before the event fires to the client with the newly updated weight. But the client still thinks the weight is of the current item being removed. I would like to adapt but currently I cannot.
Deferred engine events are now available to use in live experiences.
Can you make a thread with more information in #help-and-feedback:scripting-support? If you tag me in it I will take a look when I get a moment
Hey @WallsAreForClimbing I tried switching over one of my experiences to deferred today to see how it would do, and I’m seeing that among other things a couple of things in regards to tools are breaking and was hoping you could help me understand why.
For example, a lot of my tools start with this line (script directly inside the tool):
repeat wait() until script.Parent.Parent:FindFirstChild(“Humanoid”)
With deferred on this line errors saying attempt to index nil with parent. The tool is simply cloned out of a folder and placed directly into the player’s backpack like so:
Tool:Clone().Parent = Player.Backpack
I don’t understand why deferred would cause the code to execute when the tool has no parent, as it is placed into the backpack instantly.
There’s also this line of code elsewhere used to verify the tools the player has equipped:
Backpack.ChildAdded:Connect(function(Tool)
if PlayerHasTool(Player,Tool.ItemID.Value) == false then …
This line now errors with deferred saying that ItemID is not a valid member of tool, but it is always there from the get go before the item is cloned or placed in the backpack. These are all server-side scripts so It is not a replication thing, and I would expect that any direct descendants of the tool would be immediately reachable when it is added to the player’s backpack.
Fixing these things is not too difficult by adding WaitForChild etc., but I’m not quite understanding why this change would affect such things and was hoping you could briefly explain it to me to make it easier to find other issues this may cause in the rest of my or other’s games.
Not OP but I think that there are a lot of cases where turning on deferred mode reveals some really shaky coding practices such as what you have there in the first issue. Even without deferred mode, yielding at the start of a script is often representative of a code smell. For your case it may take an ample amount of rearchitecturing rather than things working out of the box.
I would instead recommend defining the character when the tool is equipped. You don’t have to redefine it multiple times, only once is enough. Similarly, if you need the character to be defined in the topmost scope, you can create an initialiser function to execute on the first equip.
-- Forward declare the character variable.
local character
-- Set it when the tool is equipped; if it doesn't need to be dropped, you can
-- make sure its assigned only once with Once.
Tool.Equipped:Once(function (mouse)
character = Tool.Parent
-- This might be how you choose to initialise.
Tool.Equipped:Connect(onEquipped)
onEquipped(mouse)
end)
I can’t say anything about the second one though. I don’t want to backseat engineer or anything, so just wanted to point out a potential solution you could use for the first issue. Far be it from me to give any pretentious or misinformative answers, so I don’t have any tips to offer for the rest and would also be interested in understanding how best to fix these cases or how deferred affects the code.
I figured it out!
By disabling some events I was able to narrow down the event that fired the wrong data. I had to wrap the code in a task.defer() function. This ensured the event fired at the end of the frame, AFTER the inventory weight is updated in a function way above the in the stack! This allowed the new data to be sent and updated on the client’s UI properly. Whereas in immediate everything is happening at once.