Clean code reads like well-written prose
P.S the script's too long so open it in a new page
This is a sign that you need to fix this. Long lines and deeply nested blocks make your code impossible to understand. Separate your code into functions.
Pull out the event connection to save indentation and separate code
I see that the code you have included already is two levels deep. This is why I am against using anonymous functions in connections. Create a function and meaningfully name it. That includes avoiding a meaningless name like v:
function buttonPressed(button, hit)
local player = game.Players:GetPlayerFromCharacter(hit.Parent)
......
end
......
v.Head.Touched:connect(function(hit) buttonPressed(v, hit) end)
Use early returns to save indentation
Now we can save a bunch of levels of indentation by using an early return. Instead of code that looks like this:
function buttonPressed(button, hit)
local player = game.Players:GetPlayerFromCharacter(hit.Parent)
if button.Head.CanCollide == true then
if player ~= nil then
end
end
end
You just quit if those things aren't true:
function buttonPressed(button, hit)
local player = game.Players:GetPlayerFromCharacter(hit.Parent)
if not button.Head.CanCollide then
return
elseif not player then
return
end
end
Also, don't use things like ~= nil or == true. It's redundant and not idiomatic -- it just adds noise to your code.
The function now looks like this:
function buttonPressed(button, hit)
local player = game.Players:GetPlayerFromCharacter(hit.Parent)
if not button.Head.CanCollide then
return
elseif not player then
return
elseif script.Parent.Owner.Value ~= player then
return
elseif not hit.Parent:FindFirstChild("Humanoid") then
return
elseif hit.Parent.Humanoid.Health == 0 then
return
end
local cashmoney = game.ServerStorage.MoneyStorage:FindFirstChild(player.Name)
local cashmoney = game.ServerStorage.MoneyStorage:FindFirstChild(player.Name)
if not cashmoney then
return
end
if cashmoney.Value >= button.Price.Value then
......
......
end
Pull out irrelevant code into functions
The buttonPressed code, after those early returns, isn't really dealing with button pressing anymore. The rest of the code is attempting a purchase, so why not break that code out into attemptPurchase?
That makes your buttonPressed code finish up like this:
function buttonPressed(button, hit)
local player = game.Players:GetPlayerFromCharacter(hit.Parent)
if not button.Head.CanCollide then
return
elseif not player then
return
elseif script.Parent.Owner.Value ~= player then
return
elseif not hit.Parent:FindFirstChild("Humanoid") then
return
elseif hit.Parent.Humanoid.Health == 0 then
return
end
local cashmoney = game.ServerStorage.MoneyStorage:FindFirstChild(player.Name)
if not cashmoney then
return
end
attemptPurchase(player, cashmoney, button)
end
Pulling other stuff out of attemptPurchase
It probably makes sense to pull out the two cosmetic things the attemptPurchase function is doing; fading out a brick and making an explosion.
Again, don't use == true.
Don't use coroutine.resume(coroutine.create( to just start a function. If you aren't using coroutine.yield, you have no business using coroutine at all.
Use spawn:
function fadeOut(part)
spawn(function()
for i=1,20 do
wait(config.ButtonFadeOutTime.Value/20)
part.Transparency = part.Transparency + 0.05
end
end)
end
You set the .CanCollide in both the if and the else, which is repeating yourself.
function attemptPurchase(player, cash, button)
if cash.Value >= button.Price.Value then
cash.Value = cash.Value - button.Price.Value
objects[button.Object.Value].Parent = script.Parent.PurchasedObjects
player.BuySound:Play()
if config.ButtonExplodeOnBuy.Value then
explode(button.Head.Position)
end
button.Head.CanCollide = false
if config.ButtonFadeOutOnBuy.Value then
fadeOut(button.Head)
else
button.Head.Transparency = 1
end
end
end
Your question
You want to modify what happens when dealing with money.
Because of the now split-up design, we know we can go directly to attemptPurchase, since no where else actually deals with an amount of money.
Reading the first line of that function, if cash.Value >= button.Price.Value then, we see that the whole thing of the script only happens if they have enough money.
To make something happen when they don't, just stick an else onto that condition:
if cash.Value >= button.Price.Value then
......
else
player.NotEnough:Play()
end
2 comments
Yes, yes thank you this is what i was looking for!! :)
Problem is now it plays the sound spamming it, how do i prevent it ? making a debounce will make a blue line and will not work.