Long shop script I made: Any way to improve?

In the game that I’ve been making, I’ve got a shop script that is over 500 lines long, and I just want to know if there is anything within the code that could be better written or just more readable. I’m especially concerned about the OnPurchaseButtonPressed() function as I’m calling a DataStore event several times. Just overall would be nice if I could get some feedback on the code since I have never written such a long script before.

--{Services}--
local ReplicatedStorage = game:GetService("ReplicatedStorage")
local TweenService = game:GetService("TweenService")

--{Variables}--
local player = game.Players.LocalPlayer
local playerData = {}

local mouse = player:GetMouse()
local PlayerGui = player:WaitForChild("PlayerGui")

local ShopHolder = PlayerGui.ShopGui.AspectRatioHolder.Holder
local ShopEvent = ReplicatedStorage.Events.ShopEvent
local XButton = ShopHolder.XButton

local DataStore = ReplicatedStorage.Events.DataStoreClient
local ToolEvent = ReplicatedStorage.Events.ToolEvent
local ConnectionType = nil
local VisibleInformationFrame = nil

--Main Option Buttons--
local PurchaseOptionsHolder = ShopHolder.PurchaseOptionsHolder
local HandleOptionButton = PurchaseOptionsHolder.HandleOptionButton
local PickOptionButton = PurchaseOptionsHolder.PickOptionButton
local StorageOptionButton = PurchaseOptionsHolder.StorageOptionButton

--Option Frames--
local SubOptionsHolder = ShopHolder.SubOptionsHolder
local StorageFrameHolder = SubOptionsHolder.StorageFrameHolder
local PicksScrollingFrame = SubOptionsHolder.PicksScrollingFrame
local HandlesScrollingFrame = SubOptionsHolder.HandlesScrollingFrame 

--Information Frame Variables--
local InformationDisplayHolder = ShopHolder.InformationDisplayHolder
local InformationFramesFolder = InformationDisplayHolder.InformationFrames
local EquipButton = InformationDisplayHolder.EquipButton
local PurchaseButton = InformationDisplayHolder.PurchaseButton

--Colors--
local NotEquippedColor = Color3.fromRGB(88, 227, 37)
local NotEquippedBorderColor = Color3.fromRGB(43, 117, 38)

local EquippedColor = Color3.fromRGB(240, 178, 35)
local EquippedBorderColor = Color3.fromRGB(166, 121, 24)

local InsufficientCashColor = Color3.fromRGB(230, 44, 7)
local InsufficientCashBorderColor = Color3.fromRGB(112, 20, 4)

local DarkerGreen = Color3.fromRGB(36, 145, 36)
local DarkerRed = Color3.fromRGB(130, 0, 0)

--Other Variables--
local IsOnShopScreen = false

local HasOreRequirements = 0
local BlocksToDelete = {}

local InfoTween = TweenInfo.new(0.5, Enum.EasingStyle.Cubic, Enum.EasingDirection.Out)
local UpgradeStorageButton = StorageFrameHolder.UpgradeButton

local StorageLevels = require(ReplicatedStorage.Modules.StorageLevels)
local ShopModule = require(ReplicatedStorage.Modules.ShopModule)
local Inventory = require(ReplicatedStorage.Modules.Inventory)

--{Functions}--
local function SetStorageLabels()
	local StorageLevel = playerData.StorageLevel
	local Storage = StorageLevels[string.format("Level%d", StorageLevel)][1]
	local NextLevelStorage = StorageLevels[string.format("Level%d", StorageLevel + 1)][1]
	local NextLevelPrice = StorageLevels[string.format("Level%d", StorageLevel + 1)][2]

	--Setting Up Storage labels--
	StorageFrameHolder.CurrentLevelTextLabel.Text = string.format("Level %d", StorageLevel)
	StorageFrameHolder.NextLevelTextLabel.Text = string.format("Level %d", StorageLevel + 1)
	StorageFrameHolder.CurrentLevelStorageTextLabel.Text = string.format("%d", Storage)
	StorageFrameHolder.NextLevelStorageTextLabel.Text = string.format("%d", NextLevelStorage)
	StorageFrameHolder.PriceTextLabel.Text = string.format("$%d", NextLevelPrice)

	local playerCash = playerData.Cash
	if playerCash >= NextLevelPrice then
		UpgradeStorageButton.BackgroundColor3 = NotEquippedColor
		UpgradeStorageButton.BorderColor3 = NotEquippedBorderColor

	elseif playerCash < NextLevelPrice then
		UpgradeStorageButton.BackgroundColor3 = InsufficientCashColor
		UpgradeStorageButton.BorderColor3 = InsufficientCashBorderColor
	end
