[Beta] Deferred Lua Event Handling

Updates Announcements
, , ,
#83

One of the uses for BindableEvents was to do an instant resume which worked with the coroutines of callbacks and module scripts.

BindableEvent Example 
local b = Instance.new"BindableFunction"
local function yields()
	wait(1)
	return 1
end
function b.OnInvoke(f) -- f always yields long enough so that the call to Wait happens
	local e = Instance.new"BindableEvent"
	coroutine.wrap(function()
		e:Fire(f())
	end)()
	-- ...
	return e.Event:Wait()
end
print(b:Invoke(yields)) --> 1

Using the coroutine library for this doesn’t work.

Coroutine example 
local b = Instance.new"BindableFunction"
local function yields()
	wait(1)
	return 1
end
function b.OnInvoke(f) -- f always yields long enough so that the call to Wait happens
	local t = coroutine.running()
	coroutine.wrap(function()
		coroutine.resume(t,f())
	end)()
	-- ...
	return coroutine.yield()
end
print(b:Invoke(yields)) -- nothing

With this change, how can the coroutine of a callback or module script be resumed instantly?

2 Likes
#84

There are several things in my game broken both loudly and silently by this change, most of the loud ones are third party libraries. I haven’t looked into how difficult they will be to fix, but I don’t imagine it will be too bad in my case, probably ~5 weekends assuming the problems don’t get bigger and more numerous the more I look…

I would still definitely appreciate the performance improvements associated with collapsing duplicate events though. (It sounds like this would also solve the issue with Selection events in Studio firing once per instance deleted when you are deleting many instances at once).

I think this is a significant miscalculation.

19 Likes
#86

Perhaps you’d like to share what some of these optimizations might be instead of staying silent? Might make it easier for us developers, who will have to do the work of rewriting code in preparation for change, more likely to accept the change. Might also help us figure out if we even want this change to begin with, depending on how great the optimizations that come from it will be.

Going “we’re going to force you to rewrite your code but trust me it’ll be really good in the long run because we’ll do some good optimization but no I won’t tell you what they are” does not really give me confidence that these optimizations will, in fact, be worth it.

15 Likes
#87

I checked an old game of mine to see this change for myself. Pretty much on the same boat as you. I’m more scared about the silent breaks more than anything. Behavior that I didn’t intend to happen. I probably won’t try to fix any of the issues though. Wasted time imo.

I have an actual response to this but, it was flagged as off-topic for the right reasons. I’ll move it to a discussion post once I get confirmation that it’s fine to post it there.

17 Likes
#89

I activated Deferred in my game and I didn’t pull the short straw, but I think I noticed a difference :rofl:

I’ve looked deeper into the camera problem mentioned by @DataBrain:

Using this code I’ve determined the timing of InputChanged within the game step pipeline:

local STEP_INDEX

-- 0 to 2000 is from Enum.RenderPriority.First to Enum.RenderPriority.Last
for i = 0, 2000 do
	RunService:BindToRenderStep("RenderIndex" .. i, i, function()
		STEP_INDEX = i -- During BindToRenderStep
	end)
end

RunService.RenderStepped:Connect(function()
	STEP_INDEX = 2001 -- After RenderStep and BindToRenderStep
end)

RunService.Heartbeat:Connect(function()
	STEP_INDEX = -1 -- Before RenderStep and BindToRenderStep
end)

UserInputService.InputChanged:Connect(function(input, processed)
	print(STEP_INDEX)
end)

In Immediate mode, InputChanged fires at index -1 which is before BindToRenderStep - Naturally, users would be anticipating input information to be available during BindToRenderStep to progress user-controlled game states.

In Deferred mode, InputChanged fires at index 2000 which seems to be after BindToRenderStep and before RenderStepped - This completely leaps over a coding design where user input is processed first before the rest of the game code runs during a game step. If InputChanged will fire consistently before RenderStepped, then the developer could just make a custom BindToRenderStep :rofl:

In Layman’s terms:
image
(But sort of not, because it’s after BindToRenderStep and before RenderStepped which feels awkward)

User input getting yeet’ed forward will break a good bunch of user controls code including roblox’s own.

Another relevant problem will meet developers not forking roblox defaulf controls but overriding Humanoid.Jump set by the control scripts… I think this hack could be improved by ditching Jump listeners and checking the value directly at different BindToRenderStep priorities.

32 Likes
#90

Seems like a really bad update that should never be forced onto developers.

Completely breaks PlayerAdded and other critical events in Studio, and developers should not have to write Studio-specific code to counteract PlayerAdded and other events no longer firing as expected within Studio.

12 Likes
#91

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

image
image1680×1010 311 KB 

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
#92

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.

31 Likes
#93

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
#94

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

1 Like
#95

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”

image
image730×187 12.5 KB

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
#96

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

#98 
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.

4 Likes
#99

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.

123 Likes
Documentations for the new task class?
Release Notes for 480
#100

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

2 Likes
#101

@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
Documentations for the new task class?
#102

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.

23 Likes
Documentations for the new task class?
#103

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.

17 Likes
#104

Exactly how much of a performance gain could this give?

1 Like
#105

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…

rob
rob911×522 19.9 KB

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