[Beta] Deferred Lua Event Handling

I have tried to open Console inside Studio and this happened:


CoreGui.RobloxGui.Modules.DevConsoleMaster:243: attempt to index nil with 'Disconnect'

Well yeah if this become a default thing, me myself as a beginner developer don’t really know how to prepare for it and I already know that this change is going to have big impact on my previously created games.

3 Likes

Turning on deferred signals for the mission place in Zombie Strike gave me the following error:

Maximum re-entrancy depth (10) exceeded

…and renders the game completely unplayable. Worst of all, it gives me zero info on where this problem is. I have literally no idea where to look, our codebase is massive.

29 Likes

A work-around I’ve found for the issue of PlayerAdded is to add a loop immediately after the connection that calls the initializer function with whatever players are already in the game. In reality, this will likely only ever be your studio player, but it makes testing easier than booting up a local server each time.

2 Likes

I think this should have been a Studio beta. I cannot test if this affects my plugins.

1 Like

Enabling “Deferred” breaks my game completely, it even breaks core scripts…

local function unregisterGui(element)
    registryInfoMap[element].connection:Disconnect()
    registryInfoMap[element] = nil
end
  02:49:17.443  CoreGui.RobloxGui.Modules.GameTranslator:44: attempt to index nil with 'connection'  -  Client - GameTranslator:44
  02:49:17.443  Stack Begin  -  Studio
  02:49:17.443  Script 'CoreGui.RobloxGui.Modules.GameTranslator', Line 44 - function unregisterGui  -  CoreScript - GameTranslator:44
  02:49:17.444  Script 'CoreGui.RobloxGui.Modules.GameTranslator', Line 52  -  CoreScript - GameTranslator:52
  02:49:17.444  Stack End  -  Studio

I’d like to never ever have my players experience a bug in my games especially if it’s not my fault.

the line that is highlighted is having problems when I switch to “Deferred”


My game also had issues when I enabled “Immediate” so are there any differences between “Default” and “Immediate”? @WallsAreForClimbing @tnavarts @zeuxcg

I’d like to keep my game and code the way it is and not have to work on it because of an update like this.

Making games is already hard please don’t make our lives harder.

5 Likes

Good luck fps games. you have my best wishes <3

I’ve been following this thread since yesterday, and here’s what I have to say:

This is gonna (already does, if you enable it) break literally everything.

C’mon, you guys have a big QA pipeline, and not even one human being opened up Studio, made some simple shop script, clicked F5, and saw that it didn’t work because this change made .PlayerAdded not fire, and the literal CoreScripts, something present in every game, not work?
This is especially absurd because you’re changing a vital part of the API, that’s been like this since the beginning. Roblox games depend on events to get information about what’s happening. This type of change shouldn’t be made without even testing it, and even less forced onto us.

I have to reiterate @Cindering’s response.

I’ve also tested this; it’s especially funny because it’s completely anti-pattern to the order of how you handle things in a “normal” engine.

I’m not one to usually give opinions on changes, but we all have to agree this is just a completely absurd change, without thought given to it. Yes, the pros are definitely valid, but the cons definitely outweigh the pros.

Edit: was just working on my parkour system, toggled the new behaviour for fun and the character and ledge detection is very sluggish compared to before (it relies on input and RenderStepped), also, thanks for the clarification @zeuxcg

7 Likes
local step = 0

game:GetService("RunService"):BindToRenderStep("hello", 2001, function()
	step = "while bindtorender"
end)

game:GetService("RunService").RenderStepped:Connect(function()
	step = "while render"
end)

game:GetService("RunService").Heartbeat:Connect(function()
	step = "before everything"
end)

game:GetService("ContextActionService"):BindAction("cbt", function()
	print(step)
end, false, Enum.UserInputType.MouseMovement)
--[[ game:GetService("UserInputService").InputBegan:Connect(function()
    print(step)
end) --]]

Something to note, using deferred behavior in both methods, ContextActionService preserves the existing immediate behavior, whereas input began and by extension other input events do not.

Also, a solution to the PlayerAdded event problem is to loop through existing players and call your added function for them. you probably should have been doing this anyway, but i completely agree with the counter arguments to this.

We should not have to update all our code that relies on something as fundamental as events to reflect this change that nobody really needs (and even if it were needed, this is still inappropriate as far as ergonemics go) it’s going to for sure increase performance in places where signals are abused, but the compatibility disadvantages far outweight that. Please make this forever optional, or just never force this change in the first place.

3 Likes

Thanks for all the feedback, everyone. This feature is clearly proving to be much more complicated than we anticipated, and we will adjust the development and release plans accordingly. I want to go over some misunderstandings in this thread, and our plan for next steps - you’re welcome to ask for clarification on any given point but please bear with us.

Please read this in full before posting further comments on the thread.

Is this change necessary?

