Simple BindToClose() Data handling

I’ve been lurking around the forum on how devs use BindToClose() to save data when the server shuts down. One thing that has caught my attention is everyone seems to ‘overcomplicate’ it when saving a player’s data. Why not just save the data and wait(30) as the server will yield for 30 seconds until it shuts down. It seems doing this would be the ‘safest’ way. You use the full 30 seconds.

game:BindToClose(function()
	
	for _, client in ipairs(game:GetService("Players"):GetPlayers()) do
		saveData(client)
	end
	wait(30)

end)
5 Likes

:open_mouth: Wait you can do that?? If it works then I guess there’s really no reason not to.

The way you’re doing it isn’t concurrent. You’d want to call the saving function in a new thread for every player, then wait until all threads are complete before shutting down, not necessarily the maximum 30 seconds.

Yes I use this too, but it is also important to account for a DataStore error. But by the looks of it, the example you use seems to be considering DataStore2 so there is probably functionality for that already built-in.

But for people who use plain DataStores, a loop would be helpful in case an error is thrown.

game:BindToClose(function()
	local playerCount = #game:GetService("Players"):GetPlayers()
	for _, client in ipairs(game:GetService("Players"):GetPlayers()) do
		saveData(client)
        wait((1/playerCount)*30)
	end
end)

Correct me if I’m wrong but wouldn’t that work ?

By threads he means coroutines:

game:BindToClose(function()
	for _,player in ipairs(game:GetService("Players"):GetPlayers()) do
		coroutine.resume(coroutine.create(function()
			saveData(player)
		end))
	end
end)

Someone else can explain coroutines if they like.

No, that’s in the same thread, you’d do this:

local CompletedThreadCount = 0

for _, Player in pairs(Players) do
    coroutine.wrap(function()
        SaveData(Player)
        CompletedThreadCount = CompletedThreadCount + 1
    end)()
end

while CompletedThreadCount < NumberOfPlayers do
    Services.Run.Stepped:Wait()
end
4 Likes

Oh my bad. I should probably go drown myself in this knowledge of coroutines

Do correct me if I’m wrong, but I am quite sure that the wait is ignored. There are no tasks to schedule after the yield so the server closes as in when the data has been saved because all operations are complete at that point. You shouldn’t really have a reason to use wait in BindToClose either and I feel like artifically stalling server closure has some kind of repercussions somewhere.

2 Likes

You’re spawning threads for this, so yes you do need to wait until they’re completed. There’s no rule against delaying server shutdown, you can do it for no reason, that’s why there’s a 30 second limit.

Not what I’m saying. Having a wait at the end of the for loop is unnecessary (as per OP’s code) because that wait will not get called in the first place until after all iterations of the for loop have completed, by which player data will have saved by then. Exceptions go to if saveData is a coroutine.

There is a 30 second limit before servers forcibly close to ensure that you have enough time to perform all your last-minute cleanup actions on games. In this case, nothing is being ran after the wait and the wait does not get called until the for loop completes, nullifying its purpose being there at all.

2 Likes

The problem in the original post is that he didn’t know that his code wasn’t concurrent, not that there was a wait. The solution was already posted.

I’m unsure of what you’re trying to point out though? The comments I’m making are in respect to having the wait there at all and how it’s pointless to include in respect to the code that was posted in the OP, not anyone else’s. It’s unrelated to any kind of solution.

2 Likes

The point I am trying to make is that the solution was posted, pointing out what was wrong, and then you replied four hours later with wrong and unhelpful information (“You shouldn’t really have a reason to wait in BindToClose either”) and suggested “artificially stalling server closures has some kind of repercussions”. The suggestion is to check if the post has already been solved before reviving it.

I won’t reply anymore here since it’s just dragging this thread on even further.

There is a reason I explicitly started off my post with “do correct me if I’m wrong”, to which you’ve failed to do so. Mentioning that I’m replying four hours later is irrelevant and moot to bring up, and I’ve yet to receive any word on how I am wrong unless I am not understanding something.

My understanding is simply standard understanding on the fact that all this code runs sequentially, so logically the wait is not applicable until after the for loop runs. Again, that makes the wait in itself pointless to include because saving operations will have been completed by then, and I believe that the wait is ignored because there are no other operations to run after it.

If I’m supposedly providing “wrong” and “unhelpful” information, then make an effort to actually help me understand how it is that way over brushing me off because you’ve posted something that is supposedly the solution (or if someone else did). Furthermore, what you called a “suggestion” was a feeling. It’s not an assertion nor a statement, it’s a feeling. Read my post carefully.

It’s not difficult to inform me what’s wrong about my feeling and if it’s worth having at all, which you’ve failed to do so. Please do not misinterpret the meaning of my words.

Furthermore, replying to solved posts holds no issue in any capacity so long as there’s a valid reason for doing so. Questions, comments, concerns, requests for clarification, extra information and so on are valid reasons for creating posts here. This is a learning environment.

5 Likes

It just occurred to me, when the server shuts down, wouldn’t it trigger the player removing and bindtoclose()

Couldn’t that lead into issues with regards to adding the data save to queue?

Because I get this occasionally. image

That’s why it’s a good idea to use a system which incrementally saves users data during gameplay, not just when they leave / on shutdown. Then when the server is shutting down many players will already have had their data saved since the last change and can be ignored.

Plus, the server could also randomly crash / lose power. BindToClose() is merely a courtesy, there’s no guarantee that things are always going to shut down smoothly.

2 Likes

I believe that’s why a small percentage of my players keep losing their data. It only seems to occur when the server dies or shuts down. I’ve just finished an auto save system which I plan on pushing soon. Would every 2 minutes be good? Or could that lead to problems? Keep in mind the max players per server in my game go up to 40 players.

Ideally it should be based on the request limit. Basically, keep track of your “request budget” and save at dynamic rate which is as high as you can while staying comfortably under the limit. The reason you have to be careful is because you burn some of the request budget when players enter. If a bunch of players enter at once then you will need to have data store requests left over to properly load their data.

1 Like