Thoughts on this this auto-sword saving datastore?

Help and Feedback Code Review
#1

Hey! So I made a saving system for swords that player own. Meaning whenever I add a new sword, I don’t have to put it in a table of data, it will automatically do that with a For Loop. However while this works, I have a feeling it is a bad practice, since it relies on using Indexes to find the boolean value of an owned sword. I was trying to have ‘table.insert’ insert a dictionary but it didn’t seem to work.

CODE:

local SwordDataStore = game:GetService("DataStoreService"):GetDataStore("SwordDataStores")

game.Players.PlayerAdded:Connect(function(player)
	
	local folder = Instance.new("Folder")
	folder.Name = "OwnedSwords"
	folder.Parent = player
	
	
	for _, sword in ipairs(game.ServerStorage.Swords:GetChildren()) do
		
		local Ownedsword = Instance.new("BoolValue")
		Ownedsword.Name = sword.Name
		Ownedsword.Parent = folder
		
	end
	
	local SwordData = player:WaitForChild("OwnedSwords"):GetChildren()
	local data
	local success, errorMessage = pcall(function()
		data = SwordDataStore:GetAsync(player.UserId.."_OwnedSword")
	end)

	if success then
		for i, SwordLoad in ipairs(SwordData) do
			SwordLoad.Value = data[i]
		end
	else
		warn(errorMessage)
	end
end)


game.Players.PlayerRemoving:Connect(function(player)

	local SwordData1 = player:WaitForChild("OwnedSwords"):GetChildren()
	local data = {}

	for i, SwordBool in ipairs(SwordData1) do
		table.insert(data, i, SwordBool.Value)
	end
		print(data)
	local success, errorMessage = pcall(function()
		SwordDataStore:SetAsync(player.UserId.."_OwnedSword", data)
	end)

	if success then
		print("Data Saved")
	else
		warn(errorMessage)
	end
	
end)
2 Likes
#2

Pretty basic. Read through my data saving script for an idea.

local DSS = game:GetService("DataStoreService")
local DS = DSS:GetDataStore("ArksieData")

local LOADED_PLAYERS = {}

local function GenerateDataKey(Player)
	local Key = "UID_" .. Player.UserId
	return Key
end 

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

	local bits = Instance.new("IntValue")
	bits.Name = "Bits"
	bits.Value = 10 --starter amt
	bits.Parent = ls

	local key = GenerateDataKey(player)
	local data = nil
	local Id = 1
	
	local success,err = pcall(function()
		data = DS:GetAsync(key)
		Id = DS:GetAsync("Id")
		data = data or {Bits = bits.Value}
	end)
	if not success then
		player:Kick("Couldn't fetch data!\n\nError:\n"..err)
	end

	if data then
		print("\nData for "..player.Name..":\nBits: "..data.Bits.."\nUserId: "..data.UserId.."\n\nNext Id: "..Id)
		bits.Value = data.Bits
	else
		print('no data')
	end
	LOADED_PLAYERS[player] = true
end)

game.Players.PlayerRemoving:Connect(function(player)
	local Id = player.leaderstats.UserId.Value
	
	if not LOADED_PLAYERS[player] then return end;
	LOADED_PLAYERS[player] = nil

	local key = GenerateDataKey(player)
	local data = {
		Bits = player.leaderstats.Bits.Value,
	}
	local success, err = pcall(function()
		DS:SetAsync(key, data)
	end)
	if not success then
		warn(err)
	end
end)

game:BindToClose(function()
	local leftToSave = 0
	for player in pairs(LOADED_PLAYERS) do
		LOADED_PLAYERS[player] = nil
		leftToSave += 1

		coroutine.wrap(function()
			local data = {
				Bits = player.leaderstats.Bits.Value,
				UserId = player.leaderstats.UserId.Value,
			}

			local success, errorMessage = pcall(function()
				DS:SetAsync(GenerateDataKey(player), data)
			end)
			leftToSave -= 1
		end)()
	end
	local RunService = game:GetService("RunService")
	while leftToSave > 0 do
		RunService.Heartbeat:Wait()
	end
end)
1 Like
#3

UpdateAsync could be used here

2 Likes
#4

Hi!

You should indeed not rely on GetChildren() to always get you the swords in the same order. Probably it’s better to give each sword a unique name, and as you say, use those names in a dictionary instead of a list.

One first thing about how you used table.insert here:

for i, SwordBool in ipairs(SwordData1) do
	table.insert(data, i, SwordBool.Value)
end

Notice that you could just have done the following, you didn’t need to pass the index:

for i, SwordBool in ipairs(SwordData1) do
	table.insert(data, SwordBool.Value)
end

You could also rewrite the same as follows:

for i, SwordBool in ipairs(SwordData1) do
	data[i] = SwordBool.Value
end

These would all give the exact same result.

The main difference between lists and dictionaries, is that a list is a table with consecutive integer indexes starting at 1, while a dictionary uses strings (or other objects… but the convention is to never use non-consecutive integers as indexes).

You make good use of ipairs. The ipairs function just starts at index one and stops when the next integer index number is not found. So it would miss items if there were missing integers.

On the other hand, you can use the pairs() function, but then the order that you get the key-value pairs is not guarantueed. But this normally is not an issue, it is as you say bad practice to rely on the order too much.

So you could do:

for i, SwordBool in pairs(SwordData1) do
	data[SwordBool.Name] = SwordBool.Value
end

Now when you get your data back from datastores you can check if data[SwordLoad.Name] to know if it was unlocked.

Now that you know more about lists and dictionaries, you may want to rewrite more of the program, but this will do. Just remember to never use ipairs on a dictionary, and not use pairs on a list if the order is important (but try to not let the order be important). :slight_smile:

1 Like
#5

Tysm! And also yea i forgot the face that ipairs doesnt work with dictionaries!

1 Like
#6

wait so how would i go about loading the player data though. I tried this and it doesnt seem to be working.

image

#7

Indeed if you look closely at my example you will notice something else in the data[SwordLoad] segment

1 Like
#8

ohh it worked now! Thanks a lot!

1 Like