Is there a way to make this code a lot cleaner?

Help and Feedback Code Review
#1

Hello there,

I have been working on a multi rebirth system and it works 100% but it is very repetitive is there any way I can improve this script?

local ReplicatedS = game:GetService("ReplicatedStorage")
local RS = ReplicatedS:FindFirstChild("Rebirths")
local Rebirth1 = RS:FindFirstChild("Rebirth1")
local Rebirth5 = RS:FindFirstChild("Rebirth5")
local Rebirth10 = RS:FindFirstChild("Rebirth10")
local Rebirth50 = RS:FindFirstChild("Rebirth50")
local Rebirth100 = RS:FindFirstChild("Rebirth100")
local Rebirth250 = RS:FindFirstChild("Rebirth250")
local Rebirth500 = RS:FindFirstChild("Rebirth500")
local Rebirth1000 = RS:FindFirstChild("Rebirth1000")

Rebirth1.OnServerEvent:Connect(function(plr)
	local Passes = plr.Passes
	local DoubleGems = Passes.DoubleGems
	local Rebirths = plr.leaderstats.Rebirths
	local Coins = plr.leaderstats.Coins
	local MaxTaps = plr.MaxValues.MaxTaps
	local Gems = plr.leaderstats.Gems
	local Taps = plr.leaderstats.Taps
	local totalRebirths = plr.leaderstats.Rebirths
	local rebirthPrice = (totalRebirths.Value + 1) * 100
	if Coins.Value >= rebirthPrice then
		Coins.Value = 0
		Taps.Value = 0
		Rebirths.Value = Rebirths.Value + 1
		if DoubleGems.Value == true then
			Gems.Value = (Gems.Value + 1) * 2
		else
			Gems.Value = Gems.Value + 1
		end
	end
end)
Rebirth5.OnServerEvent:Connect(function(plr)
	local Passes = plr.Passes
	local DoubleGems = Passes.DoubleGems
	local Rebirths = plr.leaderstats.Rebirths
	local Coins = plr.leaderstats.Coins
	local MaxTaps = plr.MaxValues.MaxTaps
	local Gems = plr.leaderstats.Gems
	local Taps = plr.leaderstats.Taps
	local totalRebirths = plr.leaderstats.Rebirths
	local rebirthPrice = (totalRebirths.Value + 1) * 500
	if Coins.Value >= rebirthPrice then
		Coins.Value = 0
		Taps.Value = 0
		Rebirths.Value = Rebirths.Value + 5
		if DoubleGems.Value == true then
			Gems.Value = (Gems.Value + 5) * 2
		else
			Gems.Value = Gems.Value + 5
		end
	end
end)
Rebirth10.OnServerEvent:Connect(function(plr)
	local Passes = plr.Passes
	local DoubleGems = Passes.DoubleGems
	local Rebirths = plr.leaderstats.Rebirths
	local Coins = plr.leaderstats.Coins
	local MaxTaps = plr.MaxValues.MaxTaps
	local Gems = plr.leaderstats.Gems
	local Taps = plr.leaderstats.Taps
	local totalRebirths = plr.leaderstats.Rebirths
	local rebirthPrice = (totalRebirths.Value + 1) * 1000
	if Coins.Value >= rebirthPrice then
		Coins.Value = 0
		Taps.Value = 0
		Rebirths.Value = Rebirths.Value + 10
		if DoubleGems.Value == true then
			Gems.Value = (Gems.Value + 10) * 2
		else
			Gems.Value = Gems.Value + 10
		end
	end
end)

I didn’t copy all of the code cause it is a lot and is basically just repeating this with other values.

Thank You!

#2

You can use one RemoteEvent and add string arguments to determine which kind of Rebirth it is.

2 Likes
#3

not exactly sure what you mean…

#4 
--server
local remoteEvent = blah:WaitForChild('blah')

remoteEvent.OnServerEvent:Connect(function(sender, sort)
	--u can set up any variables here that are constant then add on individual ones inside the if statements
	local a
	local b
	local c
	local d
	if sort == 1 then
		print('1')
	elseif sort == 2 then
		print('2')
	elseif sort == 3 then
		print('3')
	end
end)

--local
local remoteEvent = blah:WaitForChild('blah')

remoteEvent:FireServer(1)
--or
remoteEvent:FireServer(2)
--or
remoteEvent:FireServer(3)
#5

Oh thank you will try that :smiley:

#6

You could also use a function like so

local function blah(plr, rebirthType)
	local Passes = plr.Passes
	local DoubleGems = Passes.DoubleGems
	local Rebirths = plr.leaderstats.Rebirths
	local Coins = plr.leaderstats.Coins
	local MaxTaps = plr.MaxValues.MaxTaps
	local Gems = plr.leaderstats.Gems
	local Taps = plr.leaderstats.Taps
	local totalRebirths = plr.leaderstats.Rebirths
	local rebirthPrice = (totalRebirths.Value + 1) * (rebirthType * 10)
	if Coins.Value >= rebirthPrice then
		Coins.Value = 0
		Taps.Value = 0
		Rebirths.Value = Rebirths.Value + rebirthType
		if DoubleGems.Value == true then
			Gems.Value = (Gems.Value + rebirthType) * 2
		else
			Gems.Value = Gems.Value + rebirthType
		end
	end
end
#7

Have you considered storing all of those references to the BaseValue instances in the scope of the script as opposed to the scope of each closure?

#8

Instead of using multiple if statements, you can just make a table and get the info from that using the Rebirth option. I also changed the double gems, you multiplied their Current gems by 2 as well. Unless that was intentional, you can easily change it back.

local RebirthRemote = ReplicatedStorage

local RebirthData = {
	[1] = {100, 1};
	[5]  = {500, 5};
	[100]  = {1000, 10}; -- Cost, Gems
}

RebirthRemote.OnServerEvent:Connect(function(Player, Rebirth) -- Rebirth should be a number
	local Passes = Player.Passes
	local DoubleGems = Passes.DoubleGems
	local Rebirths = Player.leaderstats.Rebirths
	local Coins = Player.leaderstats.Coins
	local MaxTaps = Player.MaxValues.MaxTaps
	local Gems = Player.leaderstats.Gems
	local Taps = Player.leaderstats.Taps
	local totalRebirths = Player.leaderstats.Rebirths
	local RebirthInfo = RebirthData[Rebirth]
	local rebirthPrice = (totalRebirths.Value + Rebirth) * RebirthInfo[1]
	
	if Coins.Value >= rebirthPrice then
		Coins.Value = 0
		Taps.Value = 0
		Rebirths.Value = Rebirths.Value + Rebirth
		
		if DoubleGems.Value == true then
			Gems.Value = Gems.Value + (RebirthInfo[2] * 2)
		else
			Gems.Value = Gems.Value + RebirthInfo[2]
		end
	end
end)
3 Likes