How could I improve this?

Help and Feedback Scripting Support
#1

Hello everyone,

I have this shop code and it works fine. However, it’s quite messy. How would I simplify it?

if tonumber(Amount.Text) and Storage.Value >= tonumber(Amount.Text) and Gold.Value >= (tonumber(Amount.Text) * 5) then
		event:FireServer(tonumber(Amount.Text),"Buying")
		Amount.Text = ""
	elseif tonumber(Amount.Text) == nil then
		Amount.Text = ""
		TextLabel.TextColor3 = Color3.fromRGB(255,0,0)		
		TextLabel.Text = "Numbers can only be enterd in this box"
		button.Active = false
		wait(2)
		TextLabel.TextColor3 = Color3.fromRGB(255,255,255)
		TextLabel.Text = "Type The Amount of stone You want to buy:"
		button.Active = true	
	elseif Storage.Value < tonumber(Amount.Text) and Gold.Value >= (tonumber(Amount.Text) * 5) then
		Amount.Text = ""
		TextLabel.TextColor3 = Color3.fromRGB(255,0,0)		
		TextLabel.Text = "The amount of stone you orderd isn't avalibale"
		button.Active = false
		wait(2)
		TextLabel.TextColor3 = Color3.fromRGB(255,255,255)
		TextLabel.Text = "Type The Amount of stone You want to buy:"
		button.Active = true	
	elseif Gold.Value < (tonumber(Amount.Text) * 5) then
		Amount.Text = ""
		TextLabel.TextColor3 = Color3.fromRGB(255,0,0)		
		TextLabel.Text = "You don't have enough Gold to buy that amount"
		button.Active = false
		wait(2)
		TextLabel.TextColor3 = Color3.fromRGB(255,255,255)
		TextLabel.Text = "Type The Amount of stone You want to buy:"
		button.Active = true

thank you

1 Like
#2

Put the texts and colors into a table each, and have a function that can take that table, and it sets to the values of the table, and after the two seconds, resets. That way you could just call that function in each if/elseif in your setup, instead of needing to copy-paste.

You can also do it without tables, and make the function’s parameters the different values it needs to know, too.

#4

Not in any way that would make sense, since each elseif you’re using has different criteria. You can use a single script instead of a script in each individual button though, and that would make less to have to manage.

1 Like
#5 
local function Message(text)
    button.Active = false
    TextLabel.TextColor3 = Color3.fromRGB(255,0,0)		
    TextLabel.Text = text
    task.wait(2)
    TextLabel.TextColor3 = Color3.fromRGB(255,255,255)
    TextLabel.Text = "Type The Amount of stone You want to buy:"
    button.Active = true	
end

local function myFunction()
    local amount = tonumber(Amount.Text)
    Amount.Text = ""

    if amount == nil then
        Message("Numbers can only be enterd in this box")
        return
    end

    if Storage.Value < amount then
        Message("The amount of stone you orderd isn't avalibale")
        return
    end

    if Gold.Value < amount * 5 then
        Message("You don't have enough Gold to buy that amount")
        return
    end

    event:FireServer(amount, "Buying")
end
1 Like