Suggestions to simplify and shorten my code

Hello!

So, I’m currently making something similar to Google Home. Well, I’ve already made this once using Dialogs, but wasn’t really satisfied so I decided to start again (and possibly find another way to do this without dialogs).

Overview:

  • This script manages the turn on / off system for my project. It’s getting settings from a module in ReplicatedStorage, then checks if required for the current action settings is enabled or disabled. Plays animation depending on settings. I feel like it’s really messy and quite a few parts are unneeded. It generally works prefectly fine, but I feel like there is still room for improvement.

  • I’d like to get some suggestions on how can I simplify and shorten the code.

Main Code:

local neonPart = script.Parent.Parent.Parent.PartModel.Neon
local tweenInfo = TweenInfo.new(0.75, Enum.EasingStyle.Quad, Enum.EasingDirection.InOut, 0, false, 0)
local tweenInfo2 = TweenInfo.new(0.3, Enum.EasingStyle.Quad, Enum.EasingDirection.InOut, 0, false, 0)
local storage = game:WaitForChild("ReplicatedStorage"):WaitForChild("exaListenerProStorage")
local settingsModule = require(storage.Settings)

local on = script.Parent.Parent.valEnabled.Value
local startAnimPlayed = script.Parent.Parent.startAnimPlayed.Value

wait()

function clicked()
	if on == false then -- Enabling
		on = true
		script.Parent.MaxActivationDistance = 0 -- Debounce
		
		-- Check if playAnimationOnce = true and startAnimPlayed = false so it matches settings
		if settingsModule.startSettings.playAnimationOnce == true and startAnimPlayed == false then
			
			startAnimPlayed = true -- If playAnimOnce = true, it stops future animations from playing
			
			-- Checking if animation is enbaled in settings, if it's not, skipping it
			if settingsModule.startSettings.startAnimation == true then
				-- Neon animation
				game:GetService("TweenService"):Create(neonPart, tweenInfo2, {Color = Color3.fromRGB(0, 198, 122)}):Play()
				wait(0.5)
			else	
			end
			
		-- Playing the animation on every startup if the settings say so
		elseif settingsModule.startSettings.playAnimationOnce == false then -- Plays it everytime
			
			if settingsModule.startSettings.startAnimation == true then
				
				game:GetService("TweenService"):Create(neonPart, tweenInfo2, {Color = Color3.fromRGB(0, 198, 122)}):Play()
				wait(0.5)
			else
			end
		end
		
		-- Animating neon lights to white (enabled)
		game:GetService("TweenService"):Create(neonPart, tweenInfo2, {Color = Color3.fromRGB(255, 255, 255)}):Play()
		wait(0.2)
		script.Parent.Parent.turnOn:Play() -- Playing the turnOn sound
		wait(0.3)
		script.Parent.MaxActivationDistance = 15 -- Ending debounce

	elseif on == true then -- Disabling
		on = false
		script.Parent.MaxActivationDistance = 0 -- Debounce
		
		-- Animating neon lights to black (disabled)
		game:GetService("TweenService"):Create(neonPart, tweenInfo, {Color = Color3.fromRGB(27, 42, 53)}):Play()
		script.Parent.Parent.turnOff:Play() -- Playing the turnOff sound
		wait(0.5)
		script.Parent.MaxActivationDistance = 15 -- Ending debounce
	end	
end

script.Parent.MouseClick:Connect(clicked)

Settings Module:

local settingsModule = {
	
	-- Starting Settings
	startSettings = {
		
		startAnimation = true, -- When 'false' there is a short animation after enabling the listener; When 'true', the
							   -- listener enabled instanly (without animation).
		
		playAnimationOnce = false, -- [ONLY WORKS IF 'startAnimation' is TRUE!]
								  -- Only play the start animation when the listener is enabled for the first time.	
	}
}

return settingsModule

Thanks!

1 Like

I don’t see why you need to shorten it, It looks amazing and beyond what I would have thought of :+1:

I just briefly skimmed it but it doesn’t look that long to me. It looks decently sized and doesn’t appear to have any repetitive blocks. If it works I think its fine.

