Data Store not very good

Someone told me that my data store script was very bad, I dont really understand what they are trying to tell me because I am very new to scripting. Is there anyone that can explain it to me in idiot language?

Script:

--//Services
local Players = game:GetService("Players")
local DataStoreService = game:GetService("DataStoreService")
local Module = require(script.ModuleScript)
--//Variables
local dataStore = DataStoreService:GetDataStore("MyDataStore")

--//Functions
local function saveData(player)
	local tableToSave = {
		
		Coins = player.leaderstats.Coins.Value,
		
		OwnedGH = player.GoldenHook.Owned.Value,
		EquippedGH = player.GoldenHook.Equipped.Value,

		OwnedTP = player.TPose.Owned.Value,
		EquippedTP = player.TPose.Equipped.Value,
		
		OwnedUS = player.UpsideDown.Owned.Value,
		EquippedUS = player.UpsideDown.Equipped.Value,
		
		OwnedWH = player.WoodenHook.Owned.Value,
		EquippedWH = player.WoodenHook.Equipped.Value,
		
		EquippedIH = player.InvisibleHook.Equipped.Value,
		OwnedIH = player.InvisibleHook.Owned.Value
		
	}

	local success, errorMessage = pcall(function()
		dataStore:SetAsync(player.UserId, tableToSave)
	end)

	if success then -- If the data has been saved
		print("Data has been saved!")
	else -- Else if the save failed
		print("Data hasn't been saved!")
		warn(errorMessage)
	end
end

Players.PlayerAdded:Connect(function(player)
	local leaderstats = Instance.new("Folder")
	leaderstats.Name = "leaderstats"
	leaderstats.Parent = player

	--//Coins//--	
	local Coins = Instance.new("IntValue")
	Coins.Name = "Coins"
	Coins.Parent = leaderstats
	
	--//InvisibleHook//--
	local InvisibleHook = Instance.new("Folder")
	InvisibleHook.Name = "InvisibleHook"
	InvisibleHook.Parent = player
	
	local OwnedIH = Instance.new("BoolValue")
	OwnedIH.Name = "Owned"
	OwnedIH.Parent = InvisibleHook
	
	local EquippedIH = Instance.new("BoolValue")
	EquippedIH.Name = "Equipped"
	EquippedIH.Parent = InvisibleHook
	--//WoodenHook//--
	local WoodenHook = Instance.new("Folder")
	WoodenHook.Name = "WoodenHook"
	WoodenHook.Parent = player
	
	local OwnedWH = Instance.new("BoolValue")
	OwnedWH.Name = "Owned"
	OwnedWH.Parent = WoodenHook
	
	local EquippedWH = Instance.new("BoolValue")
	EquippedWH.Name = "Equipped"
	EquippedWH.Parent = WoodenHook
	
	--//UpsideDown//--
	local UpsideDown = Instance.new("Folder")
	UpsideDown.Name = "UpsideDown"
	UpsideDown.Parent = player
	
	local OwnedUS = Instance.new("BoolValue")
	OwnedUS.Name = "Owned"
	OwnedUS.Parent = UpsideDown
	
	local EquippedUS = Instance.new("BoolValue")
	EquippedUS.Name = "Equipped"
	EquippedUS.Parent = UpsideDown
	
	--//Golden Hook//--
	local GoldenHook = Instance.new("Folder")
	GoldenHook.Name = "GoldenHook"
	GoldenHook.Parent = player

	local OwnedGH = Instance.new("BoolValue")
	OwnedGH.Name = "Owned"
	OwnedGH.Parent = GoldenHook
	
	local EquippedGH = Instance.new("BoolValue")
	EquippedGH.Name = "Equipped"
	EquippedGH.Parent = GoldenHook

	--//TPose//--
	local TPose = Instance.new("Folder")
	TPose.Name = "TPose"
	TPose.Parent = player

	local OwnedTP = Instance.new("BoolValue")
	OwnedTP.Name = "Owned"
	OwnedTP.Parent = TPose
	
	local EquippedTP = Instance.new("BoolValue")
	EquippedTP.Name = "Equipped"
	EquippedTP.Parent = TPose
	--//Other//--
	local Folder4 = Instance.new("Folder")
	Folder4.Name = "NumberOfHooks"
	Folder4.Parent = player

	local IntValue2 = Instance.new("IntValue")
	IntValue2.Name = "Amount"
	IntValue2.Parent = Folder4

	local data = nil

	local success, errorMessage = pcall(function()
		data = dataStore:GetAsync(player.UserId) 
	end)

	if success and data then 
		Coins.Value = data.Coins
		OwnedGH.Value = data.OwnedGH
		OwnedTP.Value = data.OwnedTP
		EquippedTP.Value = data.EquippedTP
		EquippedGH.Value = data.EquippedGH
		EquippedUS.Value = data.EquippedUS
		OwnedUS.Value = data.OwnedUS
		EquippedIH.Value = data.EquippedIH
		OwnedIH.Value = data.OwnedIH
		EquippedWH.Value = data.EquippedWH
		OwnedWH.Value = data.OwnedWH
	else 
		print("The player has no data!") -- The default will be set to 0
	end
end)

Players.PlayerRemoving:Connect(saveData)

game:BindToClose(function() -- When the server shuts down
	for _, player in Players:GetPlayers() do -- Loop through all the players
		saveData(player)
	end
end)

Module script inside other script (I was told to make this)

local module = {}

module.PlayerData = {

}

return module

There is nothing wrong with using vanilla datastore, but I would strongly advise taking a look at tools built on top of DataStoreService for handling data safely and more robustly:
Please see: DataStore2 or ProfileService

Other tips here would be to use UpdateAsync vs SetAsync, as SetAsync can overwrite the previously cached data and cause data loss.

I also think you could be more descriptive with your variable naming conventions, I am unclear what you are trying to abbreviate within tableToSave.

If you have any questions please let me know.

2 Likes

Do not use datastore2, use profile service, or your own profile based datastore.

1 Like

I would personally use ProfileService because of session locking and data loss prevention. Session locking is very important for your game especially if you have a trading system.

ProfileService is the standard.

1 Like

I concur. Please see: How would I start a table trading system? - #2 by Blankscarface23