How does this shop manager look?

Help and Feedback Code Review
#1

How does the code for this shop system look?

ShopItems = workspace:WaitForChild("ShopItems")
ShopItemsChildren = ShopItems:GetChildren()

ChatService = game:GetService("Chat")
SS = game:GetService("ServerStorage")
Inventories = SS:WaitForChild("Inventories")
Tools = SS:WaitForChild("Items")

NPCs = workspace:WaitForChild("NPCs")
ShopKeeper = NPCs:WaitForChild("Shop Keeper")

debris = game:GetService("Debris")

function PurchaseItem(Player, ShopCounter, Money, NewItem, PlayerInventory, Price, ItemName)
	Money.Value = Money.Value - Price.Value
	ShopCounter.PurchaseSound:Play()
	ChatService:Chat(ShopKeeper.Head, "Thanks. Your '" .. ItemName.Value .. "' has been added to your inventory", Enum.ChatColor.Green)
	print("Successfully added '" .. ItemName.Value .. "' to " .. Player.Name .. "'s inventory!")
	local NewItemClone = NewItem:Clone()
	NewItemClone.Parent = PlayerInventory
end

function RegenItem(ItemToParent)
	if ItemToParent then
		ItemToParent.Parent = ShopItems
		RestartSystem()
		
	end
end


function ChatNpc(ChatPart, Message, ChatColour, ShopCounter, ErrorOutputMessage)
	ChatService:Chat(ChatPart, Message, ChatColour)
	if ErrorOutputMessage then
		warn(ErrorOutputMessage)
	end
end

function ItemTouched(ItemPart)
	ItemPart.Touched:Connect(function(hit)
		local ItemName = ItemPart:WaitForChild("ItemName")
		local Owner = ItemPart:WaitForChild("Owner")
		local Price = ItemPart:WaitForChild("Price")
		local RespawnItemBool = ItemPart:WaitForChild("RespawnItem")
		
		local ShopCounterCheck = hit:FindFirstChild("ShopCounter")
		local ShopCounter = hit
		
		if ShopCounterCheck and ShopCounter.Parent and ShopCounterCheck.Value == true and Owner.Value ~= nil then
			local Player = Owner.Value
			local Leaderstats = Player:WaitForChild("leaderstats")
			local Money = Leaderstats:WaitForChild("Money")
			if Money.Value >= Price.Value then
				local PlayerInventory = Inventories:FindFirstChild(Player.Name)
				if PlayerInventory then
					local NewItem = Tools:FindFirstChild(ItemName.Value)
					if NewItem then
						if not PlayerInventory:FindFirstChild(ItemName.Value) then
							PurchaseItem(Player, ShopCounter, Money, NewItem, PlayerInventory, Price, ItemName)
						else
							ChatNpc(ShopKeeper.Head, "Looks like you already own that", Enum.ChatColor.Blue, ShopCounter, nil)
							ShopCounter.ErrorSound:Play()
						end
					else
						ChatNpc(ShopKeeper.Head, "Uhh, looks like something went wrong.", Enum.ChatColor.Blue, ShopCounter, ItemName.Value .. " Is missing or nil")
						ShopCounter.ErrorSound:Play()
					end
				else
					ChatService:Chat(ShopKeeper.Head, "Uhh, looks like something went wrong.", Enum.ChatColor.Blue, ShopCounter, Player.Name .. "'s inventory is missing or nil")
					ShopCounter.ErrorSound:Play()
				end
			else
				ChatService:Chat(ShopKeeper.Head, "You need " .. Price.Value - Money.Value .. " more money to purchase that" , Enum.ChatColor.Red, ShopCounter, nil)
				ShopCounter.ErrorSound:Play()
			end
			RespawnItemBool.Value = true
			wait(0.5)
			ShopItemsChildren = ShopItems:GetChildren()
			RestartSystem()
		end
	end)
end

function RestartSystem()
	for i, Item in ipairs(ShopItemsChildren) do
		local SpareItem = Item:Clone()
		
		if Item:IsA("Model") then
			ItemTouched(Item.PrimaryPart)
		elseif Item:IsA("Part") then
			ItemTouched(Item)
		end
	end
end

RestartSystem()

while true do
	wait(1)
	local ShopItemsChildren2 = ShopItems:GetChildren()
	for i, Item in ipairs(ShopItemsChildren2) do
		if (Item:IsA("Part") and not Item.Anchored and Item.CanGrab.Value == true) or (Item:IsA("Model") and not Item.PrimaryPart.Anchored and Item.PrimaryPart.CanGrab.Value == true) then
			wait(3)
			if Item then
				if Item:IsA("Model") then
					if Item.PrimaryPart and Item.PrimaryPart:FindFirstChild("CanGrab") and Item.PrimaryPart.CanGrab.Value == true then
						Item.PrimaryPart.RespawnItem.Value = true
						wait(0.5)
						ShopItemsChildren = ShopItems:GetChildren()
						RestartSystem()
					end
				elseif Item:IsA("Part") then
					if Item:FindFirstChild("CanGrab") and Item.CanGrab.Value == true then
						Item.RespawnItem.Value = true
						wait(0.5)
						ShopItemsChildren = ShopItems:GetChildren()
						RestartSystem()
					end
				end
			end
		end
	end
end
5 Likes
#2

Really clean scripting, I like it.

1 Like
#3

Just from skimming over the code, I could figure out how pretty much all of it worked, which I consider a good sign of code organization. I couldn’t figure out why you have a variable for the Debris service, though.

From what I can tell, it seems your shop’s items are physically present in the store, and players touch them to attempt purchasing them. Is that correct?

The only other thing I would suggest is adding comments to your code. I just… don’t understand why nobody seems to do that. I don’t know about the rest of you, but it’s very difficult for me to return to a script, say, a month later, and immediately recognize whatever the heck I was doing without comments.

2 Likes