Simplifying The Code, Advice?

Everything within is functional. I just know there is a better way to simplify it and I don’t have the slightest clue. Using For and meta tables are an example I am interested in.

I have tried the meta tables myself and For statements but It ends up breaking. I’ve spent already about 3 hours wishing I hadn’t even bothered. Otherwise, I am learning rather well with code. I most likely will be back many, many times for help. Otherwise, ill leave it is is but I rather do things efficiently too.

local triggered = false
local RightGroup = script.Parent.Right
local LeftGroup = script.Parent.Straight
local Dright = script.Parent.DRight
local Dleft = script.Parent.DLeft

function click()
	if triggered == true then
		triggered = false
		RightGroup.One.Transparency = 1
		RightGroup.Two.Transparency = 1
		RightGroup.Three.Transparency = 1
		RightGroup.Four.Transparency = 1
		RightGroup.Five.Transparency = 1
		LeftGroup.One.Transparency = 0
		LeftGroup.One.CanCollide = true
		RightGroup.One.CanCollide = false
		RightGroup.Two.CanCollide = false
		RightGroup.Three.CanCollide = false
		RightGroup.Four.CanCollide = false
		RightGroup.Five.CanCollide = false
	else
		triggered = true
		RightGroup.One.Transparency = 0
		RightGroup.Two.Transparency = 0
		RightGroup.Three.Transparency = 0
		RightGroup.Four.Transparency = 0
		RightGroup.Five.Transparency = 0
		LeftGroup.One.Transparency = 1
		LeftGroup.One.CanCollide = false
		RightGroup.One.CanCollide = true
		RightGroup.Two.CanCollide = true
		RightGroup.Three.CanCollide = true
		RightGroup.Four.CanCollide = true
		RightGroup.Five.CanCollide = true
	end
end

Dright.Touched:Connect(function(TrainComing)
	if TrainComing.Parent:FindFirstChild("DetectionPart") then
		triggered = true
		RightGroup.One.Transparency = 0
		RightGroup.Two.Transparency = 0
		RightGroup.Three.Transparency = 0
		RightGroup.Four.Transparency = 0
		RightGroup.Five.Transparency = 0
		LeftGroup.One.Transparency = 1
		LeftGroup.One.CanCollide = false
		RightGroup.One.CanCollide = true
		RightGroup.Two.CanCollide = true
		RightGroup.Three.CanCollide = true
		RightGroup.Four.CanCollide = true
		RightGroup.Five.CanCollide = true
	end
end)
Dleft.Touched:Connect(function(TrainComing2)
	if TrainComing2.Parent:FindFirstChild("DetectionPart") then
		
		triggered = false
		RightGroup.One.Transparency = 1
		RightGroup.Two.Transparency = 1
		RightGroup.Three.Transparency = 1
		RightGroup.Four.Transparency = 1
		RightGroup.Five.Transparency = 1
		LeftGroup.One.Transparency = 0
		LeftGroup.One.CanCollide = true
		RightGroup.One.CanCollide = false
		RightGroup.Two.CanCollide = false
		RightGroup.Three.CanCollide = false
		RightGroup.Four.CanCollide = false
		RightGroup.Five.CanCollide = false
	end
end)
script.Parent.Clicker.ClickDetector.MouseClick:Connect(click)



1 Like

This would be better on #help-and-feedback:code-review

1 Like

What are inside RightGroup?

[dont mind this]

Its two groups for two directions. Its a track switch essentially.

Thanks. Im still new here, I changed it.

1 Like

Guessing that each ‘group’ only has the parts that you want to toggle.

local triggered = false
local RightGroup = script.Parent.Right
local LeftGroup = script.Parent.Straight
local Dright = script.Parent.DRight
local Dleft = script.Parent.DLeft

local function click()
	for _,v: BasePart in ipairs(RightGroup:GetChildren()) do
		if v:IsA("BasePart") then
			v.Transparency = if triggered then 1 else 0
			v.CanCollide = if triggered then false else true
		end
	end
	for _,v: BasePart in ipairs(LeftGroup:GetChildren()) do
		if v:IsA("BasePart") then
			v.Transparency = if triggered then 0 else 1
			v.CanCollide = if triggered then true else false
		end
	end
	
	triggered = not triggered
end

local function touched(trainComing: BasePart, isRight: boolean)
	local part = trainComing.Parent:FindFirstChild("DetectionPart")
	
	if not part then
		return
	end
	
	for _,v: BasePart in ipairs(RightGroup:GetChildren()) do
		if v:IsA("BasePart") then
			v.Transparency = if isRight then 0 else 1
			v.CanCollide = if isRight then true else false
		end
	end
	for _,v: BasePart in ipairs(LeftGroup:GetChildren()) do
		if v:IsA("BasePart") then
			v.Transparency = if isRight then 1 else 0
			v.CanCollide = if isRight then false else true
		end
	end
end

Dright.Touched:Connect(function(TrainComing)
	touched(TrainComing, true)
end)
Dleft.Touched:Connect(function(TrainComing2)
	touched(TrainComing2, false)
end)
script.Parent.Clicker.ClickDetector.MouseClick:Connect(click)
1 Like

So, first things first. I am reading as I am typing this. Yes, Each group has to be toggled differently. It is a Track Turnout for trains. In my case for minetrains for 2 directions. Further examination you have seem to be showing me the way I wanted to do it in, and I never really knew exactly how to implement it. So again, forgive me…

When I see _, v: what is this? is it the same as saying for example for i, in pairs? How does it differ if not? I was using In Pairs before.

I am taking this as a learning opportunity and if you have the time, could you explain what each part of the code is meant to do? More I learn, less I need to pester people with, haha. In the mean time I will give this a try and see how it runs.

1 Like

“_” is considered a variable that you will never use. And yes it is the same as for i, v. As for Ipairs and Pairs, Ipairs only iterate a list/array while pairs include dictionary.

1 Like

So what I am doing is touching base on more advanced stuff is what I am hearing. I’ve a lot to learn, but I assume I will learn as I go. I will be learning more of this education-wise later. Currently in college for Computer Sciences but I may as well take in what I can now before it becomes mandatory to pass. Thanks for the input.

1 Like

I am starting to put 2 and 2 together now but what I am now stuck on is IsA. I know where Basepart comes from, it is a reference to the group or pairs that was gotten from the group with the GetChildren. But what does IsA do?

for _,v: BasePart in ipairs(RightGroup:GetChildren()) do

This is a generic for loop. It basically loops through the children of ‘RightGroup’, which means you don’t have to manually index each child and change their properties.


: BasePart

This is typechecking. You can basically tell the script exactly what type a variable is. This is really useful as without it, you don’t get any autocompletions because the script doesn’t know what type a variable is.

For example you tell a variable is a ‘BasePart’. If you type this in:

part. 

You would see the script showing you all the properties and functions that a ‘BasePart’ has.


:IsA(className: string)

This is to check what type/class something is. For the code I gave you, I’m checking if the child given is a ‘BasePart’.

1 Like

This helps a ton. Thanks. I will go ahead and bookmark this page and use it for future reference.