Help improve my code for an inventory system

Hi, so im making an inv system for a game where you collect fruit, berries and other that kinds of stuff.

Heres my current code:

Module script:

local Inventory = {}

function Inventory.AddUI(itemTemp, itemFrame, child)
	local ClonedUI = itemTemp:Clone()
	ClonedUI.Parent = itemFrame
	ClonedUI.Name = child.Name
	ClonedUI.Text = child.Name
end

function Inventory.RemoveUI(itemFrame, child)
	for i, button in pairs(itemFrame:GetDescendants()) do
		if button.Name == child.Name then
			if button:IsA("TextButton") then
				button:Destroy()
			end
		end
	end
end

return Inventory

Local script in starterplayer scripts:

local Player = game:GetService("Players").LocalPlayer
local UIS = game:GetService("UserInputService")
local CollectionService = game:GetService("CollectionService")
local Player = game:GetService("Players").LocalPlayer
local Mouse = Player:GetMouse()

local ObjectTags = {"Fruit", "Berry"}
local PickUpKey = "F"

local Remotes = game.ReplicatedStorage:WaitForChild("Remotes")
local PickUpItemEvent = Remotes.PickUpItem
local DropItemEvent = Remotes.DropItem

local PickUpSound = game.SoundService.PickSound

local InventoryUI = Player.PlayerGui:WaitForChild("InventoryUI")

local function CreateSelectionbox(Object)
	local SBox = Instance.new("SelectionBox")
	SBox.Parent = Object

	SBox.Visible = true
	SBox.Adornee = Object
	SBox.LineThickness = 0.01
	SBox.Color3 = Color3.new(1, 1, 1)
	
	return SBox
end

local function RemoveSelectionbox(SBox)
	if SBox then
		SBox:Destroy()
	end
end

local function InputChanged(input)
	local FoundItem = false
	
	if Mouse.Target then
		for i, tag in pairs(ObjectTags) do
			if CollectionService:HasTag(Mouse.Target, tag) then
				local Item = Mouse.Target
				local SelectionBox = Item:FindFirstChildOfClass("SelectionBox")
				
				if not SelectionBox then
					SelectionBox = CreateSelectionbox(Item)
				end
				
				FoundItem = true
			end
		end
	end
	
	if FoundItem == false then
		for i, object in pairs(workspace:GetDescendants()) do
			for i, tag in pairs(ObjectTags) do
				if CollectionService:HasTag(object, tag) then
					local OldSBox = object:FindFirstChildOfClass("SelectionBox")

					if OldSBox then
						OldSBox:Destroy()
					end
				end
			end
		end
	end
end

local function PickUpItem(input)
	if input.KeyCode == Enum.KeyCode[PickUpKey] then
		if Mouse.Target then
			for i, tag in pairs(ObjectTags) do
				if CollectionService:HasTag(Mouse.Target, tag) then
					PickUpItemEvent:FireServer(Mouse.Target)
					
					PickUpSound:Play()
				end
			end
		end
	end
end


game:GetService("RunService").Stepped:Connect(function()
	for i, button in pairs(InventoryUI.ItemsFrame:GetChildren()) do
		if button:IsA("TextButton") then
			button.MouseButton1Up:Connect(function()
				DropItemEvent:FireServer(button)
			end)
		end
	end
end)

UIS.InputEnded:Connect(PickUpItem)
UIS.InputChanged:Connect(InputChanged)

Serverscript:

local Remotes = game.ReplicatedStorage:WaitForChild("Remotes")
local PickUpItemEvent = Remotes.PickUpItem
local DropItemEvent = Remotes.DropItem
local InventoryFolder = game.ServerStorage.Inventory

local InvModule = require(game.ServerScriptService.InventoryModule)

local function PickUpItem(Player, Item)
	ItemTemplate = game.ReplicatedStorage:WaitForChild("Templates").ItemTemplate
	InventoryUI = Player.PlayerGui:WaitForChild("InventoryUI")
	InvUIFrame = Player.PlayerGui.InventoryUI.ItemsFrame
	
	if Item:IsA("BasePart") then
		Item.Parent = InventoryFolder
	else
		Item:Destroy()
	end
end

local function DropItem(Player, Button)
	for i, item in pairs(InventoryFolder:GetChildren()) do
		if item:IsA("BasePart") then
			if item.Name == Button.Name then
				item.Parent = workspace
				item.Anchored = false
				item.CFrame = Player.Character.HumanoidRootPart.CFrame*CFrame.new(Vector3.new(0,5,-7.5))
			end
		end
	end
end

InventoryFolder.ChildAdded:Connect(function(child)
	InvModule.AddUI(ItemTemplate, InvUIFrame, child)
end)

InventoryFolder.ChildRemoved:Connect(function(child)
	InvModule.RemoveUI(InvUIFrame, child)
end)

DropItemEvent.OnServerEvent:Connect(DropItem)
PickUpItemEvent.OnServerEvent:Connect(PickUpItem)

Thanks in advance :slight_smile: !

3 Likes

Looks really tidy and well programmed, good work!
Only thing that I would change would be this:

game:GetService("RunService").Stepped:Connect(function()
   for i, button in pairs(InventoryUI.ItemsFrame:GetChildren()) do
   	if button:IsA("TextButton") then
   		button.MouseButton1Up:Connect(function()
   			DropItemEvent:FireServer(button)
   		end)
   	end
   end
end)

This is constantly creating new events way too fast. I’d suggest you change this to event based. Do a .ChildAdded on the folder so that you only create a new event for a new ui element when it gets added.

3 Likes

This topic was automatically closed 14 days after the last reply. New replies are no longer allowed.