How can I make this script more efficient?

Help and Feedback Code Review
1

Hi, I’m kind of new to the Lua programming language and need help to make my code more efficient and better looking. How can I make this better??

Code:

CheckSword = script.Parent.Parent.ImagePreview.Image			-- Find Out what sword it is by the preview image
SwordPrice = script.Parent.Parent.Price.Text					-- Find Out Sword Price By the Price text
CashLocation = script.Parent.Parent.Parent.Parent.Parent.Parent.leaderstats.Cash.Value		-- Location of the cash in leaderstats
SwordsFolder = script.Parent.Parent.Parent.Parent.Parent.Parent.Swords					-- Folder With all the values of owned swords

function BuyItem()                    									--- Script By Silverblade2005
	print("Button Clicked!")
	if CheckSword == "Sword1" then
		CashLocation = CashLocation - tonumber(SwordPrice)				--- Script By Silverblade2005
		SwordsFolder.Sword1.Value = SwordsFolder.Sword1.Value + 1
		print("Item Purchased!")
	elseif CheckSword == "Sword2" then			
		CashLocation = CashLocation - tonumber(SwordPrice)				--- Script By Silverblade2005
		SwordsFolder.Sword2.Value = SwordsFolder.Sword2.Value + 1
		print("Item Purchased!")
	elseif CheckSword == "Sword3" then
		CashLocation = CashLocation - tonumber(SwordPrice)				--- Script By Silverblade2005
		SwordsFolder.Sword3.Value = SwordsFolder.Sword3.Value + 1
		print("Item Purchased!")
	elseif CheckSword == "Sword4" then
		CashLocation = CashLocation - tonumber(SwordPrice)				--- Script By Silverblade2005
		SwordsFolder.Sword4.Value = SwordsFolder.Sword4.Value + 1
		print("Item Purchased!")
	elseif CheckSword == "Sword5" then
		CashLocation = CashLocation - tonumber(SwordPrice)				--- Script By Silverblade2005
		SwordsFolder.Sword5.Value = SwordsFolder.Sword5.Value + 1
		print("Item Purchased")
	end
end

script.Parent.MouseButton1Down:Connect(BuyItem)

And thank you for your help!

2

You could use a table instead of constantly doing if checks.
example:

local Table = {["Sword1"] = 100 [Some price]}
local Sword = Table[Checksword] -- Checksword = "Sword1"
if Sword  then -- Instead of doing if Table[Checksword], you can use variables to prevent multiple table lookups
    CashLocation = CashLocation - Sword -- Sword is a price
end

You should also localize your variables; your scripts should theoretically “run” faster as a result.
example:

local Variable = ...
local function BuyItem() ...

Instead of using values, you could use a module script or a table in your current script to store values, prices, etc.

I’m going to assume this is all in a local script, if so, you should do all in game transactions and checks on the server (to prevent exploiters from taking advantage of vulnerabilities), you can do this through the use of Remote Events.
If you would like to learn about how to utilise Remote Events I recommend having a read through some tutorials on the Roblox Wiki.
Client-Server Runtime | Documentation - Roblox Creator Hub [Recommended]
Custom Events and Callbacks | Documentation - Roblox Creator Hub [Recommended]

4 Likes
3

Im kind of new to module scripts but ill give it a shot thx!

4

So can I put all the variable im going to use in the game in one module script or is it going to be inefficient?

5

I wouldn’t recommend putting all your variables in one module script but, table values or, instead of using value instances, you could put them in a module script

6

Okay thank you for your help! Much appreciated.