end

local function MakeAllInformationFramesInvisible()
	for _, infoFrame in ipairs(InformationFramesFolder:GetChildren()) do
		infoFrame.Visible = false
	end
	
	VisibleInformationFrame = nil
end

local function CheckForStorageAction(Action)
	if Action == "LoadedData" or Action == "StorageModification" or Action ~= "StorageUpgrade" then
		SetStorageLabels()
	end
end

local function StorePlayerDataAsVariable(Data, Action)
	if not Data then return end
	if Action ~= "LoadedData" and Action ~= "UpdatingPlayerData" and Action ~= "StorageModification" and Action ~= "StorageUpgrade" then return end
	
	playerData = Data 
	
	if Action == "LoadedData" then
		ToolEvent:FireServer(playerData, "ToolUpdate")
	end
	
	--Since Data is being loaded, we mind as well set up storage labels and stuff like that--
	CheckForStorageAction()
end

local function SetDefaultVisibilityValues()
	StorageFrameHolder.Visible = false
	PicksScrollingFrame.Visible = false
	HandlesScrollingFrame.Visible = false
	
	MakeAllInformationFramesInvisible()
	
	EquipButton.Visible = false
	PurchaseButton.Visible = false
end

local function OpenShopScreen()
	if IsOnShopScreen == false then
		IsOnShopScreen = true
		ConnectionType = nil
		SetDefaultVisibilityValues()

		--Tween Portion--
		ShopHolder.Size = UDim2.new(1, 0, 0, 0)
		ShopHolder.Visible = true

		--Create and play Tween--
		local TweenGoal = {Size = UDim2.new(1, 0, 1, 0)}
		local Tween = TweenService:Create(ShopHolder, InfoTween, TweenGoal)
		Tween:Play()
	end
end

local function OpenOptionFrame(FrameToOpen: string)
	ConnectionType = nil
	EquipButton.Visible = false
	PurchaseButton.Visible = false
	
	MakeAllInformationFramesInvisible()
	
	for _, frame in ipairs(SubOptionsHolder:GetChildren()) do
		
		local OptionContent = frame:GetAttribute("OptionContent")
		
		if OptionContent ~= nil then
			if OptionContent == FrameToOpen then
				frame.Visible = true
				
				--For Updating Storage Labels--
				if OptionContent == "Storage" then
					SetStorageLabels()
				end
			
			else
				frame.Visible = false
			end
		end
	end
end

