Scripting Helpers is winding down operations and is now read-only. More info→
Ad
Log in to vote
-1

Purchasing script ?

Asked by
Bulvyte 388 Moderation Voter
7 years ago
Edited 7 years ago
        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

1 answer

Log in to vote
1
Answered by
BlueTaslem 18071 Moderation Voter Administrator Community Moderator Super Administrator
7 years ago

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
            -- 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

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
0
Yes, yes thank you this is what i was looking for!! :) Bulvyte 388 — 7y
0
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. Bulvyte 388 — 7y
Ad

Answer this question