Is my shop framework good?

Help and Feedback Code Review
1

Is my shop framework good? Before I push and start adding tweening and the likes, I just made a little prototype for the system where it just increments a value. Everything works good in terms of printing, just wanted to make sure there is no memory leaks or anything. This is all on the client

local leftBtn = script.Parent.Frame.Left
local rightBtn = script.Parent.Frame.Right
local buyBtn = script.Parent.Frame.Buy
local costTxt = script.Parent.Frame.Cost
local pos = script.Parent.Position
local itemTransfer = workspace.StoreArea:FindFirstChild("Item"..pos.Value)
local transferDb = false

leftBtn.MouseButton1Down:Connect(function()
	if pos.Value - 1 > 0 and not transferDb then
		transferDb = true
		pos.Value -= 1
		itemTransfer = workspace.StoreArea:FindFirstChild("Item"..pos.Value)
		print(itemTransfer.Name)
		wait(2)
		transferDb = false
	end
end)
rightBtn.MouseButton1Down:Connect(function()
	if pos.Value + 1 < 5 and not transferDb then
		transferDb = true
		pos.Value += 1
		itemTransfer = workspace.StoreArea:FindFirstChild("Item"..pos.Value)
		print(itemTransfer.Name)
		wait(2)
		transferDb = false
	end
end)


game.ReplicatedStorage.StoreArea.OnClientEvent:Connect(function(hum)
	local cam = workspace.CurrentCamera
	cam.CameraType = Enum.CameraType.Scriptable
	local rsCon
	rsCon = game:GetService("RunService").RenderStepped:Connect(function()
		cam.CFrame = workspace.StoreArea.StoreCamera.CFrame
	end)
	script.Parent.Frame.Visible = true
	
	local uisCon
	uisCon = game:GetService("UserInputService").InputBegan:Connect(function(key)
		if key.KeyCode == Enum.KeyCode.X then
			game.ReplicatedStorage.StoreArea:FireServer(hum)
			rsCon:Disconnect()
			uisCon:Disconnect()
			script.Parent.Frame.Visible = false
			cam.CameraType = Enum.CameraType.Custom
		end
	end)
end)
1 Like
2

There will be no memory leaks whatsoever. Your code seems fine but it can be significantly be improved.

Starting off, use Activated rather than MouseButton1Down, the later works for all devices. Also define services first, that way you won’t have to do game:GetService everytime you want to get a service.

The connections are named weirdly, and inconsistent, uisCon should be just channged to uisConnection instead, same goes for other variables for connections.

For added flexibility, it’s much better to loop through the buttons instead of manually connection a function for each button to check if they were activated. (Only if you have more than 3 buttons).

How I would write your code:

local ReplicatedStorage = game:GetService("ReplicatedStorage")
local RunService = game:GetService("RunService")
local UserInputService = game:GetService("UserInputService")

local leftButton = script.Parent.Frame.Left
local rightButton = script.Parent.Frame.Right
local buyButton = script.Parent.Frame.Buy
local costText = script.Parent.Frame.Cost
local pos = script.Parent.Position
local transferDebounce = false

local itemTransfer = workspace.StoreArea:FindFirstChild("Item"..pos.Value)

leftButton.Activated:Connect(function()
	if (pos.Value - 1 > 0 and transferDebounce == false) then
		transferDebounce = true
		pos.Value -= 1
		itemTransfer = workspace.StoreArea:FindFirstChild("Item"..pos.Value)
		print(itemTransfer.Name)
		wait(2)
		transferDebounce = false
	end
end)

rightButton.Activated:Connect(function()
	if (pos.Value + 1 < 5 and transferDebounce == false) then
		transferDebounce = true
		pos.Value += 1
		itemTransfer = workspace.StoreArea:FindFirstChild("Item"..pos.Value)
		print(itemTransfer.Name)
		wait(2)
		transferDebounce = false
	end
end)

local connection1
local connection2

ReplicatedStorage.StoreArea.OnClientEvent:Connect(function(hum)
	local cam = workspace.CurrentCamera
	
	cam.CameraType = Enum.CameraType.Scriptable
	
	connection1 = RunService.RenderStepped:Connect(function()
		cam.CFrame = workspace.StoreArea.StoreCamera.CFrame
	end)
	
	script.Parent.Frame.Visible = true
end)

connection2 = UserInputService.InputBegan:Connect(function(input, processed)
	if (processed == true) then return end 

	if input.KeyCode == Enum.KeyCode.X then
		ReplicatedStorage.StoreArea:FireServer(hum)
		connection1:Disconnect()
		connection2:Disconnect()
		script.Parent.Frame.Visible = false
		cam.CameraType = Enum.CameraType.Custom
	end
end)
3 Likes