Multiple events, bad practice?

Help and Feedback Scripting Support
#1

My setup in studio when creating a game is generally:

Server

Datastore Module
Physics Module
States Module
Stat Handler
Character Loader
Other Modules Related to the game

Within these modules, I’d have a PlayerAdded event in a few of them as well as CharacterAdded.

My question is would it be better to have 1 event per and then run all the functions and stuff or what I’m doing is fine.

I feel like I’m using a lot of lag by creating many events and listeners.

I’d like to improve upon my habits for better scripting.

7 Likes
#2

Not sure if it’s related, but I recently went through the hassle of learning about memory leaks and their prevention. You mentioned you create connections in each module - these would take up space in memory, correct? Those would only be created on the initial run of the module even if you’re requiring them more than once, but those connections could create references to objects that don’t get properly garbage-collected.

Sorry if I’m spouting useless info and you’ve already made sure nothing leaks. Just adding my own two cents.

#3

Thanks, I’m trying to understand memory and how to improve general performance. Could you tell me more about not cleaning up objects? Also I thought that every time you required the module it would create a new event / listener.

EDIT: I create bool values and instances and parent it to a player. Do I have to set these variables to nil after? For example:

game.Players.PlayerAdded:Connect(function(Player)
local StatusFolder = Instance.new("Folder")
StatusFolder.Parent = Player

StatusFolder = nil
end)

Also I was told that each time that events run they create a new thread, do these threads get cleaned up? Also doesn’t ROBLOX auto garbage collect (During certain intervals)?

1 Like
#4

Don’t worry about creating too many connections in this case. However, in practice it’s usually best to perform all player joining logic in a single place, because usually there are very basic things you have to do such as loading player data before you want to do anything else.

For example, if a player rejoins the same server before their last data has been saved and destroyed, you really want to wait until the player data code checks for this before doing much else.

1 Like
#5

I’d urge you to scour the devforum because that’s entirely how I learned - there’s almost zero official documentation on the subject. I guess Roblox doesn’t expect the average developer to run into those types of situations.

When you create any object you don’t plan to keep around forever - maybe a bullet, maybe a visual effect, or maybe your own character - when the object is destroyed, lua’s routine garbage-collection system will free up the memory used to locate/render the object - but only if no strong references exist to the object.

Strong references can be variables, functions, connections, etc.
Anything that tells lua ‘hey, this exists’ will prevent garbage collection from freeing up the memory it inhabits.

Generally, though, those circumstances are rare. Variables isolated to a scope - such as local variables inside functions or loops - lose reference once that scope ends. Variables created outside of scopes are still contained within the scope of the entire script, so once that script stops running (think of a localscript inside a character, like your default Animator), the variables created within it lose reference as well. You don’t need to set unused variables to nil, as they’ll be treated as nil once they lose reference.

The most common offenders - at least in my own practice - are connections that never break and tables that are never cleared. Even if a script stops running, connections are objects just like a part or a sound - they remain in memory until they’re broken, either manually, or because the object the connection was attached to was destroyed, such as a Touched event on a bullet being broken because the bullet was destroyed.

And if those connections aren’t broken, anything they reference within them will ALSO stay in memory.

Again, I’d do your own research because it definitely pays to do so, but I think your own code should be fine. If you’re suspicious, use the F9 console in studio and observe the Instances entry in the Server tab on the memory section. See if any interaction in your game causes a permanent rise in memory consumption, and then isolate it.

8 Likes
#6

The amount of memory all of these connections combined would take up is far less than the amount of memory a single high resolution image would use.

You misunderstand memory leaks. Having an active connection is not a memory leak. Memory leaks are when you no longer need memory but do not or prevent it from being freed.

This is not true, but this is mostly irrelevant. Temporary scripts are bad practice and should never be used, but if you do use them then Roblox will clean up the memory of the script when it is being destroyed.

It’s also weird to focus on memory, which is also irrelevant here.

#7

I agree. Although memory isn’t really an issue I still think it’s bad practice to have multiple scripts listening for the same event, especially when there could be dependencies between them. There may be logic issues if you aren’t sure which functions might be fired before others.

#8

I’ve asked this question in the past, or something to the extent (I was more interested in the scheduling of events). I think it would help you to take a look, as it does have an engineer response attached.

The TL;DR of it is that your memory consumption is negligible and connecting multiple listeners to an event isn’t bad. If it helps for readability, you can sacrifice memory not worth large concern over shooting yourself in the foot attempting to consolidate. Just be wary of how you organise your systems and dependencies.

1 Like
#9

