Scripting HelpersARCHIVE
A little piece of Roblox history. Preserved for everyone, one question at a time.Read about the archive →
✓ ANSWER ACCEPTED

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

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

✓ Accepted answer

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)

3 comments

0

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

0

Nevermind, figured it out. Had to add a wait after the while true do.

0

Oops, I fixed it a different way. I meant to stop the loop when source was nil

This discussion is preserved as it was. New answers, comments, and votes are closed.