Any advice or additional content for DataStore Functions

minimic2002 · March 5, 2021, 3:42pm

I have been working on my own set of DataStore functions for anything I might need.
At the moment it can:

Save Models (Via my own Module)
Load Models (Via my own Module)
Save unlimited data that can be accessed similarly to an array (Within some limits, This is also used to create backups)
Basic Stuff (Save, Load, Update, Array Save/Load)

(Edit: Added a limit to number of attempts on my studio version)
.

My DSS Functions

local DataStoreService = game:GetService("DataStoreService")

----------------------------------------- // Base Functions (Save, Load, Update, Remove, CheckForData)

function SetData(Store,Player,Data)
	while true do
		local S, R = pcall(function()
			local NewStore = DataStoreService:GetDataStore(Store)
			NewStore:SetAsync(Player,Data)
		end)
		if S then
			print("Set Data")
			break
		else
			print("DSS Error:")
			print(R)
			wait(2)
		end
	end
	
end

function UpData(Store,Player,Data)
	while true do
		local S, R = pcall(function()
			local NewStore = DataStoreService:GetDataStore(Store)
			NewStore:UpdateAsync(Player,Data)
		end)
		if S then
			break
		else
			print("DSS Error:")
			print(R)
			wait(2)
		end
	end
	
end

function GetData(Store,Player)
	local Result
	while true do
		local S, R = pcall(function()
			local NewStore = DataStoreService:GetDataStore(Store)
			Result = NewStore:GetAsync(Player)
			
		end)
		if S then
			return Result
		else
			print("DSS Error:")
			print(R)
			wait(2)
		end
	end
	
end

function RemoveData(Store,Player)
	while true do
		local S, R = pcall(function()
			local NewStore = DataStoreService:GetDataStore(Store)
			NewStore:RemoveAsync(Player)
		end)
		if S then
			print("Removed Data")
			break
		else
			print("DSS Error:")
			print(R)
			wait(2)
		end
	end
end

function CheckForExistingData(Store,Player)
	local Data
	for i = 1,#3 do
		local S, R = pcall(function()
			Data = GetData(Store,Player)
			if Data == nil then
				local Result = false
				return Result
			else
				
			end
		end)
		
		wait(1)
		if S then
			break
		end
	end
	if Data == nil then
		local Result = true
		return Result
	end
end
----------------------------------------- // ArraySaving, ArrayLoading
function SaveArray(Store,Player,Data)
	local Combined = table.concat(Data,";Split;")
	SetData(Store,Player,Data)
end

function LoadArray(Store,Player)
	return string.split(GetData(Store,Player),";Split;")
end

----------------------------------------- // DataStore Listing (For BackUps or Databases)
function CreateListing(Store,Data)
	while true do
		local S, R = pcall(function()
			local EntryCount = GetData(Store .. "EntryCount","EntryCount")
			if tonumber(EntryCount) == nil then
				EntryCount = 0
			end
			EntryCount = tostring(tonumber(EntryCount) + 1)
			SetData(Store,EntryCount,Data)
			SetData(Store .. "EntryCount","EntryCount",EntryCount)
			
		end)
		if S then
			break
		else
			print("DSS Error:")
			print(R)
			wait(2)
		end
	end
	
end

function GetListings(Store)
	local StoreNames
	while true do
		local S, R = pcall(function()
			local EntryCount = GetData(Store .. "EntryCount","EntryCount")
			EntryCount = tonumber(EntryCount)
			StoreNames = {}
			for i = 1, EntryCount do
				local Name = tostring(i)
				local Check = CheckForExistingData(Store,tostring(i))
				if Check == true then
					table.insert(StoreNames,Name)
				end
			end
		end)
		if S then
			return StoreNames
		else
			print("DSS Error:")
			print(R)
			wait(2)
		end
	end
	
	
end

function RemoveListing(Store,Number)
	SetData(Store,Number,nil)
end

function GetEntryCount(Store)
	local EntryCount = GetData(Store .. "EntryCount","EntryCount")
	EntryCount = tonumber(EntryCount)
	return EntryCount
end


----------------------------------------- // Save Or Load Models (Via My Modules)
local PartSavingMod = require(script.PartSavingModule)
function SaveModel(Model)
	return PartSavingMod.GetModelProperties(Model)
end

local PartLoadMod = require(script.PartLoadingModule)
function LoadModel(Model)
	return PartLoadMod.LoadModel(Model)
end

angrybino · March 24, 2021, 8:59am

Firstly, you should always retry if a pcall fails, you’re calling SetAsync which yields the thread and is prone to errors. I recommend not creating a new function just to call an asynchronous function, but the proper way pcall(function, self ?, args).

local MAX_TRIES = 3

while true do 
    local tries = 0
    local NewStore = DataStoreService:GetDataStore(Store)

	local success, response = pcall(NewStore.SetAsync, NewStore, Player, Data) 
	if success then
		print("Set Data")
	else
	    tries += 1
 
        if tries == MAX_TRIES then
              warn(response)
        end
	end
end

There is also no point in having 2 functions which just do the same thing; UpData and SetData, the only difference is that one uses UpdateAsync while the other uses SetAsync. UpData isn’t using the features that UpdateAsync provides so it is rendered useless.

Instead, what you should do is check if the old data and the new data are same and return nil as it cancels the save effectively.

local MAX_TRIES = 3

local function IsOldData(oldData, data)
	for key, value in pairs(data) do
		if value ~= oldData[key] then
			return false
		end
	end
	
	return true