Not much i could optimize and shorten as this was decent code anyway but heres what i could do

if not game:IsLoaded() then game.Loaded:Wait() end

local tweenService = game:GetService("TweenService")

local neonPart = script.Parent.Parent.Parent.PartModel.Neon
local tweenInfo = TweenInfo.new(0.75, Enum.EasingStyle.Quad, Enum.EasingDirection.InOut, 0, false, 0)
local tweenInfo2 = TweenInfo.new(0.3, Enum.EasingStyle.Quad, Enum.EasingDirection.InOut, 0, false, 0)
local storage = game:GetService("ReplicatedStorage").exaListenerProStorage
local settingsModule = require(storage.Settings)

local on = script.Parent.Parent.valEnabled.Value
local startAnimPlayed = script.Parent.Parent.startAnimPlayed.Value

function clicked()
	if not on then -- Enabling
		on = true
		script.Parent.MaxActivationDistance = 0 -- Debounce

		-- Check if playAnimationOnce = true and startAnimPlayed = false so it matches settings
		if settingsModule.startSettings.playAnimationOnce and not startAnimPlayed then
			startAnimPlayed = true -- If playAnimOnce = true, it stops future animations from playing

			-- Checking if animation is enbaled in settings, if it's not, skipping it
			if settingsModule.startSettings.startAnimation then
				-- Neon animation
				tweenService:Create(neonPart, tweenInfo2, {Color = Color3.fromRGB(0, 198, 122)}):Play()
				wait(0.5)
			end

			-- Playing the animation on every startup if the settings say so
		elseif not settingsModule.startSettings.playAnimationOnce then -- Plays it everytime
			if settingsModule.startSettings.startAnimation then
				tweenService:Create(neonPart, tweenInfo2, {Color = Color3.fromRGB(0, 198, 122)}):Play()
				wait(0.5)
			end
		end

		-- Animating neon lights to white (enabled)
		tweenService:Create(neonPart, tweenInfo2, {Color = Color3.fromRGB(255, 255, 255)}):Play()
		wait(0.2)
		script.Parent.Parent.turnOn:Play() -- Playing the turnOn sound
		wait(0.3)
		script.Parent.MaxActivationDistance = 15 -- Ending debounce
	else -- Disabling
		on = false
		script.Parent.MaxActivationDistance = 0 -- Debounce

		-- Animating neon lights to black (disabled)
		tweenService:Create(neonPart, tweenInfo, {Color = Color3.fromRGB(27, 42, 53)}):Play()
		script.Parent.Parent.turnOff:Play() -- Playing the turnOff sound
		wait(0.5)
		script.Parent.MaxActivationDistance = 15 -- Ending debounce
	end	
end

script.Parent.MouseClick:Connect(clicked)

I added a check at the top to see if the game is loaded then it waits until it is if its not this basically removes the need for :WaitForChild() in any client / replicated areas this helps a ton with memory usage as the program doesn’t need to check if somethings there before doing something with it

I made a variable for TweenService to stop the script from getting TweenService every time you wanted to tween something this helps with performance but since there was minimal tweening it wasn’t really needed

I removed the random unneeded wait() call at line 10

I removed any statements such as if variableExample == true / false to a shorter way of checking a Boolean value this doesn’t effect anything just shortens the character amount

I removed the random else from line 37 as there’s no need for the program to go there as there’s nothing after the else statement

You didn’t need elseif on == true then you could have just done else as on cant be anything but true or false

This is all I could find I’m sure there’s some very small ways to optimize and shrink this more but this is all i could find

2 Likes

You should replace wait() with task.wait()

I will admit this does feel a bit cluttered as well but its more efficient and is as short as i could get it without removing any comments

Yeah I know it’s pretty short right now, but It’s just the “start” of the script because I will be adding much more functions to it and I would rather have it clean and organized before doing so.

You can use guard clauses to prevent spaghetti code and nested if statements

local var = "string"


if not var then
      warn("false")
end
if type(var) == "number" then
      warn("number")
end
if string.len(var) > 6 then
      return
end

print("All checks passed")

And you can also split long code into more functions, so you are able to manage your code more easily.