Need Review/Advice for my DataStore Handler

Help and Feedback Creations Feedback
, ,
1

Hi everyone! I made this DataStore handler (ModuleScript and ServerScript) a few weeks ago, and now I’d like to use it in another game. And I’m wondering if it’s solid or if there’s anything missing or worth improving.

Let me know what you think. I’d really appreciate any tips or improvements :>

Module Script:

local DATA_STORE_NAME = "PlayerDataStore"
local DSS = game:GetService("DataStoreService")
local PlayerDataStore = DSS:GetDataStore(DATA_STORE_NAME)

local PlayerData = {}

function PlayerData.SavePlayerData(player)
	local UserId = player.UserId
	
	local Data = {}
	for _,value in ipairs(player:GetDescendants()) do
		if value:IsA("NumberValue") or value:IsA("StringValue") or value:IsA("BoolValue") then
			Data[value.Name] = value.Value
		end
	end	
	
	local succes, errorMessage = pcall(function()
		PlayerDataStore:SetAsync(tostring(UserId), Data)
	end)
	if not succes then
		warn(errorMessage)
	end
end

function PlayerData.LoadPlayerData(player, CallBack)
	local UserId = player.UserId
	
	local success, result = pcall(function()
		return PlayerDataStore:GetAsync(tostring(UserId))
	end)
	
	if success and CallBack then
		CallBack(result)
	elseif not success then
		warn("Failed To load Data for" .. player.Name)
	end
end
return PlayerData

‎
‎
‎
Server Script:

local Players = game:GetService("Players")
local PlayerDataModule = require(script.Parent)

local Defaults = {
    leaderstats = { Coins = 300, Kills = 0, Wins = 0},
    Stats = { Level = 1, Exp = 0, Blocks = 0, Gems = 0 },
	Settings = { afk = true, Night = false, Music = true },
	Wins = { map1 = 0, map2 = 0, map3 = 0, map4 = 0}
}

Players.PlayerAdded:Connect(function(plr)
    -- 1. Create folders
    local folders = {}
    for _, name in ipairs({"leaderstats", "Stats", "Settings", "Wins"}) do
        local f = Instance.new("Folder")
        f.Name = name
        f.Parent = plr
        folders[name] = f
    end

    --
    for cat, stats in pairs(Defaults) do
        for statName, defaultValue in pairs(stats) do
            local className = type(defaultValue) == "boolean" and "BoolValue"
                            or type(defaultValue) == "string"  and "StringValue"
                            or "NumberValue"
            local v = Instance.new(className)
            v.Name = statName
            v.Value = defaultValue
            v.Parent = folders[cat]
        end
    end

    -- 3. Load saved data into those ValueObjects
    PlayerDataModule.LoadPlayerData(plr, function(data)
        if data then
            for _, v in ipairs(plr:GetDescendants()) do
                if v:IsA("NumberValue") or v:IsA("BoolValue") or v:IsA("StringValue") then
                    local newVal = data[v.Name]
                    if newVal ~= nil then
                        v.Value = newVal
                    end
                end
            end
        end
    end)

    -- This auto saves every 60 seconds
    local alive = true
    task.spawn(function()
        while alive do
            task.wait(60)
            PlayerDataModule.SavePlayerData(plr)
        end
    end)
    plr.AncestryChanged:Connect(function()
        if not plr:IsDescendantOf(game) then
            alive = false
        end
	end)
	
	-- dont mind this :) (its just a hitbox) 
	plr.CharacterAdded:Connect(function()
		local hitbox = Instance.new("Part")
		hitbox.Name = "PLRHitbox"
		hitbox.Size = Vector3.new(3, 4, 3) -- sized around torso
		hitbox.Transparency = 0.67
		hitbox.Massless = true
		--hitbox.Anchored = true
		hitbox.CanCollide = false
		hitbox.CanTouch = true
		local char = plr.Character or plr.CharacterAdded:Wait()
		local hrp = char:WaitForChild("HumanoidRootPart")

		hitbox.Position = hrp.Position
		hitbox.Parent = char
		-- Keep hitbox aligned
		local weld = Instance.new("WeldConstraint", hitbox)
		weld.Part0 = hitbox
		weld.Part1 = hrp
	end)
end)

Players.PlayerRemoving:Connect(function(plr)
    PlayerDataModule.SavePlayerData(plr)
end)
1 Like
2

As long as there are no breaks or Leaks no matter the lag, The Handler should be fine (i think so)

3

Seems pretty good to me, I do see room for one improvement, which is “error” handling loading and saving the data.

I would recommend implementing a “try” system, let me explain. In the function Save/LoadPlayerData, if the data fails to save/load, it just returns a warn. While this is unlikely, there is a possibility that Set/GetAsync could fail, you never know. So if you can’t load the player’s data on the first try, it just eventually gets overwritten.

