Better way to do this?

Is there a better way to rescript this? It’s a luck rarity script.

local rs = game:GetService("ReplicatedStorage")
local Shapes = rs:WaitForChild("Shapes", 60):GetDescendants()

local Spawner = workspace:WaitForChild("Spawner")
local MoneyEvent = rs.MoneyEvent
local Chance = require(rs:WaitForChild("Chance"))

local gui = script.Parent
local Button = gui.ShapeButton

local HasShape = false
local OldShape = nil

local MoneyPoints = 0

print("Everything loaded.")

Button.MouseButton1Down:Connect(function()
	local Rarity = Chance.PickRarity("Rarities")
	print(Rarity)
	for i, v in pairs(Shapes) do
		if v:IsA("BasePart") or v:IsA("MeshPart") or v:IsA("UnionOperation") then
			local Folder_Shapes = v.Parent

			if Rarity == Folder_Shapes.Name then

				local RandomShape = Folder_Shapes:GetChildren()[math.random(1, #Folder_Shapes:GetChildren())]:Clone()

				if HasShape ~= true then
					HasShape = true

					RandomShape.Parent = Spawner
					RandomShape.CFrame = CFrame.new(Spawner.CFrame.X, Spawner.CFrame.Y, Spawner.CFrame.Z)

					print(RandomShape)
					OldShape = RandomShape
					
					if Folder_Shapes.Name == "Common" then
						MoneyPoints = 10
						MoneyEvent:FireServer(MoneyPoints)
					end

					if Folder_Shapes.Name == "Rare" then
						MoneyPoints = 30
						MoneyEvent:FireServer(MoneyPoints)
					end

					break

				elseif HasShape == true then

					OldShape:Destroy()
					HasShape = false
				end
			end
		end

	end

end)
local rs = game:GetService("ReplicatedStorage")
local Shapes = rs:WaitForChild("Shapes", 60):GetDescendants()

local Spawner = workspace:WaitForChild("Spawner")
local MoneyEvent = rs.MoneyEvent
local Chance = require(rs:WaitForChild("Chance"))

local gui = script.Parent
local Button = gui.ShapeButton

local HasShape = false
local OldShape = nil

local MoneyPoints = 0

print("Everything loaded.")

Button.MouseButton1Down:Connect(function()
	local Rarity = Chance.PickRarity("Rarities")
	print(Rarity)
	for i, v in pairs(Shapes) do
		if v:IsA("BasePart") or v:IsA("MeshPart") or v:IsA("UnionOperation") then
			local Folder_Shapes = v.Parent

			if Rarity == Folder_Shapes.Name then

				local RandomShape = Folder_Shapes:GetChildren()[math.random(1, #Folder_Shapes:GetChildren())]:Clone()

				if HasShape == false then --better written as '== false' because it makes more sense if you ever go back to this code and forgot what you did, but you dont have to it just makes it easier to read
					HasShape = true

					RandomShape.Parent = Spawner
					RandomShape.CFrame = Spawner.CFrame --you could shrink this by just getting the entire cframe instead of getting x,y, and z separately 

					print(RandomShape)
					OldShape = RandomShape
					
					if Folder_Shapes.Name == "Common" then
						MoneyPoints = 10
						MoneyEvent:FireServer(MoneyPoints)
					elseif Folder_Shapes.Name == "Rare" then --you could do elseif instead
						MoneyPoints = 30
						MoneyEvent:FireServer(MoneyPoints)
					end

					break

				else --you dont have to check if its true if you already checked if its false. Booleans can only be true or false 

					OldShape:Destroy()
					HasShape = false
				end
			end
		end

	end

end)

this is just a personal preference, but I like making things in lowercase, it just make things faster slightly faster to write. Although if you prefer it in uppercase, you can do that. It is just a personal preference thing.

1 Like

other than that, this seems like the best way to do things

The thing is, I want to do an automated system, and not do this manually so it’s easy to add new rarities without changing much. I just can’t wrap that around my head. Thanks for the help though

local Rarities = {
["Common"] = {
MoneyPoints = 10
},
["Rare"] = {
MoneyPoints = 20
},
}
-- Now get rid of the if statement and the MoneyPoints variable
MoneyEvent:FireServer(Rarities[Folder_Shapes.Name].MoneyPoints)
2 Likes

Hello! Thanks for the help! I’ll test it out tomorrow since it’s rather late

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