Are there any flaws with this data store?

If there’s anything I could improve upon or something that could cause my script to break, please let me know.

It’s pretty long so I don’t expect anybody to read the whole thing lol

local dataStore = {}

-- [[ Services ]] --
local DSS = game:GetService("DataStoreService")
local SSS = game:GetService("ServerScriptService")
local RS = game:GetService("RunService")

-- [[ Settings ]] --
local Settings = {
	AutoSaveInterval = 30,
	NotificationsColor = Color3.new(0, 1, 0),
	SaveInStudio = false,
}

function dataStore.new(player, name)
	local MAXINTERVAL = 60
	local MININTERVAL = 10
	local BASEINTERVAL = 30

	if Settings.AutoSaveInterval < MININTERVAL or Settings.AutoSaveInterval > MAXINTERVAL then
		Settings.AutoSaveInterval = BASEINTERVAL
	end

	local indexFormat = string.format("%d:%s", player.UserId, name)
	if dataStore[indexFormat] then
		return dataStore[indexFormat]
	end
	
	local self = {}
	self.name = name;
	self.player = player;
	self.userId = player.UserId;
	self.notifications = false;
	self.__store = DSS:GetDataStore(name);
	self.__lastEntry = -math.huge;
	self.__queue = {};
	self.__updateCallbacks = {};
	self.__sent = 0;

	local meta = {}
	meta.__index = dataStore;

	dataStore[indexFormat] = setmetatable(self, meta)

	coroutine.wrap(dataStore.wrap)(dataStore[indexFormat])

	return dataStore[indexFormat]
end

