Making this Update function faster?

Help and Feedback Code Review
#1

So I got my inventory system done and it’s fully working, my only “problem” is that it updates very slow.

This is probably due to all these checks and loops to check the items.

sendinv.OnClientEvent:Connect(function()
	print("a")
local getinv = replicatedStorage:WaitForChild("GetInventory"):InvokeServer()
local getShirt = replicatedStorage:WaitForChild("GetShirt"):InvokeServer()
local getPants = replicatedStorage:WaitForChild("GetPants"):InvokeServer()
local getSlot1 = replicatedStorage:WaitForChild("GetSlot1"):InvokeServer()
local getSlot2 = replicatedStorage:WaitForChild("GetSlot2"):InvokeServer()
local getSlot3 = replicatedStorage:WaitForChild("GetSlot3"):InvokeServer()
	--updateWeightMeter(max,weight)
	clearUI()

	for Position,Value in next,getinv do
    	local Frame = slots:FindFirstChild("Slot"..tostring(Position)) -- where the name is the number
		if getinv[Position][3] == 1 then
			Frame.ItemName.Text = getinv[Position][2]
		else
    	Frame.ItemName.Text = getinv[Position][2].." x"..getinv[Position][3]
		end
		Frame.ItemAmount.Text = "x"..getinv[Position][3]
		--Frame.ID.Value = getinv[Position][1]
		Frame.ItemPic.ImageTransparency = 0
		
		Frame.ItemPic.Image = "rbxassetid://"..itemsmodule[getinv[Position][2]]["Picture"]
	end

	--Update shirt
	for Position,Value in next,getShirt do
		local Frame = charframe.Shirt
		Frame.ItemName.Text = getShirt[Position][2]
	end
	--Update pants
	for Position,Value in next, getPants do
		local Frame = charframe.Pants
		Frame.ItemName.Text = getPants[Position][2]
	end
	--Update slot 1 
	for Position,Value in next, getSlot1 do
		local Frame = toolsframe.Slot1
		Frame.ItemName.Text = getSlot1[Position][2]
	end
	--Update slot 2
	for Position,Value in next, getSlot2 do
		local Frame = toolsframe.Slot2
		Frame.ItemName.Text = getSlot2[Position][2]
	end
	--Update slot 3
	for Position,Value in next, getSlot3 do
		local Frame = toolsframe.Slot3
		Frame.ItemName.Text = getSlot3[Position][2]
	end
	print("b")
end)

Basicly I am storing my inventory in serverstorage and I am using a remotefunction to ask the server for a table, the server then checks serverstorage and the right hierarchy and gives the client back a table containing all stored items and there names,amount etc.

This is the only Way I know on how to check from client to server to client.

These are my methodes in the serverscript

--[[Methodes to get all the stuff in ServerStorage]]--
game.ReplicatedStorage:WaitForChild("GetInventory").OnServerInvoke = function(player)
	local inv = {}
	
	for i, v in pairs(game.ServerStorage:WaitForChild("Inventory"):FindFirstChild(player.Name):GetChildren()) do
		if v:IsA("StringValue") then
		local vprop = {i,v.Name,v.Amount.Value}
		table.insert(inv,vprop)
		end
	end
	
	return inv
end

game.ReplicatedStorage:WaitForChild("GetShirt").OnServerInvoke = function(player)
	local inv = {}
	
	for i, v in pairs(game.ServerStorage.Inventory:FindFirstChild(player.Name).Character.Shirts:GetChildren()) do
		if v:IsA("StringValue") then
		local vprop = {i,v.Name}
		table.insert(inv,vprop)
		end
	end
	return inv
end

game.ReplicatedStorage:WaitForChild("GetPants").OnServerInvoke = function(player)
	local inv = {}
	
	for i, v in pairs(game.ServerStorage.Inventory:FindFirstChild(player.Name).Character.Pants:GetChildren()) do
		if v:IsA("StringValue") then
		local vprop = {i,v.Name}
		table.insert(inv,vprop)
		end
	end
	return inv
end

game.ReplicatedStorage:WaitForChild("GetHead").OnServerInvoke = function(player)
	local inv = {}
	
	for i, v in pairs(game.ServerStorage.Inventory:FindFirstChild(player.Name).Character.Hats:GetChildren()) do
		if v:IsA("StringValue") then
		local vprop = {i,v.Name}
		table.insert(inv,vprop)
		end
	end
	return inv
end

game.ReplicatedStorage:WaitForChild("GetSlot1").OnServerInvoke = function(player)
	local inv = {}
	
	for i, v in pairs(game.ServerStorage.Inventory:FindFirstChild(player.Name).Tools.Slot1:GetChildren()) do
		if v:IsA("StringValue") then
		local vprop = {i,v.Name}
		table.insert(inv,vprop)
		end
	end
	return inv
end

game.ReplicatedStorage:WaitForChild("GetSlot2").OnServerInvoke = function(player)
	local inv = {}
	
	for i, v in pairs(game.ServerStorage.Inventory:FindFirstChild(player.Name).Tools.Slot2:GetChildren()) do
		if v:IsA("StringValue") then
		local vprop = {i,v.Name}
		table.insert(inv,vprop)
		end
	end
	return inv
end

game.ReplicatedStorage:WaitForChild("GetSlot3").OnServerInvoke = function(player)
	local inv = {}
	
	for i, v in pairs(game.ServerStorage.Inventory:FindFirstChild(player.Name).Tools.Slot3:GetChildren()) do
		if v:IsA("StringValue") then
		local vprop = {i,v.Name}
		table.insert(inv,vprop)
		end
	end
	return inv
end

To get a better understanding why I am doing this, here is my structur from a players inventory.
image
Every stringvalue (except Weight) representing a Slot in the inventory

image
image867×555 271 KB

Thanks if you have any idea!

2 Likes
#2

This function takes a lot of time because of all the round trips to the server. You call :InvokeServer 6 times which means a lot of time is spent sending requests to the server. Instead of this you should send the information as arguments to :FireClient. Since this event is being triggered by the server you don’t need to then request information from the server because it can send the information with the event instead.

So on the server you would create a table with all the inventory info. Example:

local inventoryInfo = {
inv = getInventory(player),
shirts = getShirts(player),
pants = getPants(player),
--ect...
}
sendinv:FireClient(player, inventoryInfo)

On the client you would use the info from the table:

sendinv.OnClientEvent:Connect(function(inventoryInfo)
local inv = inventoryInfo.inv
--ect...
6 Likes
#3

Why didn’t I thought about this, worked like a charm! Thanks!