Looking for opinions and possible improvements on my DataModule I've made

Bankrovers · October 3, 2024, 9:58am

Hello! I’ve spent the past few hours creating a ModuleScript for loading/saving player’s data. And I’m currently looking for any improvements!

local DataStoreService: DataStoreService = game:GetService("DataStoreService")

local dataStore: DataStore = DataStoreService:GetDataStore("PlayerData_2")

local dataTemplate = {
	["Boosts"] = {
		["Red"] = 0;
		["Blue"] = 0;
		["Green"] = 0;
		["Pink"] = 0;
		["Purple"] = 0;
		["Golden"] = 0;
	};

	["CharacterPositions"] = {
		["X"] = 0;
		["Y"] = 0;
		["Z"] = 0;
	};

	["Currency"] = {
		["Candy"] = 0;
		["Redeemed"] = 0;
		["Time"] = 0;
	};

	["GamePasses"] = {
		["X2Time"] = false;
		["X5Time"] = false;
		["X10Time"] = false;
		["Vip"] = false;
	};

	["Misc"] = {
		["Halloween2024"] = false;
		["TransferredData"] = false;
		["ReceivedVipPerks"] = false;
	};

	["CodesRedeemed"] = {};

	["UGCsPurchased"] = {};
}

local dataModule = {}

function dataModule.loadData(player: Player)
	local playerData = dataModule.getData(player)

	local function loadBoosts()
		for boostName: string, boostTime: number in playerData.Boosts do
			player:SetAttribute(boostName.."Boost", boostTime)
		end
	end

	local function loadLastPosition()
		local posX: number = playerData.CharacterPositions.X
		local posY: number = playerData.CharacterPositions.Y
		local posZ: number = playerData.CharacterPositions.Z

		player:SetAttribute("LastPosition", Vector3.new(posX, posY, posZ))
	end

	local function loadCurrency()
		for currencyName: string, currencyAmount: number in playerData.Currency do
			player:SetAttribute(currencyName, currencyAmount)
		end
	end

	local function loadGamePasses()
		for gamePassName: string, hasGamePass: boolean in playerData.GamePasses do
			player:SetAttribute("Owns"..gamePassName, hasGamePass)
		end
	end

	local function loadMisc()
		for miscName: string, miscValue: boolean in playerData.Misc do
			player:SetAttribute(miscName, miscValue)
		end
	end

	local loadSuccess, errorMessage = pcall(function()
		loadBoosts()
		loadLastPosition()
		loadCurrency()
		loadGamePasses()
		loadMisc()
	end)

	if not loadSuccess then
		warn("Error while loading player data: "..errorMessage)
		player:Kick("Error while loading your data. Please rejoin.")
	end
end

function dataModule.getData(player: Player)
	local playerData: any

	local getSuccess, errorMessage = pcall(function()
		playerData = dataStore:GetAsync("Player_"..player.UserId)
	end)

	if not playerData then
		playerData = dataTemplate
	end

	for dataType, _ in dataTemplate do
		if not playerData[dataType] then
			playerData[dataType] = dataTemplate[dataType]
			warn("Added DataType: "..dataType)
		end
		
		for dataName, dataValue in dataTemplate[dataType] do
			if playerData[dataType][dataName] == nil then
				playerData[dataType][dataName] = dataValue
				warn("Added DataName: "..dataName)
			end
		end
	end

	if getSuccess then
		print(playerData)
		return playerData
	else
		warn("Error while getting player data: "..errorMessage)
		player:Kick("Error while getting your data. Please rejoin.")
	end
end

function dataModule.saveData(player: Player)
	local function createPlayerData()
		local playerData = table.clone(dataTemplate)
		
		playerData["Boosts"] = {
			["Red"] = player:GetAttribute("RedBoost");
			["Blue"] = player:GetAttribute("BlueBoost");
			["Green"] = player:GetAttribute("GreenBoost");
			["Pink"] = player:GetAttribute("PinkBoost");
			["Purple"] = player:GetAttribute("PurpleBoost");
			["Golden"] = player:GetAttribute("GoldenBoost");
		};

		playerData["CharacterPositions"] = {
			["X"] = if player:GetAttribute("LastPosition") then player:GetAttribute("LastPosition").X else nil;
			["Y"] = if player:GetAttribute("LastPosition") then player:GetAttribute("LastPosition").Y else nil;
			["Z"] = if player:GetAttribute("LastPosition") then player:GetAttribute("LastPosition").Z else nil;
		};

		playerData["Currency"] = {
			["Candy"] = player:GetAttribute("Candy");
			["Redeemed"] = player:GetAttribute("Redeemed");
			["Time"] = player:GetAttribute("Time");
		};

		playerData["GamePasses"] = {
			["X2Time"] = player:GetAttribute("OwnsX2Time");
			["X5Time"] = player:GetAttribute("OwnsX5Time");
			["X10Time"] = player:GetAttribute("OwnsX10Time");
			["Vip"] = player:GetAttribute("OwnsVip");
		};

		playerData["Misc"] = {
			["Halloween2024"] = player:GetAttribute("Halloween2024");
			["TransferredData"] = player:GetAttribute("TransferredData");
			["ReceivedVipPerks"] = player:GetAttribute("ReceivedVipPerks");
		};

		playerData["CodesRedeemed"] = {};

		playerData["UGCsPurchased"] = {};
		
		return playerData
	end
	
	local playerData = createPlayerData()
	
	local function doesDataExist()
		for dataType, _ in playerData do
			for dataName, dataValue in dataTemplate[dataType] do
				if playerData[dataType][dataName] == nil then
					return false
				end
			end
		end
		
		return true
	end
	
	if not doesDataExist() then
		warn("Player data does not fully exist. Not saving data for player: "..player.Name)
		return
	end
	
	local saveSuccess, errorMessage = pcall(function()
		dataStore:SetAsync("Player_"..player.UserId, playerData)
	end)
	
	if saveSuccess then
		print("Successfully saved data for player: "..player.Name)
	else
		warn("Error while saving player data: "..errorMessage)
	end