local function CheckOreRequirements(ComponentOreRequirements)
	local HasRequirements = 0 -- If this number remains zero after checking for blocks then that means there were no ore requirements
	BlocksToDelete = {} -- Layout: {The index to search, #OfBlocks}
	local playerInventory
	
	if ComponentOreRequirements ~= nil then
		playerInventory = Inventory.FetchInventory()
		local OreRequirementInfo = {}

		local InfoLength = 0
		local AccquiredMaterials = 0

		--Create RequirementInfo--
		for oreRequirement, quantity in pairs(ComponentOreRequirements) do
			OreRequirementInfo[oreRequirement] = false -- boolean value is for whether the player has enough of that material or not
			InfoLength += 1
		end

		--Fill Dictionary out--
		for index, itemArray in ipairs(playerInventory) do
			for oreRequirement, quantity in pairs(ComponentOreRequirements) do
				if string.format("%sOre", itemArray[1]) == oreRequirement and itemArray[2] >= quantity then
					OreRequirementInfo[oreRequirement] = true

					table.insert(BlocksToDelete, {index, quantity})
				end
			end
		end

		for oreRequirement, HasSufficientMaterials in pairs(OreRequirementInfo) do
			if HasSufficientMaterials == true then 
				AccquiredMaterials += 1 
				
				for _, TextLabel in ipairs(VisibleInformationFrame.OreRequirementsHolder:GetChildren()) do
					local OreType = TextLabel:GetAttribute("OreType")
					
					if OreType and string.format("%sOre", OreType) == oreRequirement then
						TextLabel.TextColor3 = DarkerGreen
						break
					end
				end
				
			else
				for _, TextLabel in ipairs(VisibleInformationFrame.OreRequirementsHolder:GetChildren()) do
					local OreType = TextLabel:GetAttribute("OreType")

					if OreType and string.format("%sOre", OreType) == oreRequirement then
						TextLabel.TextColor3 = DarkerRed
						break
					end
				end
			end
		end
		if AccquiredMaterials == InfoLength then
			HasRequirements = 2 -- Has all needed blocks

		else
			HasRequirements = 1 -- Does not have all needed blocks
		end
	end
	
	return HasRequirements
end

local function OnMouseClick()
	if IsOnShopScreen == false then return end
	
	local Button = nil
	
	--Make selected Information Frame Visible--
	local GuiObjects = PlayerGui:GetGuiObjectsAtPosition(mouse.X, mouse.Y)

	for _, Gui in pairs(GuiObjects) do
		if Gui:IsA("Frame") and Gui:GetAttribute("StringConnection") and Gui.Parent.Parent == SubOptionsHolder then
			local Connection: string = Gui:GetAttribute("StringConnection")
			Button = Gui
			
			for _, infoFrame in ipairs(InformationFramesFolder:GetChildren()) do
				if infoFrame:GetAttribute("StringConnection") == Connection then
					ConnectionType = Connection
					VisibleInformationFrame = infoFrame
					
					infoFrame.Visible = true
					
				else
					infoFrame.Visible = false
				end
			end
		end
	end
	
	--Declare Button State--
	if Button == nil then return end
	local Option = Button.Parent:GetAttribute("OptionContent")
	
	local CurrentlyEquipped
	local PurchasedList
	
	if Option == "Picks" then
		CurrentlyEquipped = playerData.CurrentEquippedPick
		PurchasedList = playerData.PurchasedPicksData
	
	elseif Option == "Handles" then
		CurrentlyEquipped = playerData.CurrentEquippedHandle
		PurchasedList = playerData.PurchasedHandlesData
		
	else
		return
	end
	
	--Check if the component needs to be purchased or if it can be equipped + Change Button State--
	local ComponentPrice: number = ShopModule[VisibleInformationFrame:GetAttribute("StringConnection")][1]
	local playerCash: number = playerData.Cash
	
	if table.find(PurchasedList, VisibleInformationFrame:GetAttribute("StringConnection")) then
		EquipButton.Visible = true
		PurchaseButton.Visible = false
		
		if ConnectionType == nil then return end
		if ConnectionType == CurrentlyEquipped then
			EquipButton.Text = "Equipped"
			EquipButton.BackgroundColor3 = EquippedColor
			EquipButton.BorderColor3 = EquippedBorderColor

		else
			EquipButton.Text = "Equip"
			EquipButton.BackgroundColor3 = NotEquippedColor
			EquipButton.BorderColor3 = NotEquippedBorderColor
		end
		
	else
		PurchaseButton.Visible = true
		EquipButton.Visible = false
		
		--Check if player has required Materials--
		local VisibleComponent = VisibleInformationFrame:GetAttribute("StringConnection")
		local ComponentOreRequirements = nil
		
		if #ShopModule[VisibleComponent] > 1 then
			ComponentOreRequirements = ShopModule[VisibleComponent][2]
		end
		
		HasOreRequirements = CheckOreRequirements(ComponentOreRequirements)
		local PriceLabel = VisibleInformationFrame.PriceLabel
		
		--Declare Purchase Button State--
		if ComponentPrice <= playerCash and HasOreRequirements ~= 1 then
			PriceLabel.TextColor3 = DarkerGreen
			
			PurchaseButton.BackgroundColor3 = NotEquippedColor
			PurchaseButton.BorderColor3 = NotEquippedBorderColor
			
		else
			if ComponentPrice <= playerCash then PriceLabel.TextColor3 = DarkerGreen else PriceLabel.TextColor3 = DarkerRed end
			
			PurchaseButton.BackgroundColor3 = InsufficientCashColor
			PurchaseButton.BorderColor3 = InsufficientCashBorderColor
		end
		
		VisibleInformationFrame.PriceLabel.Text = string.format("$%d", ComponentPrice)
	end
end

local function OnEquipButtonPressed()
	if EquipButton.Text == "Equipped" then
		return
			
	elseif EquipButton.Text == "Equip" then
		local ComponentToEquip: string
		local EquipmentToChange: string
		
		--Find Currently Visible Information Frame--
		for _, infoFrame in ipairs(InformationFramesFolder:GetChildren()) do
			if infoFrame.Visible then
				ComponentToEquip = infoFrame:GetAttribute("StringConnection")
				break
			end
		end
		
		if ComponentToEquip == nil then return end
		
		--Find which Equipment to modify--
		for _, frame in ipairs(SubOptionsHolder:GetChildren()) do
			for _, button in ipairs(frame:GetChildren()) do
				if button:GetAttribute("StringConnection") and button:GetAttribute("StringConnection") == ComponentToEquip then
					EquipmentToChange = button.Parent:GetAttribute("OptionContent")
					break
				end
			end
		end
		
		if EquipmentToChange == nil or EquipmentToChange == "Storage" then return end
		
		--Find which Equipment is being updated--
		if EquipmentToChange == "Picks" then
			playerData.CurrentEquippedPick = ComponentToEquip
			DataStore:FireServer("UpdateData", "UpdateEquippedPick", playerData)
			
		elseif EquipmentToChange == "Handles" then
			playerData.CurrentEquippedHandle = ComponentToEquip
			DataStore:FireServer("UpdateData", "UpdateEquippedHandle", playerData)
		end
		
		local PrevData = playerData
		--If Everything was Successful:
		
		--Change Equip Button State--
		EquipButton.Text = "Equipped"
		EquipButton.BackgroundColor3 = EquippedColor
		EquipButton.BorderColor3 = EquippedBorderColor
		
		--Events--
		DataStore:FireServer("GetData", "UpdatingPlayerData") -- Update playerData Variable
		repeat task.wait() until PrevData ~= playerData
		
		ToolEvent:FireServer(playerData, "ToolUpdate")
	end
end

local function OnPurchaseButtonPressed()
	if VisibleInformationFrame == nil then return end -- If Purchase Button was pressed when nothing is supposed to be purchased (shouldn't be possible)
	
	local playerCash = playerData.Cash
	local playerInventory
	
	local ComponentToPurchase = VisibleInformationFrame:GetAttribute("StringConnection")
	local ComponentPrice = ShopModule[ComponentToPurchase][1]
	
	local OptionType
	local DataArrayToSearch
	
	--Check if Component is purchasable--
	if playerCash >= ComponentPrice and HasOreRequirements ~= 1 then
		local playerInventory = Inventory.FetchInventory()
		
		for _, frame in ipairs(SubOptionsHolder:GetChildren()) do
			for _, button in ipairs(frame:GetChildren()) do
				if button:GetAttribute("StringConnection") and button:GetAttribute("StringConnection") == ComponentToPurchase then
					OptionType = button.Parent:GetAttribute("OptionContent")
					break
				end
			end
		end
		
		if HasOreRequirements == 2 and #BlocksToDelete > 0 then
			for _, array in ipairs(BlocksToDelete) do
				local PlayerBlockAmount = playerInventory[array[1]][2]

				if PlayerBlockAmount == array[2] then
					Inventory.Remove(player, playerInventory[array[1]][1])

				elseif PlayerBlockAmount > array[2] then
					Inventory.Increment(player, playerInventory[array[1]][1], -array[2])
				end
			end
		end
		
		if OptionType == nil or OptionType == "Storage" then return end
		local PrevData = playerData
		
		if OptionType == "Picks" then
			table.insert(playerData.PurchasedPicksData, ComponentToPurchase)
			playerData.CurrentEquippedPick = ComponentToPurchase
			
			DataStore:FireServer("UpdateData", "UpdatePurchasedPicksData", playerData)
			
		elseif OptionType == "Handles" then
			table.insert(playerData.PurchasedHandlesData, ComponentToPurchase)
			playerData.CurrentEquippedHandle = ComponentToPurchase
			
			DataStore:FireServer("UpdateData", "UpdatePurchasedHandlesData", playerData)
		end
		
		playerData.Cash -= ComponentPrice
		DataStore:FireServer("UpdateData", "UpdateCash", playerData)
		
		--Automaticly Equip Tool after Purchase--
		PurchaseButton.Visible = false
		EquipButton.Visible = true
		
		EquipButton.Text = "Equipped"
		EquipButton.BackgroundColor3 = EquippedColor
		EquipButton.BorderColor3 = EquippedBorderColor
		
		DataStore:FireServer("GetData", "UpdatingPlayerData") -- Update playerData Variable
		repeat task.wait() until PrevData ~= playerData
		
		DataStore:FireServer("GetData", "SetCashDisplay")
		ToolEvent:FireServer(playerData, "ToolUpdate")
	end
end

local function OnStorageUpgradeButtonPressed()
	local playerCash = playerData.Cash
	local StorageLevel = playerData.StorageLevel
	
	local NextLevelStoragePrice = StorageLevels[string.format("Level%d", StorageLevel + 1)][2]
	
	--Check if user can purchase Next Level Storage--
	if playerCash >= NextLevelStoragePrice then
		playerData.Cash -= NextLevelStoragePrice
		playerData.StorageLevel += 1
		
		DataStore:FireServer("UpdateData", "StorageUpgrade", playerData)
		SetStorageLabels()
		
		DataStore:FireServer("GetData", "SetCashDisplay")
		DataStore:FireServer("GetData", "SetStorageDisplay")
	end
end

local function OnXButtonPressed()
	ShopHolder.Visible = false
	ConnectionType = nil
	
	task.wait(1) -- debounce
	IsOnShopScreen = false
end

--{Signals}--
DataStore.OnClientEvent:Connect(StorePlayerDataAsVariable)

ShopEvent.OnClientEvent:Connect(OpenShopScreen)
XButton.MouseButton1Click:Connect(OnXButtonPressed)
mouse.Button1Down:Connect(OnMouseClick)

EquipButton.MouseButton1Click:Connect(OnEquipButtonPressed)
PurchaseButton.MouseButton1Click:Connect(OnPurchaseButtonPressed)
UpgradeStorageButton.MouseButton1Click:Connect(OnStorageUpgradeButtonPressed)

--Main Button Signals--
PickOptionButton.MouseButton1Click:Connect(function()
	OpenOptionFrame("Picks")
end)
HandleOptionButton.MouseButton1Click:Connect(function()
	OpenOptionFrame("Handles")
end)
StorageOptionButton.MouseButton1Click:Connect(function()
	OpenOptionFrame("Storage")
end)
4 Likes

I think having an overall update function would be good since you keep passing in the same data but with different tags, then the complexity would decrease. It could also benefit from the use of more module scripts to break it down. I see you have check ore requirements which should probably be a separate function from that script.

2 Likes