Is there any way to further optimize?

Help and Feedback Code Review
1

Hey!

After writing this section of code for my spell creation system, I was curious if there was any way to further optimize this?

-- Static Values: CheckSpell = ["SpellName1"]
game.ReplicatedStorage.Storage.CraftSpell.OnServerInvoke = function(player, spellName)
	local playerData = game.ServerStorage.PlayerData:FindFirstChild(player.Name)
	if not playerData then return end
	if SpellRequirements[spellName] == nil then return end -- player tried to request a non existent spell
	print(spellName)
	local CheckSpell = game.HttpService:JSONDecode(playerData.SpellsCreated.Value)
	if table.find(CheckSpell, spellName) ~= nil then return false end -- player tried to craft the same spell twice
	local GetItems = string.split(SpellRequirements[spellName].ScriptCost, ",")
	local VerifyItems = {}
	-- get all items and check if they match the ones in the backpack
	for i,v in pairs(GetItems) do
		for o,b in pairs(player.Backpack:GetChildren()) do
			print(v)

			if v == b.Name then
				table.insert(VerifyItems, b.Name)
			end
		end

	end
	local JSONVerifyItems = game.HttpService:JSONEncode(VerifyItems)
	-- if player meets the requirement then we give them the spell
	if JSONVerifyItems == SpellRequirements[spellName].Requirements then
		local createdSpells = game.HttpService:JSONDecode(playerData.SpellsCreated.Value)
		table.insert(createdSpells, spellName)
		playerData.SpellsCreated.Value = game.HttpService:JSONEncode(createdSpells)
		print("Found all items")
		for i,v in pairs(VerifyItems) do
			player.Backpack:FindFirstChild(v):Destroy() -- destroy their items
		end
		return true
	else
		return false
	end
	
end
2

Optimizations

Optimization 1

You should use a dictionary-like table when encoding this, so it gets encoded to a JSON object. After that, to check if a player has crafted the spell, you can index the dictionary and check if you get a truthy value. I’ve explained that in more detail here.

This optimization requires a little bit of modification but should be worth it.

Optimization 2

I assume that there is only one child with the name b. Am I right? In that case, you can delete the inner for loop and replace it with this:

local child = player.Backpack:FindFirstChild(v)
if child then
	table.insert(VerifyItems, v)
end

You will need to apply the modifications in optimization #1 here too, if you’ve applied that optimization.

Micro Optimizations

  • Make it so you use GetService to get HttpService and put that into a variable.
  • Same for ReplicatedStorage.
  • if not SpellRequirements[spellName] then return end will also work unless SpellReuirements is a table of booleans.
3 Likes