I’m working on a custom backpack from my game and it should function the same as the default one but with only 5 slots. Even after looking through the backpack’s source code I’m still having trouble wrapping my head around it all.
More specifically, some issues I’m having are whenever the player respawns they keep one of their slots for a tool instead of clearing them all, removing a tool from the player’s backpack or character doesn’t work, the hotbar refreshes items slots when unequipping an item rather than just keeping the icon there, etc.
How would I improve my code to fix these issues?
My current code
function UIManager.SetupHotbar()
local Humanoid = Player.Character:WaitForChild("Humanoid")
local Backpack = Player:WaitForChild("Backpack")
local Hotbar = MainGameGuis.Hotbar
local hotbarSlots = {}
local equippedTool = nil
local function ChildAdded(child)
if child:IsA("Tool") then
for i, slot in pairs(hotbarSlots) do
if slot.Tool == child then
return
end
end
for i, slot in pairs(hotbarSlots) do
if slot.Tool == nil then
slot.Tool = child
slot.UI.ToolTip.Text = child.ToolTip
local toolModel = Funcs.CreateModelFromTool(child)
toolModel.PrimaryPart = toolModel:GetChildren()[1]
toolModel:SetPrimaryPartCFrame(CFrame.new())
UIManager.RenderViewport(slot.UI.ImageHolder, {WorldModel = toolModel}, "building-material", "hover", slot.UI) --Don't worry about this part is just sets up a ViewportFrame
break
end
end
end
end
local function ChildRemoved(child)
if child:IsA("Tool") then
for i, slot in pairs(hotbarSlots) do
if slot.Tool == child and equippedTool ~= slot.Tool then
slot.Tool = nil
slot.UI.ToolTip.Text = ""
slot.UI.ToolTip.Visible = false
slot.UI.ImageHolder:ClearAllChildren()
break
end
end
end
end
local function CharacterAdded(character)
Humanoid = character:WaitForChild("Humanoid")
Backpack = Player:WaitForChild("Backpack")
Backpack.ChildAdded:Connect(ChildAdded)
Backpack.ChildRemoved:Connect(ChildRemoved)
character.ChildAdded:Connect(ChildAdded)
character.ChildRemoved:Connect(ChildRemoved)
for i, child in pairs(Backpack:GetChildren()) do
ChildAdded(child)
end
end
for i = 1, 5 do
local slot = Hotbar.Template:Clone()
slot.Number.Text = i
slot.Name = i
slot.Parent = Hotbar
slot.Visible = true
hotbarSlots[i] = {UI = slot, Tool = nil}
slot.MouseEnter:Connect(function()
if hotbarSlots[i].Tool ~= nil then
slot.ToolTip.Visible = true
end
end)
slot.MouseLeave:Connect(function()
slot.ToolTip.Visible = false
end)
slot.MouseButton1Click:Connect(function()
if hotbarSlots[i].Tool ~= nil then
SoundService.ClientSounds.GUIClick:Play()
if equippedTool then
equippedTool = nil
Humanoid:UnequipTools()
else
equippedTool = hotbarSlots[i].Tool
Humanoid:EquipTool(hotbarSlots[i].Tool)
end
end
end)
end
Player.CharacterAdded:Connect(CharacterAdded)
CharacterAdded(Player.Character)
end
Here’s an approach. You should have another function whose job it is to set up (and tear down) a button. Use ChildAdded on the character/backpack to detect new tools. Use AncestryChanged on the tool to detect when the tool is no longer under the player’s control.
local toolButtons = {}
function UIManager.SetupHotbarButtonForTool(tool)
local button = createSomeButtonForThisTool(tool) -- abstract
toolButtons[tool] = button
local conn
local function cleanup()
conn:Disconnect()
toolButtons[tool] = nil
button:Destroy()
end
conn = tool.AncestryChanged:Connect(function ()
-- Check if the tool has left the Backpack or Character.
-- if it did, we need to tear down
end)
end
function UIManager.SetupHotbar(tool, button)
-- Find the player's backpack and character...
local backpack = ...
local character = ...
local function onChildAdded(child)
if child:IsA("Tool") and not toolButtons[child] then
UIManager.SetupHotbarButtonForTool(child)
end
end
backpack.ChildAdded:Connect(onChildAdded)
character.ChildAdded:Connect(onChildAdded)
end
On a side note, a backpack like this would benefit greatly from using an object-oriented pattern, which I’ve written about on my website if you want to take your code to a more organized and workable level.
You might also consider using a Maid pattern to tear down tool buttons. I have a very straightforward, ready-to-use version in Modules. It is based on Quenty’s Maid class from Nevermore. See below:
When developing the system I have currently, I thought about making it object-oriented but shelved that idea because I thought it might be a more simple task. Judging by what I have now, and the code you sent, would it really be a good idea to go through the extra programming to make it object-oriented though? I’m sure there would be benefits like having references for all client scripts to see what the player has equipped, of course.
Here’s some arguments for and against. I’ll start with for:
Simplicity. It’s very easy to conceptualize what your object should be. In your case, the object’s job is to bind the Tool with the button in your UI. Knowing this, it’s much easier to keep code related to that task in-scope in one ModuleScript.
Easier setup and teardown. Include a maid in the object and give it tasks which tear down the object and you’ll not have to worry about a memory leaks.
Adding more behaviors is much easier. Adding member variables and methods (functions) is really simple with the OOP pattern. Everything’s encapsulated in its own object, so you don’t need to worry about cluttering up some other thing in your game.
Against:
Uses metatables. If you aren’t terribly familiar, the finer mechanics of simulating OOP in Lua can be confusing, especially when trying to implement inheritance. Otherwise, this isn’t a drawback.
Less performant, albeit not by much. Of course you’ll use slightly more memory, but you’re only storing 5 of these, so this isn’t a concern. Also, Luau has everything working blazingly fast these days.
Refactoring. If you’ve got something that works already, refactoring it just for the sake of being cool and using OOP (which, yes, it’s cool) isn’t really worth your time.
Hopefully that can help you make an informed decision on whether you end up refactoring your code to use an OOP pattern. I highly recommend it for personal growth. This is a good opportunity to use it, so you might as well take it