How would I make this code neater

I’m pretty sure the title explains the premise but right now almost everything is nested so I want tips or things I could do better to make my code neater and easier to read.

local button = script.Parent
local ServerStorage = game:GetService("ServerStorage")
local replicatedStorage = game:GetService("ReplicatedStorage")
local buyCrate = replicatedStorage.CaseEvents:WaitForChild("BuyCrate")
local itemsFolder = ServerStorage.Items
local inventories = ServerStorage.Inventories

local Rarities = {
	Common = 0,      -- 60% chance
	Uncommon = 0.62, -- 38% chance
	Rare = 0.90,     -- 10% chance
	Legendary = 0.99 -- 2% chance
}

-- Function to pick a rarity based on chance
local function PickRarity()
	local Index = math.random()
	local HighestRarity = "Common"

	for RarityName, Value in pairs(Rarities) do
		if Index >= Value and Value >= Rarities[HighestRarity] then
			HighestRarity = RarityName
		end
	end

	return HighestRarity
end

-- Connect the button click event
buyCrate.OnServerEvent:Connect(function(localPlayer, cost)
	-- Deduct cost from the player's money
	localPlayer.datastats.Money.Value -= cost

	-- Pick a random rarity
	local Rarity = PickRarity()

	-- Find and give the player a random item of the selected rarity
	for _, RarityFolder in pairs(itemsFolder:GetChildren()) do
		if RarityFolder.Name == Rarity then

			local items = RarityFolder:GetChildren()
			local item = items[math.random(1, #items)] -- Pick a random item from the rarity's folder

			for _, playerInventory in pairs(inventories:GetChildren()) do
				if playerInventory.Name == localPlayer.Name then
					local newItem = playerInventory:FindFirstChild(item.Name)
					if newItem then
						-- give them a refund depending on what rarity; not done yet
						print("Player already has item")
					else
						print("Player doesn't have item")
						newItem = item:Clone()
						newItem.Parent = playerInventory
					end
				end
			end
		end
	end
end)

first you could try to be consistent with your spelling

local SS = game:GetService("ServerStorage")
local RS = game:GetService("ReplicatedStorage")

local Button = script.Parent

local BuyCreate = RS.CaseEvents:WaitForChild("BuyCrate")
local Items = SS:WaitForChild("Items")
local Inventories = SS:WaitForChild("Inventories")

local Rarities = {
	["Common"] = 0;      -- 60% chance
	["Uncommon"] = 0.62; -- 38% chance
	["Rare"] = 0.90;     -- 10% chance
	["Legendary"] = 0.99; -- 2% chance
}

-- Function to pick a rarity based on chance
local function PickRarity()
	local Index = math.random()
	local HighestRarity = "Common"

	for RarityName, Value in pairs(Rarities) do
		if Index >= Value and Value >= Rarities[HighestRarity] then
			HighestRarity = RarityName
		end
	end

	return HighestRarity
end

-- Connect the button click event
BuyCreate.OnServerEvent:Connect(function(Player, Cost)
	-- Deduct cost from the player's money
	Player.datastats.Money.Value -= Cost

	-- Pick a random rarity
	local Rarity = PickRarity()

	-- Find and give the player a random item of the selected rarity
	for _, RarityFolder in pairs(Items:GetChildren()) do
		if RarityFolder.Name == Rarity then

			local Items = RarityFolder:GetChildren()
			local Item = Items[math.random(#Items)] -- Pick a random item from the rarity's folder

			for _, Inventory in pairs(Inventories:GetChildren()) do
				if Inventory.Name == Player.Name then
					local NewItem = Inventory:FindFirstChild(Item.Name)
					
					if NewItem then
						-- give them a refund depending on what rarity; not done yet
						print("Player already has item")
					else
						print("Player doesn't have item")
						NewItem = Item:Clone()
						NewItem.Parent = Inventory
					end
				end
			end
		end
	end
end)

next you could try to remove the extra indentation

local SS = game:GetService("ServerStorage")
local RS = game:GetService("ReplicatedStorage")

local Button = script.Parent

local BuyCreate = RS.CaseEvents:WaitForChild("BuyCrate")
local Items = SS:WaitForChild("Items")
local Inventories = SS:WaitForChild("Inventories")

local Rarities = {
	["Common"] = 0;      -- 60% chance
	["Uncommon"] = 0.62; -- 38% chance
	["Rare"] = 0.90;     -- 10% chance
	["Legendary"] = 0.99; -- 2% chance
}

-- Function to pick a rarity based on chance
local function PickRarity()
	local Index = math.random()
	local HighestRarity = "Common"

	for RarityName, Value in pairs(Rarities) do
		if Index >= Value and Value >= Rarities[HighestRarity] then
			HighestRarity = RarityName
		end
	end

	return HighestRarity
end

-- Connect the button click event
BuyCreate.OnServerEvent:Connect(function(Player, Cost)
	-- Deduct cost from the player's money
	Player.datastats.Money.Value -= Cost

	-- Pick a random rarity
	local Rarity = PickRarity()

	-- Find and give the player a random item of the selected rarity
	for _, RarityFolder in pairs(Items:GetChildren()) do
		if RarityFolder.Name ~= Rarity then continue end

		local Items = RarityFolder:GetChildren()
		local Item = Items[math.random(#Items)] -- Pick a random item from the rarity's folder

		for _, Inventory in pairs(Inventories:GetChildren()) do
			if Inventory.Name ~= Player.Name then continue end
			local NewItem = Inventory:FindFirstChild(Item.Name)
			print(NewItem and "Player doesn't have item" or "Player already has item")
			NewItem = NewItem or Item:Clone()
			NewItem.Parent = Inventory
		end
	end
end)

and last i would remove all of the comments, but you should keep them if you really need them

local SS = game:GetService("ServerStorage")
local RS = game:GetService("ReplicatedStorage")

local Button = script.Parent

local BuyCreate = RS.CaseEvents:WaitForChild("BuyCrate")
local Items = SS:WaitForChild("Items")
local Inventories = SS:WaitForChild("Inventories")

local Rarities = {
	["Common"] = 0;
	["Uncommon"] = 0.62;
	["Rare"] = 0.90;
	["Legendary"] = 0.99;
}

local function PickRarity()
	local Index = math.random()
	local HighestRarity = "Common"

	for RarityName, Value in pairs(Rarities) do
		if Index >= Value and Value >= Rarities[HighestRarity] then
			HighestRarity = RarityName
		end
	end

	return HighestRarity
end

BuyCreate.OnServerEvent:Connect(function(Player, Cost)
	local Rarity = PickRarity()
	Player.datastats.Money.Value -= Cost

	for _, RarityFolder in pairs(Items:GetChildren()) do
		if RarityFolder.Name ~= Rarity then continue end

		local Items = RarityFolder:GetChildren()
		local Item = Items[math.random(#Items)]

		for _, Inventory in pairs(Inventories:GetChildren()) do
			if Inventory.Name ~= Player.Name then continue end
			local NewItem = Inventory:FindFirstChild(Item.Name)
			print(NewItem and "Player doesn't have item" or "Player already has item")
			NewItem = NewItem or Item:Clone()
			NewItem.Parent = Inventory
		end
	end
end)
1 Like

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