What I personally do to combat this is by using an iteration loop (tries to save/load data multiple times, if failing saving after all tries then assume there is something wrong with our own code), (but if it fails to retrieve after all tries then we just make new data). Here is some pseudo code to help you get the idea, this is from my data saving module:

--// The module
local PlayerDataService = {}
local DataStore = DataStoreService:GetDataStore("Robloxing")

local PlayerDataTemplate = require(PlayerDataTemplateModule)

--// I have found that 10 is the magic number, if it STILL fails, then it is most likely a coding error with data handling somewhere.
--// It is very VERY unlikely that it will fail after 10 
local MaxTriesForDataStore = 10

--// Basic function to load data
local function GetData(User)
	local Data = nil
	local Success, Error = pcall(function()
		Data = DataStore:GetAsync(User)
	end)

	return Data, Success, Error
end

--// Basic function to save data
local function SaveData(User, Data)
	local Success, Error = pcall(function()
		DataStore:SetAsync(User, Data)
	end)
	
	return Success, Error
end

--// Basic data making function
local function MakeData(User)
	local Data = PlayerDataTemplate.New() --// The template table is made as a return function to prevent the template from being overwritten.
	local Success, Error = SaveData(User, Data)

	if Success then
		print("Created data for player:", User)
	else
		print("Failed to create data for player:", User)
	end
	
	return Data, Success, Error
end

--// User is the Player's UserId, Data is their data table
function PlayerDataService.Save(User, Data)
	local SaveDataSuccess = SaveData(User, Data)
	local Tries = 1
	
	if SaveDataSuccess then
		print("saved data for:", User)
	else
		repeat
            print("failed to save data for:", User, "on try:", Tries, "retrying...")
			task.wait(3) --// give the DataStore time to cool down. The wait is only added for saving just because saving is doing more work than loading.
			Tries += 1
			SaveDataSuccess = SaveData(User, Data)
			
			if Tries >= MaxTriesForDataStore then
				break
			end
		until SaveDataSuccess
	end
	
	if not SaveDataSuccess then
		warn("failed to save data on all tries!")
	end
	
	return SaveDataSuccess
end

function PlayerDataService.Get(User)
	local PlayerData = nil
	
	for Iteration = 1, MaxTriesForDataStore do
		local Data, Success, Error = GetData(User)

		if Data and Success then
			PlayerData = Data
			print("retrieved data for:", User, "on try:", Iteration)
			break
		else
            print("failed to get data for:", User, "on try:", Iteration, "because:", Error, "retrying...")
		end
	end

	if not PlayerData then --// Assume that there is no data for the user, so we create a new slate.
        print("failed to get data for:", User, "on all tries of:", MaxTriesForDataStore, "creating data...")
		local Data, Success, Error = MakeData(User)
		
		if Data and Success then
			PlayerData = Data
		end	
	end
	
	return PlayerData
end

return PlayerDataService

One other issue I could see is that if somehow the autosave loop saves at the same time a player leaves the game, you would get a DataStore queue warn.

Hope this helps!

1 Like
4

Yo, this is awesome. I’m gonna change it and let you know if I have any further questions. Also, I really like the way you cloned the default table
And about the last thing u mentionned I dont think theres anyway to avoid it right?

1 Like
5

No not really. Its better practice to have an auto-saving function anyways. Worst case is they both save at the same time and the datastore will just queue.

If you save a players data multiple times at the same time, it goes through the queue every 6 seconds.

Only issue you have to worry about is if the player leaves and rejoins during those 6 seconds for the data save queue, then you risk data not being saved, but the likelihood of this is small.

There are tutorials/posts on how to cache data and prevent this.

1 Like
6

Yeah you’re right I don’t think that would realistically happen. But uhh, I don’t see how caching data would help, since the DataStore still has to be called when saving whether you use a cache or not. Could you explain a bit more?

7

Not too sure off the top of my head. I know there is a few datastore modules, like ProfileService, that prevent the double data saving and also caches data. You can look through the source code if that helps.

1 Like
8

I’m pretty sure your datastore system is vulnerable to a common mistake that causes occasional, complete, data wipes

The issue stems from how you are loading the data. The default values are loaded prior to the datastore’s saved data, and nothing is preventing saving if loading fails. If loading fails (which isn’t uncommon), the save function will overwrite the player’s datastore with the default values

Your datastore system also lacks session locking (or an alternative to session locking) to prevent data loss from a player’s session due to the data being loaded in a new server before another server finished saving it. This issue is more minor, as the data loss in question would be small (especially with the autosave system), but it can allow players to abuse this flaw to duplicate items or money, if you have stuff in your game that allows items or money to be transferred to another datastore (for example, a trading system between players)

I went into depths about session locking at the end of this thread, which had similar issues to your script

I also have this thread I wrote about an alternative to session locking. It is a bit technical though, and section 2 and later are about the framework rather than the technique

3 Likes
9

Yeah, I ended up using ProfileStore, but I still read your posts and they helped me understand the things u mentionned better. Thank you all for helping me through this! :P.

1 Like