BindToClose API Improvements

Updates Announcements
#22

This announcement also provides some information around game servers and memory limits: Increasing Server Memory Experiments

8 Likes
#23

Thanks a lot! This is such a good change! Our game had a problem in the early days where memory leaks caused server crashes. Everyone in the server would lose their hard-earned winstreaks if they were mid-game during a server crash.

Now, this seems to never happen, but if it ever does, I can be confident that they won’t be recognized as having “left the game during a match” and losing their winstreak, because BindToClose automatically removes the mid-game status in our game.

6 Likes
#24

This helps improve the migration feature, it allows us to trigger migration teleports when necessary.

It helps, but not 100%, and I appreciate that this was added. It’s very useful.

8 Likes
#25

Just want to throw my hat in the ring to support this idea.

This is absolutely the main use case for us and, pending an official analytics integration, all I see us using this Enum for is passing it to our own analytics to get this data.

9 Likes
#26

Pretty cool, I like this kind of additions, keep it up :pray:

5 Likes
#27

This is a great update! What happens if a private server owner shuts down their server? Does it count as a developer shutdown or something else

5 Likes
#28

I would really appreciate if BindToClose function would return RBXScriptConnection, so I can disconnect the event. Right now it returns () (AKA nothing), and there seems to be no way of disconnecting it.

My library allows to start a service, and stop the service. If the service is stopped by the script, it can’t disconnect BindToClose and will most likely result in a memory leak.

4 Likes
#29

I don’t know about BindToClose returning an event, however, for your issue, I’d make your service store the methods in a seperate table that can be accessed from DataModel:BindToClose and just add/remove methods from the list when necessary.

And, in the BindToClose method, you can just iterate over the list and call each method. If the list is empty, nothing happens

function service:BindToClose(identifier: string, method: (closeReason: Enum.CloseReason) -> ())
    methodsList[identifier] = method -- assign the method to the list of methods
end

function service:UnbindFromClose(identifier: string)
    methodsList[identifier] = nil -- remove the method from the list
end

-- ^^ This is a more simplified version of what I mean


-- (a seperate script)
-- Run each method from a BindToClose callback
game:BindToClose(function(closeReason)
    for _, boundMethod in methodsList do
        task.spawn(boundMethod, closeReason)
    end
end)

But I don’t know if this would work for you

5 Likes
#30

This is awesome!

Can we get a separate reason for when it’s closing because of Studio closing or play-test stopping? That way, we can skip saving, et cetera if we want.

4 Likes
#31

Assuming BindToClose does run asynchronously, you’ll need to yield your main BindToClose callback since everything is being ran asynchronously.

3 Likes
#32

Wouldn’t RunService:IsStudio()/RunService:IsRunMode() suffice?

7 Likes
#33

I would suggest this as well ^

7 Likes
#34

BindToClose is called when the server shuts down, so I don’t think it matters if there’s a memory leak that late in the server lifecycle

3 Likes
#35

Memory leak occurs because functions keep getting added via the BindToClose method. Not when the functions in the internal list are called. The continous growth of the internal list of functions that you cannot remove from is a problem.

-- Pretend that this is C++ code
local callbacks = {}

local function onBindClose(callback)
    table.insert(callbacks, callback)
end

-- This is Luau code (with ability to call "onBindClose")
for i = 1, 100 do
    onBindClose(function() end)
end

-- Pretend that this is C++ code
print(#callbacks) -- 100
4 Likes
#36

I am aware of this workaround, but it doesn’t solve the problem that there is no way to remove function from the internal list. It’s a bad design in my opinion, because If I can add a function, I should always be able to remove it.

It should either return RBXScriptConnection or a function that you can call.

local connection = game:OnBindClose(function() end)
connection:Disconnect()

local disconnect = game:OnBindClose(function() end)
disconnect()

The OnBindClose method is a bit weird, because it doesn’t follow the current standard where if it was implemented today, it would’ve been something like this:

local connection = game.Closing:Connect(function(reason)
    print("Game is about to close", reason)
end)
connection:Disconnect()

So, most likely the disconnect function would be more fitting. At end of the day, I don’t care how it is implemented, as long as there is a way to remove the function I passed into OnBindClose from the internal list.

3 Likes
#37

Bump! Would be very insightful to know how many servers have crashed due to faulty code, versus planned shutdowns and the like.

1 Like
#38

But still, why not divide this reason into 2 different ones? It makes sense and adds more freedom to developers, that’s even the point of this update, to give developers the opportunity to find out the reason for closing the server. But by combining 2 slightly similar reasons into one that will describe both of them, you put the responsibility on the developers to write a function that determines whether the reason for the closure is the closure of the playtest studio or the shutdown of the game servers by developer.

It makes much more sense (at least for me) to write a function that will determine whether this is a DeveloperShutdown at all, rather than function that will determine if current DeveloperShutdown is a studio playtest closure or developer servers shutdown. Also, determining the real reason for the closure of the server by another service creates the feeling that the update hasn’t been thought through and the developers have to use crutches to figure out a real closing reason in case of getting closing reason argument with DeveloperShutdown enum value.

But anyway, thanks for such an update, this might be really helpful for developers to know reason of game server closure!

#39

Good change, but can we also get why PLAYERS shutted down? Internet problems, just left, kicked or smth else?

3 Likes
#40

There are literally no negatives to this though? Implementing this reason already gives you the option to fine tune your shutdown handler conditions yourself, pretty easily I may add. It’s not even remotely close to a crutch, programming is about piecing things together, just like any API in this engine.

Anything they gave and will give us can technically always be “easier”, but at some point the developers need to take care of the rest because there will always be a specific “improvement” for someone else.

3 Likes
#41

Can server crash go in here? What can other possibly unknown reasons go there?

1 Like