There’s a misconception that this change breaks existing games for no benefit. Our view is that if we don’t ever do this, in 5 years the platform will have been much worse off. I won’t belabor this too much but just three quick examples:

  • Client and server-side processing of incoming packets is largely single-threaded. We parallelized physics processing to an extent and are working on some event improvements on the server side, but while we have a contract with developers that events from properties coming in the network stream immediately invoke Lua callbacks, this will not change.
  • The events around instance reparenting are very complicated; doing any modification of the said hierarchy from a Lua callback can often result in bugs that are extremely difficult for us to fix. We have found cases where these mutations can break subtle invariants in engine state, cases where this can result in complex memory leaks, and all of these are very difficult for us to fix systemically without performance degradation in a different way.
  • Due to the sporadic nature of Lua events, it’s difficult to ensure the engine sandbox safety. We want to make sure it’s impossible to compromise security of any system that runs any Roblox application; many vulnerabilities we’ve fixed in the past would not have been possible to achieve if this change was in effect.

In short - it’s not just that there’s some minor performance gains to be had from this change - the immediate nature of Lua event invocation coupled with the very high density of the event invocation points severely hinders our ability to innovate wrt performance and robustness.

When will this change happen?

Something that we should have been much more clear about in the initial announcement - the reason why we don’t have a timeline for this yet is precisely because we don’t know to what extent:

a) The new set of rules works for most existing games
b) We can expect developers to adapt to the new set of rules in a timely manner

We tested this change internally, however unfortunately we can’t really test this on any games you have created, because we don’t have source access and we can’t just enable something like this for testing for a given game; as such, the set of different games we’ve tested was limited. Our intention was to start talking to the community to help highlight areas of concern and problems due to this change - hence this thread - but:

It is not our intention to break your games.

Our intention is to polish this set of behaviors until it works out of the box for most games with minimal set of issues, work with developers to test their games using the new mode, change the defaults after we’ve given plenty of notice, then work with developers who had to disable this change for their games to understand how best to proceed, and only if the set of games that had to opt out is very small, would we completely switch to the new behavior.

We would expect this entire process to take years. Our initial hope was that we would be able to change the defaults at the end of this year, but given the volume of feedback we will need to reassess to what extent this is practical.

Again, we will not change the behavior of your games until we’re convinced that doing so results in minimal disruption through a combination of us improving the system and developers adapting to change over time.

Should I switch to the new mode right now?

To give us a bit of time to carefully comb through the feedback, categorize it and make sure we have a plan for everything, we are changing the new mode to only be active in Studio. This more accurately reflects the status of this change, which is an exploration / beta.

Please still let us know when you find issues with this change. There is some leeway for us to change the behavior here; for example, people brought up FastSpawn as an example. I’m hoping that there’s no disagreement that FastSpawn is a hack and really shouldn’t exist - in fact, we have a set of new APIs like task.defer that should make it unnecessary, but unfortunately we didn’t correctly order this beta so you don’t have access to these - but if we need to keep it working, we can change rules around some events to be immediate, because specifically bindables are less problematic than other events as the engine will never trigger these directly.

Our intention is for the new mode to be reasonably easy to migrate to (this should be much less difficult than FilteringEnabled) - we are clearly not there yet but we will try to get there.

What are your next steps?

Again, thanks for all the feedback. Here’s what we’re doing in response.

We will carefully review each specific problem that people brought up and see if we can come up with fixes to the existing rule set to make code like this just work. Please continue to submit examples of code that gets broken as a result of this change, but please be as specific as possible to help us debug this problem.

We have changed the new behavior to only be active in Studio for the time being to not give folks a wrong impression - we have bugs to fix and more testing to do. We will not give a timeline for any changes in existing behavior until we’re confident that the new behavior doesn’t have bugs and we’ve done all we can to mitigate the breakage from our end.

We will perform more internal testing, reaching out to some of you that expressed concerns about the game malfunctioning as a result of the change and working with you to understand how to best proceed.

We will release a separate document on DevHub explaining the difference between old and new behavior in detail as well as explaining when this change can lead to issues and how to best fix them.

We will release the task.* APIs to make sure developers have good clean alternatives to migrate to when using this new mode, when this is necessary.

We will investigate and fix bugs in our own scripts, thanks @DataBrain for bringing these up.

After this is done, we will enable this change on client/servers again (only for games that chose to opt into the new behavior) which would mean that developers can start adjusting their games in hopefully minimal ways to make them compatible with this change.

Only after we see some initial feedback from that will we be ready to announce the first timeline - one where the change would become the default (with an understanding that for any developer that has a game severely impacted by this it takes a single click in Studio to go back to the old behavior).

And it’s only after that that we will be able to see, based on working with developers on this migration, whether a full switch to the new behavior is practical and when it can realistically happen. Expect this to take years.

115 Likes

How come you don’t check it on your side?

2 Likes

@zeuxcg Thanks for this very clarifying and thoughtful response.

Small point on FastSpawn, since I do agree it’s a hack and should not exist—I don’t think task.defer (if it works in any ways similar to golang’s defer statement) has the same effect that is trying to be achieved with FastSpawn; rather, FastSpawn is meant to run a new coroutine immediately (which is already possible with coroutine.wrap and coroutine.create), but in a separate thread that does not stop the current thread if there is an error, while maintaining full debug functionality (i.e. being able to click on the error or parts of the traceback from a thread that was fastspawned and have it link back to the source script). Most people prefer FastSpawn over coroutines for debug purposes only. Typically this overhead isn’t significant enough to cause lag compared to the code processes in general or just the Roblox engine itself.