end

function UpData(Store ,Player, Data)
	local tries = 0
	
	while true do
		local success, response = pcall(Store.UpdateAsync, Store, Player, function(oldData)
			if IsOldData(oldData, Data) then
				return
			end
			
			return Data
		end)
		
		if success then
			break
		else
			tries += 1
			
			if tries == 3 then
				warn(response)
			end
		end
	end
end

I noticed that you’re using global functions instead of local, though the performance difference is negligible, it is good practice to use local functions over globals.

Your functions have the parameter Player and it seems you are taking that as the key, if Player is not the user Id, consider passing it as the user id as the player can just change their user and end up with a new data.

CheckForExistingData uses extraneous variables just to return a value, which is definitely not ideal and redundant.

local t = false
return t -- bad

return false -- good

minimic2002 · March 25, 2021, 1:50pm

My script does handle failed attempts. I was using a while loop to do this but as that will keep trying until it works I changed it to max out at 10 attempts.

As for the SetAsync() and UpdateAsync() it is commom knowlage that the two save data in a different way, the reasoning for having both is that UpdateAsync() will not cause thr value to become nil should there be a major issue (Couod be bad server connection or planned server outages).

As for check for existing value, you are correct that this is an unnessisary use of datastores. Its only use is for when checking data with my DSS Listing functions which can save masses of data. Although it seems it is not needed at the time it plays a major role in the functionality of making infinite datastores, the only thing is that I could make it also return either the data or false.

Please make sure you understand code before making comments on it. The only valid recommendation you had was on my checker which I thank you for and will take into consideration when I remodel the functions.

Planned changes:

checker to return value and length of data
DSS Listing Functions to inherit store translations
Proposed expansion to automatically change over stores when exceeding 200k character count.
Proposed DSS encrypted Cross-server communication to allow for immediate shutdown of servers under certain situations.

angrybino · March 25, 2021, 2:05pm

My script does handle failed attempts. I was using a while loop to do this but as that will keep trying until it works I changed it to max out at 10 attempts.

Your code just keeps retrying unless the pcall is successful WHICH is definitely not ideal. If you’ve made that change, then at least try to edit the changes you made so that the reviewer doesn’t misunderstand what the code is intended to do.

Despite that, your code isn’t respecting the limits of writing data to the same key, 2 seconds isn’t sufficient and will throttle the data stores significantly.

Also, 10 tries is too much; consider changing it to 5 or 3.

As for the SetAsync() and UpdateAsync() it is commom knowlage that the two save data in a different way, the reasoning for having both is that UpdateAsync() will not cause thr value to become nil should there be a major issue (Couod be bad server connection or planned server outages).

This is incorrect; UpdateAsync isn’t guaranteed to give you the argument for data as a truthy value, especially if data for that key was never previously saved. In the case of a bad server connection or server outages, UpdateAsync won’t even be guaranteed to function at all and “it will never cause the value to become nil” is non sense, if it succeeds, it will call the callback function passing in a argument of the key’s data, else it will return an error.

UpdateAsync has some functionality that allows you to carefully save data, but your code is using none of that functionality.

I suggest you read more about DataStore limits, since your points regarding them are completely invalid: Data Stores

minimic2002 · March 25, 2021, 2:07pm

Perhaps you could give me your source of how to ‘correctly’ use the UpdateAsync(). As far as I know it doesnt need any extra parameters to fufil its purpose.

Also about my retrys, it breaks once it succeeds. The reason I set it to 10 is incase of slow server times and for saving large quanties of data. I will have to consider changing it based on the number of charaters being saved so high quantity saving doesnt become a issue later on.

I am more than aware of the limitations of DSS which is what most of my script is based on. Hence why umlimited data saving and backups was one of my scripts most powerful aspects.

angrybino · March 25, 2021, 2:12pm

Your code should only use UpdateAsync to safely save the data when needed. It gives you the data and you should check if the data that you’re trying to save isn’t the same as the given data, if it is the same, return nil as it will cancel the save, else save it.

If the pcall fails, yield the thread for 6 or 7 seconds and then retry again.

local function IsSame(t1, t2)
     for key, value in pairs(t1) do
         if t2[key] ~= value then
              return false
         end
     end

     return true
end


function UpData(Store ,Player, Data)
	local tries = 0
	
	while true do
		local success, response = pcall(Store.UpdateAsync, Store, Player, function(loadedData)
			if IsSame(oldData, loadedData) then
				return -- cancel the save
			end
			
			return Data -- save data
		end)
		
		if success then
			break
		else
			tries += 1
			
			if tries == 3 then
				warn(response)
			end

           wait(6) -- yield to respect the cooldown
		end
	end
end

Another main reason you should be using UpdateAsync is explained by this:

Also about my retrys, it breaks once it succeeds. The reason I set it to 10 is incase of slow server times and for saving large quanties of data. I will have to consider changing it based on the number of charaters being saved so high quantity saving doesnt become a issue later on.

No, data store API calls aren’t reliable and there is a chance that one will never even succeed if Roblox servers are experiencing a severe bad connection or a terrible downage. Hence you should retry 5 times, as it is more than enough.

Saving large quantities of data isn’t the problem, make sure that the characters don’t exceed 4,000,000 characters.

I am more than aware of the limitations of DSS which is what most of my script is based on. Hence why umlimited data saving and backups was one of my scripts most powerful aspects.

What? Your code doesn’t respect the cooldown for writing for each key.