My bad scripting habits, and how do I fix them?

Hi!
I’d say I have gotten pretty good with Scripting over 3 years. The only thing is, I have some bad habits with it. I use wait() and while do (while wait() do) too much. I have been told to stop using wait() as it just slows down after time, and while do loops just make the game lag. I have heard about loops such as:

for I = 1,x do

but, that just wouldn’t work for me as sometimes I just need an infinite loop.

Do you have any suggestions?

1 Like

From what I’ve seen, a lot of what developers do in while wait() do loops are things that should be done in RunService event handlers, like on Heartbeat, Stepped, or RenderStepped. The main game “loop” on your server, for example, should not literally be one of the Lua loop constructs (while, for, repeat), it should be a function connected to RunService.Heartbeat.

In Roblox development, anything that you think of as an “infinite loop”, i.e. meant to run the whole time the server is up, should not be a loop. for and while loops should be used to iterate, either numerically, or over collections (arrays, table keys, etc). Game event loops and input handling should not use these constructs.

1 Like

The porblem with RunService is it updates every frame, making it more laggy than a while wait loop.

It updates twice as much, but it really shouldn’t be a performance concern unless you have really poorly optimized code.

This is a misconception. While the Heartbeat event fires every frame, you are in total control over how often your code executes, and how much code executes per frame. It’s trivial to make your Heartbeat function only run code once per second, or once per 100ms, or every 5th frame, etc. The overhead of one function call per frame is insignificant.

Here is a simple illustration you can run in a server Script to see something that will happen once per second for the life of a game server:

local RunService = game:GetService("RunService")
local accum = 0
local t0 = tick()
local counter = 0
RunService.Heartbeat:Connect(function(dt)
    accum = accum + dt
    if accum > 1 then
        counter = counter + 1
        accum = accum - 1
        print("Tick",counter," happened, it is now",(tick()-t0),"seconds since game start")
    end
end)

Now compare with the output of this code, which would be the wrong way to do a 1-second game server timer:

local t0 = tick()
local counter = 0
while wait(1) do
    counter = counter + 1
    print("Tick",counter," happened, it is now",(tick()-t0),"seconds since game start")
end

You will see that because wait(t) waits for at least t seconds, usually longer, that this clock quickly accumulates error and drifts. But that’s not the worst thing about it; if there were any code in here that could error, rather than just a print statement, this loop would terminate and not restart–a bad thing if it’s your server game loop.

4 Likes

Eh. While it’s true that using RunService.<Event>::Wait is preferred to use over the built-in wait global, you shouldn’t be replacing all of your while <condition> do loops with RbxScriptConnections created by calling <Event>::Connect because it creates a new thread using your callback each time the event is fired. Not only that but there’s no guarantee the events you connect will all fire in order unless you hardcode it, and no priority than one will run before or after another unless - again - you hardcode it (or use BindToRenderStep if on client).

That’s a good way to utilize Heartbeat but again it’s spawning threads that aren’t guaranteed to run in order, when we could just have a single loop

local Heartbeat = game:GetService('RunService').Heartbeat

local elapsed = 0

while true do
	local step = Heartbeat:Wait()
	elapsed += step
	
	if elapsed % 1 < step then
		print(string.format('%d seconds since game started', elapsed))
	end
end

TL;DR: if you’re going to do a while <condition> do loop then use a RunService.<Event>::Wait yield because it’s a lot more precise than the built in wait() global. And only use .<Event>::Connect if your intention is to spawn a new thread each step

1 Like

I believe depending on the situation, your solution is excessive.

Making all your loops dependent on render stepped and heartbeat is impractical as sometimes you don’t need a perfect 1-second loop. Additionally, your solution ignores factors that require yields before the loop continues.

I personally see nothing inherently wrong with making a game loop based on an infinite while loop. Binding this to an false loop which is event based seems impractical.

You should not try to circumvent errors like that. If you have a level of unpredictability such as an async function like HTTP:GetAsync, you should use a pcall. Other than that, you should debug and check your conditionals to ensure your loop doesn’t break prematurely.

That’s not what I was recommending. I was calling out one particular pattern where developers use infinite while loops for code they want to loop for the lifetime of their server. In most cases, binding to a RunService event is going to be the more robust option.

But things like async http calls erroring are predictable unpredictability :smile: You should pcall those either way. That will not save you from your own bugs, and unforseen conditions that can just happen from time to time that you couldn’t predict. How you drive your game loop can make the difference between any small bug or occasional error being able to bring down your whole game server, and a game server that just logs the error to the console and powers on.

Keep in mind, this is RunService.Heartbeat’s intended purpose. One can argue that rolling your own solution with while loops, extra pcalls, and restart/watchdog mechanisms is a solution that is circumventing the intentions of the engine developers.

I’d like to argue that “unforeseen conditions” is just a fancy phrase for laziness. A simple if statement can solve any unforeseen condition you may have. This is why QA testing exists to find these conditions you may have missed.