One event and run all of the functions is certainly the easier path. The reason being that if you code it that way, you get to be explicit about exactly what order to run your initialization in, so that it’s entirely predictable. If you use a bunch of separate connections, and decide to rename your module or shuffle your module structure in the hierarchy around a bit then the order the event handlers are called in may change.

Multiple events is architecturally more sound, because it actually forces you to write orthogonal subsystems for your game, which can handle being initialized in any order / know how to wait on other other init not being complete yet. The downside being that you’ll have to write a lot more glue / framework code to handle cooperation between the modules.

For example: If you write everything in one event handler, part of that handler is probably waiting on the DataStore returning player data, holding up all other init while it happens. If you write your game with multiple independent inits that know how to cooperate it’s more likely that you’ll be able to get other init to happen while you’re waiting on the DataStore, in order to speed up the time between joining and getting to start gameplay.

2 Likes
#10

You should do an events based system of your own. You should probably never have a PlayerAdded listener to begin with, realistically the client should request loading from the server after it initializes what it needs to.

This is what my listener looks like:

local function HandleImmigrationRequest(Player)
    if Immigrants[Player.UserId] ~= nil then
        ImmigrationQueue[Player.UserId] = Player
        return
    end

    Immigrants[Player.UserId] = true

    Imports.Server.PlayerData.CreateTransient(Player)
    Imports.Server.PlayerData.LoadPersistent(Player)

    local TransientData = Imports.Server.PlayerData.GetTransient(Player)
    Imports.Shared.Broadcaster.Send("PlayerImmigrating", Player, TransientData.Data)

    CompletedImmigrants[Player.UserId] = true
    Imports.Server.NetworkServer.Send(Player, "CompleteImmigration")
end

Imports.Shared.Broadcaster.Connect("RequestImmigration", HandleImmigrationRequest)

I don’t see any advantage to doing much before data loads. There’s usually relatively little you need to do that you wouldn’t have to redo after player data loads.You’re not going to speed up much of anything.

#11

What in your experience brought you to that conclusion? I tried that approach once before and found that it just made things more complicated for little benefit.

If there’s only a single “the data loads” then yes. But if your userdata is more complicated and requires loading multiple keys you may have enough information to get them into the game before all of the the data they will eventually need has been loaded.

For example, if you’re making an RPG game. You probably want to pre-load the user’s bank contents as part of loading, but you don’t need them to get them into the game.

2 Likes
#12

What I do is mainly this:

  1. Client sets up event listeners.
  2. Client requests server creates data.
  3. Server creates, loads data.
  4. Server sends data to modules through broadcaster (BindableEvents abstraction, basically).
  5. Each server module can choose how to replicate data to a corresponding client module.

I want to make sure my client gets a chance to set up its event listeners before the server sends any data to it. This greatly simplifies logic on the client.

The main thing I’m concerned with is what happens when a player rejoins the same server before their old data got destroyed. I definitely don’t want to check this in every module. There’s other things you’d want to check, like if the player was banned for your game, that just makes more sense for me to do in one listener before anything else.

This is a fair point. I try to pack all my data into one key, but in certain cases (like freebuild tycoon games where you need to store arbitrary plot data) multiple requests might be necessary. I’d maybe use concurrency in this case.

#13

You’re worrying unnecessarily:

--  Server Script
game.Players.PlayerAdded:Connect(function(player) 
    game.ReplicatedStorage.RemoteEvent:FireClient(player, "Test")
end)

-- Client Script
wait(15)
game.ReplicatedStorage.RemoteEvent.OnClientEvent:Connect(function(...)
    print("Client got:", ...)
end)

Even with the wait(15) the client will still correctly receive the event data from the server.

9 Likes
#14

I didn’t know RemoteEvent events got queued if there were no listeners on them. Thanks!

I’d still probably stick with my one listener approach though, just because I still have code to run like bans or rejoin checking before other less generic join handling, but you’re right on this.

i still might keep requesting from the client to do initialization work that might yield first, but maybe not.

1 Like
#16

I try and follow the DRY principle, Don’t Repeat Yourself. Put everything important in modules in ServerScriptService after writing up a system on https://draw.io and Notepad; This personally really helps. I tend to avoid “listeners” server side and keep them to a minimum on client.

My main advice would be to try and create a thoughtful system using the DRY principle but make sure it’s human code. This may be worth a read? The importance of human code in game development. <–( A post I wrote a couple weeks ago.)

#17

Using one event means you have to marshall around the data using your own custom code. In other words, you add another layer of abstraction. IMO, it’s best to always use separate events, well-named to what they do. You’ll save yourself a lot of headaches down the road.

My co-workers would not be happy with me if I created an event that actually does multiple different things that are only defined in the code.

7 Likes