end

return dataModule

LiamKyd · October 3, 2024, 11:08pm

Addressing the main issues

It’s naive (it assumes the DataStore API is perfect and overlook all the statements below).
It lacks session locking.
It rely on inefficient ways to cache and save data.
It uses Attributes to replicate data.
It uses GetAsync & SetAsync to retrieve/save data over UpdateAsync.
It doesn’t have migrations neither assertions (last is optional, very annoying).
It doesn’t do much to prevent data corruptions.
It doesn’t have rollbacks or snapshots mechanics to revert to previous stored data (I think Roblox added a feature like this recently).
It doesn’t support Individual management of profiles (You can’t wipe data to free memory. There is no interface to modify existing data (to handle player requests who have lost certain data and have proof - Optional)

You have to catch keys that are oddly similar to each other, this is usually done by checking if the current entry that you want to change exists before giving it a value.

Failing to detect typos can lead to more data consumption.

UpdateAsync

Today standards for DataStore is using only UpdateAsync for data retrieval and saving. Why? UpdateAsync gives the most up-to-date data regardless of session and is the best for session locking. It’s also very flexible, it allows you to manipulate data before saving, and it doesn’t overwrite if you return nil. SetAsync would completely overwrite your data with an empty table if the attributes failed to set by any chance.

You also save budget for the other calls (if you run out of budget, you can rely on the other calls, that are pent up).

Budget

Your module also lacks checks to make sure there is enough budget for each Get & Set call. But this is normal because you don’t have retry logic, meaning there is very little per-key-budget being used.

Replication

Attributes will update for every single player regardless if their meant for them or not. You want to use FireClient here, to save as much bandwidth as possible, in conjunction with buffers, if updates are very frequent for whatever reasons.

Some replication libraries like ReplicaService uses FireAllClients because there are cases when the client also need access to other clients data. (e.g. display information for widgets). But, you can change the replication behavior, to either individual or shared, because some smaller projects, maybe don’t need to access other clients data.

You can’t use buffers with attributes, but you can use the value of the attributes with buffers. However, that would increase complexity and reduce performance, because attributes are quite expensive to use.

BindToClose

Your data store also lacks a BindToClose implementation, meaning that if you shutdown all servers, everyone would lose their data.

Resume

retry logic for the DataStore requests.
session locking.
reserve get & set for niche behavior, rely mostly on UpdateAsync.
use remote events to replicate data over attributes and valuebase objects.
improve retrieval and saving algorithms
migrations and assertion (last is optional, very annoying)
better data replication with remotes (consider using a CRUD approach (you can google this))
need to be a lot more robust
implement unit tests to make sure data is saving correctly in many scenarios
implement mocks to be able to test your data store correctly.
needs to be more modular. It’s not up to programming standards.
Your Data Store lacks a game:BindToClose() implementation.

Data-Stores are delicate and require careful consideration to avoid issues that could affect the entire game

You can ask Chat GPT to give you a list of ‘safe precautions cases’ for you to implement in your data store or use ProfileService.

Take a read to the DataStore documentation as well, but unfortunately, they fail to explain or teach the most important things addressed here…

EDIT: polished it for future references.

Bankrovers · October 4, 2024, 8:04am

Thanks for the explaination. I have replaced my SetASync function with this:

	local saveSuccess, errorMessage = pcall(function()
		dataStore:UpdateAsync("Player_"..player.UserId, function(pastData)
			local currentDataVersion: number = player:GetAttribute("DataVersion")
			local pastDataVersion: number = pastData.Misc.DataVersion

			if currentDataVersion ~= pastDataVersion then
				return
			end
			
			playerData.Misc.DataVersion += 1
			return playerData
		end)
	end)
	
	if saveSuccess then
		if not isAutoSave then
			print("Successfully saved data for player: "..player.Name)
		end
		
		return true
	else
		warn("Error while saving player data: "..errorMessage)
	end

I now use UpdateASync for reading data:

	local playerData: any

	local getSuccess, errorMessage = pcall(function()
		playerData = dataStore:UpdateAsync("Player_"..player.UserId, function(pastData)
			if not pastData then
				return dataTemplate
			else
				return pastData
			end
		end)
	end)

I have also added a BindToClose function (I forgot to do this)

game:BindToClose(function()
	if RunService:IsStudio() then return end
	
	for _, player: Player in Players:GetPlayers() do
		dataModule.saveData(player)
	end
	
	task.wait(10)
end)

Please correct me if I used UpdateASync wrong. I’m still learning.