That’s why my FastSpawn implementation goes through the overhead of using BindableFunctions; even though it spawns a new coroutine to call that bindable function, it abuses the fact that OnInvoke will create a new traceback for which, if an error occurs, it will be like it came from a unique entry point in a regular Script, LocalScript, or ModuleScript. The only difference is that this creation of a new entry point is not deferred, and runs immediately during the FastSpawn call, unless the thread being FastSpawned yields or errors.

10 Likes

Yeah, thanks for this clarification. The initial version of my reply had task.spawn in it but we changed it to task.defer; I thought that we already have a immediate spawn in our planned task. APIs but it appears that we don’t yet; we will address this as well.

22 Likes

Thank you for this detailed response, this definitely helps ease a lot of the initial worries I had about the rollout of this change and is incredibly appreciated. The lack of information was the main concern for me, as it was extremely hard to gauge just how much would be affected by this change and how we would be able to fix affected code. With more resources being made available to assist us with this change, this will definitely help make the transition go a lot smoother.

15 Likes

Exactly how much of a performance gain could this give?

1 Like

This update seems to be breaking some Roblox core scripts, too. Sometimes it’s as simple as a single, inconsequential error being thrown every time the developer console is opened, and sometimes it’s as broken as, well…

This error wasn’t even triggered with events. It’s odd to develop with with differed signals in mind when Roblox itself seems unprepared.

3 Likes

With the deferred behaviour will it be possible to do something like this?

local myEvent = Instance.new("BindableEvent")

myEvent.Event:Connect(function()
    wait(1.5) -- some work
end)

myEvent.Event:Connect(function()
    wait(0.5) -- some work
end)

myEvent:Fire()
myEvent.Event:Wait() -- thread resumes after all event handlers have finished (~1.5 seconds)

If you try the above code today, you’ll just yield forever because by the time you call Wait() on the event, the signal has already fired. If anyone knows of a clean way to do this let me know :slight_smile:

2 Likes

For people who’s complaining about this change.
If your game breaks because of it. that’s a sign of a bad game structure.
Because if so. your game was relying on a chaotic behavior that will eventually bug down when the right conditions are met.
This new update is really good. Roblox is making sure the results are consistent with-in all sessions for events firing order. From what I’ve seen, they really needed to change this because they’re working on synchronizing roblox instances with parallel lua I suppose.

This sort of messed up the respawn logic inside one of my games. I use the CharacterRemoving event to check the player’s position right before they respawn so I can set the respawn location to the nearest spawn rather than a random spawn location. Since the event is deferred, it runs after the player respawns, which completely ruins the logic. I would expect this kind of behavior on the CharacterRemoved event rather than the CharacterRemoving event.

2 Likes

In Deferred SignalBehavior mode, it seems that changes to gui items are also deferred. For example making a gui element like a Frame not visible changing is Size and/or Position then making it Visible again will give ghost artifacts of the Frame in the previous position/size before it is shown in the new position/size.

The simple example below makes a frame not visible, make the size 0, 0 and moves it to the position of the left mouse click. Then makes the frame visible and tweens to size. 1 seconds later it makes the frame not visible. When you left click again you will see a flash of the frame in the old position. This does not happen in Immediate mode.

At this point the only way to avoid the artifacts in Deferred mode is to add a wait() after changing the size and position before making it visible again. This is not a desirable and definitely not a performance boost.

local Players = game:GetService("Players")
local player = Players.LocalPlayer
local pGui = player:WaitForChild("PlayerGui")
local mouse = player:GetMouse()

local DEFAULT_FRAME_SIZE = UDim2.fromOffset(100, 100)
local MINIMIZED_FRAME_SIZE = UDim2.fromOffset(0, 0)

local sGui = Instance.new("ScreenGui", pGui)
sGui.Enabled = true

local frame = Instance.new("Frame", sGui)
frame.Visible = false
frame.Size = DEFAULT_FRAME_SIZE
frame.AnchorPoint = Vector2.new(0.5, 0.5)
frame.BorderSizePixel = 0
frame.Position = UDim2.fromOffset(50, 50)
frame.BackgroundColor3 = Color3.new(1, 0, 0)


local debounce = false

local function positionFrame(pos: Vector2)
	frame.Visible = false
	frame.Size = MINIMIZED_FRAME_SIZE
	frame.Position = UDim2.fromOffset(pos.X, pos.Y)
	frame.Visible = true
	frame:TweenSize(DEFAULT_FRAME_SIZE, Enum.EasingDirection.In, Enum.EasingStyle.Quad, 0.5, true, function()
		wait(1)
		frame.Visible = false
		debounce = false
	end)
end

mouse.Button1Down:Connect(function()
	if not debounce then
		debounce = true
		positionFrame(Vector2.new(mouse.X, mouse.Y))
	end
end)
7 Likes

Hopefully there is a noticeable performance improvement in big servers with this feature enabled. I tested my game and it seemed like nothing was broken.