Problems with checking an item with a data store

NinjoOnline · December 16, 2018, 12:31pm

local classDataStore = dataStoreService:GetDataStore('ClassDataStore', 'Test')

local remotes = replicatedStorage:WaitForChild('Remotes')

local functions = remotes:WaitForChild('Functions')
local equipClass = functions:WaitForChild('EquipClass')

local classData = {}

function equipClass.OnServerInvoke(player, class)
	for i, v in pairs(classData) do
		if v[1] == player.UserId then
			for _, table in pairs(v[2]) do
				if table == class then
					return true
				else
					return false
				end
			end
		end
	end
end

function dataManager:GetData(player)
	local playerClasses = {player.UserId, {'Knight', 'Archer'}}
	local loadedClasses = classDataStore:GetAsync(player.UserId .. '-classes')

	if loadedClasses ~= nil then
		playerClasses[2] = loadedClasses
	end

	table.insert(classData, playerClasses)
end

Basics of this script. Setups up the players classes when they join, and the function at the top (equipClass) basically fires whenever a player tries to equip a class. If they have the class saved then it returns true, otherwise it returns false.

On this line:

local playerClasses = {player.UserId, {'Knight', 'Archer'}}

It sets what classes a player owns. So every player owns those two classes, for free. But when I try the equip function, it only returns true for the Knight class and not Archer. Have I entered something wrong on the above line?? Not entirely sure if this is the most efficient way to go about saving multiple items in a data store either (as there will eventually be 10+ classes) so if there is a better way then any info would be great

C_Sharper · December 16, 2018, 6:32pm

There’s a good chance I’m wrong on this, but perhaps it’s because you’re using a variable with the name “table” in your for loop? The reason why I’m suggesting this could be the reason why is because table is actually a class/type of it’s own, and perhaps it’s overriding your data with the table data type.

Pretty sure this isn’t the case, but I mean, I can’t detect anything else that could be causing this to work incorrectly.

EmeraldSlash · December 16, 2018, 6:44pm

I don’t know immediately what is causing your issue, but it is probably time related (e.g. checking the Archer class before the player’s data has loaded). I recommend you fix the things I’ve pointed out below, and then if you still have your issue, let us know and show us the code you are using to test it.

Not important, just a simple optimisation

Saving classes in an array is fine; however if you do want to save on space later on when players end up holding lots of stuff (potentially enough for going over DataStore limit?) you could replace the string IDs with numbers (using an enum-like system):

local CLASSES = {KNIGHT = 1, ARCHER = 2, MAGE = 3, THIEF = 4}

local playerClasses = {player.UserId, {CLASSES.KNIGHT, CLASSES.ARCHER}}

--> {000000, {1, 2, 3, 4}} instead of {000000, {'Knight', 'Archer', 'Mage', 'Thief'}}

for _, table in pairs(v[2]) do
    -- code
end

A general bad practice I see here is that you are overriding the built-in table library. If you needed to use this library within the for loop, you’d need to save it a variable before the for loop, or choose a different variable name. The name also becomes highlighted, which would be misleading as highlighted words represent built-in Lua stuff. I changing the variable name – and in addition to this, table isn’t an accurate name for a value which actually represents a class, which is another reason why you want to change it.

local playerData = {player.UserId, {}}

There’s no need to save the player’s UserId in here. It’s just wasting space. Either way, you can’t get this data if you don’t have the player’s DataStore key (which should be unique to the player and therefore require the player object and the data it holds to retrieve). It’s redundant.

local classData = {}

This variable would likely be better off as a dictionary than as an array. It means you can quickly and easily retrieve data for a specific player, and I’m pretty sure the related data is removed when a player leaves automatically because their player object becomes nil (therefore fixing a memory leak).

Here’s a comparison:

local classData = {[EmeraldSlash] = {}, [NinjoOnline] = {}}
-- Cleaned up automatically
local playerData = classData[player]
-- Easy and quick to access

-- OR

local classData = {{}, {}}
-- Cleaned up manually
local playerData
for _, v in pairs(classData) do
    if v[1] == player.UserId do
        playerData = v
        break
    end
end
-- Takes a relatively long time to access in both performance and writing

The great thing about dictionaries is that you can use Instances as keys. That’s what I’m doing here. If you were adding a player to the dictionary, you’d do classData[game.Players.PlayerName] = playerData/

NinjoOnline · December 17, 2018, 1:38am

I was told to always use player.UserId for data stores though? What if a player was to change their name??

NinjoOnline · December 17, 2018, 1:55am

Also trying to change to a number,

function equipClass.OnServerInvoke(player, class)
	for i, v in pairs(classData) do
		if v[1] == player.UserId then
			for _, k in pairs(v[2]) do
				local classID = classStats.Stats[class].ID
				if k == class then
					return true
				else
					return false
				end
			end
		end
	end
end
local playerClasses = {player.UserId, {1, 2}}
local loadedClasses = classDataStore:GetAsync(player.UserId .. '-classes')

if loadedClasses ~= nil then
	playerClasses[2] = loadedClasses
end

table.insert(classData, playerClasses)

Module

classStats.Stats = {
	['Knight'] = {
		['ID'] = 1,
		['Health'] = 125,
		['Speed'] = 14,
		['Damage'] = 10,
	},
	['Archer'] = {
		['ID'] = 2,
		['Health'] = 100,
		['Speed'] = 18,
		['Damage'] = 8,
	}
}

Still doesn’t work, not sure what I’m doing atm. Changed the table to k. Printing (k, class) prints 1 Knight when clicking knight, prints 1 Archer when clicking archer. So it never prints 2. Not sure how to reference the ID’s from within the module either. I did this local classID = classStats.Stats[class].ID but not sure if it’s necessary? I didn’t use it afterwards, but unsure if I would have to actually reference that or not?

EmeraldSlash · December 17, 2018, 2:38am

You use player.UserId as the key for the DataStore entry, not in the data itself. Having the UserId in the data achieves nothing and just makes things more complicated.

As for your changes, you define classID but you never use it, so there’s no point having it BUT… you want to be comparing k with classID, not class, so that’s where you should be using it: if k == classID then

NinjoOnline · December 17, 2018, 2:43am

That’d work fine for the knight, but once again, not for the archer.

Prints 1 1 for knight, 1 2 for archer

When the player selects the archer class, it should go through the players owned classes and see that the archer class ID matches with an ID they already have (2)

I would have thought it should be printing it like this:

Click Knight
Output:
1 1
2 1

Click Archer
Output
1 2
2 2

NinjoOnline · December 17, 2018, 2:46am

Wait I think I know why…

if k == class then
	return true
else
	return false
end

So it prints 1 2, which ain’t the same, so it returns false… which I’m guessing stops the rest of the loop? If so, where should I be putting the return false, so it runs through all of them and if there was no match, then return false??

NinjoOnline · December 17, 2018, 2:48am

function equipClass.OnServerInvoke(player, class)
	for i, v in pairs(classData) do
		if v[1] == player.UserId then
			for _, k in pairs(v[2]) do
				local classID = classStats.Stats[class].ID
				if k == classID then
					return true					
				end
			end
			return false
		end
	end
end

Works!!