v.Head.Touched:connect(function(hit) local player = game.Players:GetPlayerFromCharacter(hit.Parent) if v.Head.CanCollide == true then if player ~= nil then if script.Parent.Owner.Value == player then if hit.Parent:FindFirstChild("Humanoid") then if hit.Parent.Humanoid.Health > 0 then local cashmoney = game.ServerStorage.MoneyStorage:FindFirstChild(player.Name) if cashmoney ~= nil then if cashmoney.Value >= v.Price.Value then cashmoney.Value = cashmoney.Value - v.Price.Value objects[v.Object.Value].Parent = script.Parent.PurchasedObjects player.BuySound:Play() if config.ButtonExplodeOnBuy.Value == true then local explosion = Instance.new("Explosion",workspace) explosion.Position = v.Head.Position explosion.DestroyJointRadiusPercent = 0 explosion.BlastPressure = 0 end if config.ButtonFadeOutOnBuy.Value == true then v.Head.CanCollide = false coroutine.resume(coroutine.create(function() for i=1,20 do wait(config.ButtonFadeOutTime.Value/20) v.Head.Transparency = v.Head.Transparency + 0.05 end end)) else v.Head.CanCollide = false v.Head.Transparency = 1 end end end end end end end end end) end end
Everything works here, but my mind's confused where do i put these 2 lines: if cashmoney.Value <= v.Price.Value then -- this line when player doesn't have enough money player.NotEnough:Play() -- make it play a sound
problem is where i put it in the script ? do i need an else in it somewhere or ? the script works good once again i just need to insert these 2 lines
P.S the script's too long so open it in a new page
THANKS! :D
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.
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)
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 -- do stuff! 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 -- do stuff! 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
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
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
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