Additionally, if you can’t predict your own code you should consider writing new code that you can predict. Thats eqivlent to writing an essay without being able to read what you wrote.

I just cant see the practicality in using an event that gets fired 60 times a second for a game loop which is only meant to be started maybe once every few minutes depending on the round. If the game loop is currently running, I have to essentially ignore all the other attempts to start the loop again until it’s supposed to get started again all for the sake of preventing an error from breaking a loop.

Don’t get me wrong though, Im in no way trying to discredit your solution and mark it invalid, I just believe the particular use case of your solution with game loops is impractical.

Realistically I suppose this all comes down to personal preference.

Depending on what point in the game loop the system errors, it’s possible the loop won’t be able to restart seamlessly unless you design the entire thing around the idea that it’s going to terminate prematurely.

There should at no point be “extra pcalls” in any system.

1 Like

But it’s not necessarily my code that I have to worry about. We’re not publishing games to a specific version of the Roblox engine; it’s a live environment with changes every week that are not predictable. You can’t unit test against features or not-quite-fully-backwards-compatible character structure or API changes that don’t exist yet.

I’m not exactly sure what your concern is here. If you have a game loop that only runs during the round, and not during intermission, then you just Connect it at the start of the round and Disconnect it at the end.

I believe you misunderstood what I was trying to say here. Im talking about the entire game loop as a whole. In the context I was speaking in, the primary game loop would restart at the start of every round in a standard round based game. Not that there is another loop running at the same time while the round is on-going.

I appreciate this conversation, I believe it is very constructive and lots of interesting points are being made which may help developers formulate their own conclusions and ideas.

Could you give an example of this? Honestly I can’t think of a situation where binding a callback to RunService would be better than creating a coroutine with a while loop in it

So, by “game loop”, what I meant was the fundamental periodic code executor of the game server that starts when the server starts up, and doesn’t stop until the server shuts down; the regularly-scheduled clock off of which you drive all time-based things on your server. This is what RunService.Heartbeat is intended for.

It sounds to me like the notion of “game loop” you’re refering to is a cycle in your game state. i.e. “restart at the start of every round” sounds to me like game state being reset. These things are happening a layer up from the game loop I’m talking about. Anything that’s time based or cyclic ultimately has to be driven off of some periodic event, whether it’s heartbeat or a iteration of your own while loop. I’m merely saying that for a Roblox server, the best practice is for the lowest-level driver of what’s meant to be an infinitely-running machine should be the Heartbeat event, not a user-implemented infinite loop. It’s just generally more robust.

With all the necessarily error handling, what you propose is fine. But now you’ve got a coroutine wrapping the loop, and you’re yielding on Heartbeat:Wait(), so this no longer matches the common bad pattern I was objecting to. The bad pattern that results in “dead lobbies” in a lot of round-based games is when the game loop is literally something like this…

while wait() do
   --< All of the game server's per-frame logic* *>--
end

or

while true do
   --< All of the game server's per-frame logic* *>--
    wait()
end

… just at the bottom of their main server Script. No coroutines. No Event:Wait() calls. Just a loop and vanilla wait() like the OP is asking about. Usually no pcalls either, or only pcalls on Async things that are known to throw.

And what @Stratiz mentioned above, about servers not just being able to “power on” through errors is definitely true. I was thinking about transient errors only; if you hit a constant blocker, your server can certainly still die at the erroring line of code every frame no matter how you coded your loop. But for transient errors, a function bound to RunService can recover without a watchdog or having to pcall everything that could ever possible throw an error (which again, is out of developers’ control).

**by “all of the game’s per-frame logic”, I don’t necessarily mean it’s all inlined, just that functions called from the loop are the top of the call stack for majority of the game server logic.

There are many zealots who have their favorite peeves about potential pitfalls–Often they are overstated.
A common practice which I think is much more likely to bite you is not being vigilant about the use of
WaitForChild(). This one can lurk for a long time and not show any signs of a problem because
object hierachy loading can often be quite consistent. But it is not guaranteed. And as soon as Roblox
changes some code, or servers get sluggish, say on a holiday, or they are being attacked, etc.
All those timings that you thought were pretty much fixed or stable go straight out the window.
It also can happen that things can be so delayed that you should be mindful of failing gracefully.

Something to keep in mind, Numbers 1-255 will always be faster than strings, which hints to why enumerate exist.

Bools will always be faster than functions, hints why true and false is an option for many things.

Initalise variables once, don’t call a function more than once if needed, if you need to call it more than once, split it up into smaller functions to work as a collective whole.

Look into OOP in Lua, this keeps code clean and organized.

Free up unused variables and tables/indexes/keys when they are not needed. This will free up memory.

And here are some articles to help:

I believe you could do,

for x = 1, math.huge do

but I am not sure about the performance of this.

1 Like

do not do

for x = 1, math.huge do

this will only end badly, a while true do loop is infinitely better in this situation, as it will always resolve to true immediately, it will not take up as much memory, and not store a variable that will increase forever (since math.huge is literally equal to inf, or lua’s infinity)