Scripting Helpers is winding down operations and is now read-only. More info→

Is there an easy way to condense or loop this code, I feel like it should be simple?

Asked by

6 years ago

I have been coding for a while now but one of the main things I struggle with is repeating a lot of stuff in the code rather than trying to make it less work for me to type. Here is an example:

local part = script.Parent

local seq = 1

part.Changed:connect(function(property)
    local text = script.Parent:FindFirstChild(seq).Top.Value
    local count = string.len(text)
    for i = 1, count, 1 do
            script.Parent.Parent.Parent.Text.TextLineOne.Text = string.sub(text, 1, i)
            wait(0.1)
    end
    local text2 = script.Parent:FindFirstChild(seq).Bottom.Value
    local count2 = string.len(text2)
    for i = 1, count2, 1 do
            script.Parent.Parent.Parent.Text.TextLineTwo.Text = string.sub(text2, 1, i)
            wait(0.1)

    end -- (NEW ONE BELOW)

    if part:FindFirstChild(seq+1) ~= nil then
        seq = seq + 1
        local text = script.Parent:FindFirstChild(seq).Top.Value
    local count = string.len(text)
    for i = 1, count, 1 do
            script.Parent.Parent.Parent.Text.TextLineOne.Text = string.sub(text, 1, i)
            wait(0.1)
    end
    local text2 = script.Parent:FindFirstChild(seq).Bottom.Value
    local count2 = string.len(text2)
    for i = 1, count2, 1 do
            script.Parent.Parent.Parent.Text.TextLineTwo.Text = string.sub(text2, 1, i)
            wait(0.1)
    end 
    else
    end
end)

This wouldn't even be all of it if I didn't condense it. I'm trying to make a dialog system that appears clean. I don't want to keep repeating the same code over and over when all I am doing is changing one value. Please help!

1 answer

Answered by

BlueTaslem 18071

6 years ago

Edited 6 years ago

First, space and indent your code properly.

You can remove the empty else at the end.

, 1 at the end of a for loop is unnecessary (the step is implicitly 1).

Instead of string.sub(text, 1, i) you can write text:sub(1, i).

You can use #text instead of string.len(text). This is short enough you can put it right into the for i = 1, #text do to shorten it more.

~= nil is unnecessary after :FindFirstChild().

local part = script.Parent

local seq = 1

part.Changed:connect(function(property)
    local text = script.Parent:FindFirstChild(seq).Top.Value
    for i = 1, #text do
        script.Parent.Parent.Parent.Text.TextLineOne.Text = text:sub(1, i)
        wait(0.1)
    end
    local text2 = script.Parent:FindFirstChild(seq).Bottom.Value
    for i = 1, #text2 do
        script.Parent.Parent.Parent.Text.TextLineTwo.Text = text2:sub(1, i)
        wait(0.1)
    end

    -- (NEW ONE BELOW)

    if part:FindFirstChild(seq + 1) then
        seq = seq + 1
        local text = script.Parent:FindFirstChild(seq).Top.Value
        for i = 1, #text do
            script.Parent.Parent.Parent.Text.TextLineOne.Text = text:sub(1, i)
            wait(0.1)
        end
        local text2 = script.Parent:FindFirstChild(seq).Bottom.Value
        for i = 1, #text2 do
            script.Parent.Parent.Parent.Text.TextLineTwo.Text = text2:sub(1, i)
            wait(0.1)
        end
    end
end)

There is a repeated chunk of code for typing out a message into a GUI. You can extract that repeated chunk into a function.

local part = script.Parent

local seq = 1

function typeout(gui, text)
    for i = 1, #text do
        gui.Text = text:sub(1, i)
        wait(0.1)
    end
end

part.Changed:connect(function(property)
    local text = script.Parent:FindFirstChild(seq).Top.Value
    typeout(script.Parent.Parent.Parent.Text.TextLineOne, text)

    local text2 = script.Parent:FindFirstChild(seq).Bottom.Value
    typeout(script.Parent.Parent.Parent.Text.TextLineTwo, text2)

    -- (NEW ONE BELOW)

    if part:FindFirstChild(seq + 1) then
        seq = seq + 1
        local text = script.Parent:FindFirstChild(seq).Top.Value
        typeout(script.Parent.Parent.Parent.Text.TextLineOne, text)

        local text2 = script.Parent:FindFirstChild(seq).Bottom.Value
        typeout(script.Parent.Parent.Parent.Text.TextLineTwo, text2)
    end
end)

script.Parent.Parent.Parent is not very enlightening; I would suggest making a shorter name for it. You can already shorten it to part.Parent.Parent, but most likely you can give that a better name.

It seems like seq shouldn't really persist between invocations of Changed. Do you want to cycle through all of the messages in-order inside script.Parent?

local part = script.Parent

function typeout(gui, text)
    for i = 1, #text do
        gui.Text = text:sub(1, i)
        wait(0.1)
    end
end

part.Changed:connect(function(property)
    local seq = 1

    while true do
        local source = script.Parent:FindFirstChild(seq)
        if not source then
            return
        end
        local text = source.Top.Value
        typeout(script.Parent.Parent.Parent.Text.TextLineOne, text)

        local text2 = source.Bottom.Value
        typeout(script.Parent.Parent.Parent.Text.TextLineTwo, text2)

        seq = seq + 1
    end
end)

This works great, except the game freezes after the last line is played. How would I fix that? Dad_Bot 34 — 6y

Nevermind, figured it out. Had to add a wait after the while true do. Dad_Bot 34 — 6y

Oops, I fixed it a different way. I meant to stop the loop when `source` was `nil` BlueTaslem 18071 — 6y

Is there an easy way to condense or loop this code, I feel like it should be simple?

1 answer

Answer this question