function dataStore.wrap(self)
	local PlayerId = string.format("Player_%d", self.userId)
	
	local function save()
		if #self.__queue == 0 then
			return
		end
		
		local lastIndex = self.__queue[#self.__queue]

		if typeof(lastIndex) ~= "table" then
			for i = 1, #self.__queue do
				if self.__queue[i] then
					lastIndex = self.__queue[i]
				end
			end
		end

		lastIndex[2] = tick()

		self.__store:SetAsync(PlayerId, lastIndex)
		
		if self.notifications then
			self.__sent = self.__sent + 1
			
			if self.__sent%5 == 0 or self.__sent == 1 then
				local ChatService = require(SSS.ChatServiceRunner.ChatService)

				local Speaker = ChatService:GetSpeaker(self.player.Name)
				local extraData = {ChatColor = Settings.NotificationsColor}
				
				local notification = string.format("Hey %s, your data has been saved!", self.player.Name)
				
				Speaker:SendSystemMessage(notification, "All", extraData)
			end
		end
	end
	
	game:BindToClose(function()
		if RS:IsStudio() and not Settings.SaveInStudio then
			return
		end
		if tick() - self.__lastEntry < Settings.AutoSaveInterval then
			repeat
				RS.Heartbeat:Wait()
			until tick() - self.__lastEntry >= Settings.AutoSaveInterval
		end
		save()
	end)

	while wait(Settings.AutoSaveInterval) do
		self.__lastEntry = tick()

		save()

		for i, v in ipairs(self.__queue) do
			table.remove(self.__queue, i)
		end

		if #self.__queue ~= 0 then
			local _, index = next(self.__queue)
			while index ~= nil do
				self.__queue[index] = nil
				_, index = next(self.__queue)
			end
		end

	end
end

function dataStore.leaderstats(player, stats)
	local leaderstats = player:FindFirstChild("leaderstats") or Instance.new("Folder", player)
	leaderstats.Name = "leaderstats"

	local ls = {}

	for _, stat in pairs(stats) do

		local newStat = Instance.new(stat.StatType .. "Value", leaderstats)
		newStat.Name = stat.Name

		local newStore = dataStore.new(player, stat.Alias or stat.Name)
		newStat.Value = newStore:Get(stat.DefaultValue)

		newStore:OnUpdate(function(value)
			newStat.Value = value
		end)

		table.insert(ls, newStore)
	end

	--[[
		
		{
			Name = "",
			Alias = "",
			DefaultValue = 0,
			StatType = "Int",
		}	
			
	]]

	return unpack(ls)
end

function dataStore:Get(defaultValue)
	local defaultValue = defaultValue or 0
	local PlayerId = string.format("Player_%d", self.userId)

	if #self.__queue > 0 then
		return self.__queue[#self.__queue][1]
	else
		local retrieved, result = pcall(function()
			return self.__store:GetAsync(PlayerId)
		end)
		if retrieved then

			if typeof(result) == "table" then
				if result[2] ~= nil and tick() - result[2] >= Settings.AutoSaveInterval then
					return result[1] or defaultValue
				else
					repeat
						RS.Heartbeat:Wait()
						if #self.__queue > 0 then
							return self.__queue[#self.__queue]
						end
					until tick() - result[2] >= Settings.AutoSaveInterval
					return result[1] or defaultValue
				end
			else
				return defaultValue
			end

		else

			local errorMsg = string.format("Error occurred: %q", result)
			warn(errorMsg)

		end
	end
end

function dataStore:Set(value)
	table.insert(self.__queue, {value, 0})

	for _, callback in pairs(self.__updateCallbacks) do
		callback(value)
	end
end

function dataStore:SetNotificationsEnabled(boolean)
	self.notifications = boolean
end

function dataStore:Increment(amnt, defaultValue)
	local defaultValue = defaultValue or 0
	self:Set(self:Get(defaultValue) + amnt)
end

function dataStore:OnUpdate(callback)
	table.insert(self.__updateCallbacks, callback)
end

return dataStore

seems like a pretty decent script. I’m a bit tired rn, so i’ll just point out a few tips.

  1. Define your services and modules at the top of your script. It doesn’t affect your code in any way. It’s just personal preference tbh.

  2. This seems like an improper use of metatables

	local meta = {}
	meta.__index = dataStore;

	dataStore[indexFormat] = setmetatable(self, meta)

	coroutine.wrap(dataStore.wrap)(dataStore[indexFormat])

	return dataStore[indexFormat]

Idk why you decided to overcomplicate your code. You can just do this instead (the standard way).

local dataStore = {}
dataStore.__index = dataStore

function dataStore.new(player, name)
	-- blah blah blah
	dataStore[indexFormat] = setmetatable({
		_name = name,
		_player = player,
		_userId = player.UserId,
		_notifications = false,
		__store = DSS:GetDataStore(name),
		__lastEntry = -math.huge,
		__queue = {},
		__updateCallbacks,
		__sent = 0,
	}, dataStore)
	
	coroutine.wrap(dataStore.wrap)(dataStore[indexFormat])
	return dataStore[indexFormat]
end
  1. typeof is meant to be used only on luau datatypes. ex. Vector3. though it is compatable with lua datatypes too. type is also faster than typeof when it comes to lua datatypes ex. table, string, number, userdata, boolean

Here you should have used the type function instead of typeof:

if typeof(lastIndex) ~= "table" then
  1. use ipairs here, since you are trying to retrieve the value. Indexing like array[i] is not instantaneous. Thats where the ipairs iterator saves the day! ipairs caches both the index and the value for whatever code that needs to be ran inside the loop
-- Not Good
			for i = 1, #self.__queue do
				if self.__queue[i] then
					lastIndex = self.__queue[i]
				end
			end
-- Good
            for _, value in ipairs(self.__queue) do
                  lastIndex = v  
            end
  1. This is not necessary, the generic iterator will always halt at nil, so there is no point in trying to check if self.__queue[i] exists. Its an unneeded calculation.
  2. When it comes for readability, always try to make your variable names as specific and readable as possible. Especially when working with a team.
-- Not good
local RS
-- Good
local RunService

I like your datastore tho! it seems pretty tidy and organized.

2 Likes

Adding on to what @DarthFS said, the only thing I noticed as I was skimming your code was the use of SetAsync. UpdateAsync is much much more efficient and I just overall recommend you to use it as an alternative to SetAsync. I am pretty sure it helps with the queueing of data stores although I am not entirely sure.

It doesn’t help with reducing queue of data stores. It is useful for session locking, respects incoming update calls and gives you the old data as an argument.

Thank you for the clarification. I wasn’t entirely sure about my claim.

Vector3 is not a Luau specific data type, therefore it isn’t a “Luau” data type.

Thats where the ipairs iterator saves the day! ipairs caches both the index and the value for whatever code that needs to be ran inside the loop

The wording is a bit wrong here. You’re the one caching the key and value inside a variable, not ipairs, it just returns them.

for k, v in ipairs({1, 2, 3}) do --> k and v are variables !
     
end

The standard datatypes you would see in lua are: tables, numbers, booleans, userdata, string, function, nil, and thread which are the only datatypes the ‘type’ function works on (except thread).

By luau datatypes I was referencing datatypes that only work in roblox’s superset of lua (luau). ex. Vector3. There is not Vector3 datatype in lua.

yeah what I meant.

By luau datatypes I was referencing datatypes that only work in roblox’s superset of lua (luau). ex. Vector3. There is not Vector3 datatype in lua.

Luau contains the Vector3 data type, Vector3 is not a Luau specific data type, if it was, it would only be available in Luau which doesn’t make sense.

I do not understand your point. We are talking about roblox studio specifically, not other game engines (I am aware that vectors exist in almost every game engine where other lower level programming languages are used like C). I was originally comparing how roblox has other datatypes compared to the usual datatypes you see in lua: tables, numbers, booleans, userdata, string, function, nil, and thread.

My point was when replying to @Q_ubit is that the ‘type’ function should be used in specific situations over the ‘typeof’ function. The ‘typeof’ function does work with lua datatypes but is slower compared to when using ‘type’.

I do not understand your point. We are talking about roblox studio specifically, not other game engines (I am aware that vectors exist in almost every game engine and other lower level programming languages like C). I was originally comparing how roblox has other datatypes compared to the usual datatypes you see in lua: tables, numbers, booleans, userdata, string, function, nil, and thread.

You said that it was Luau “specific” which was the wrong wording to use.

It’s a luau specific because Vector3s don’t exist in lua unlike luau.

Saying “Luau specific” usually means “Only in Luau” which is incorrect.

Like I said…, I was comparing lua to luau. I’m talking about roblox studio. Not other game engines like Unity.