Code Review [Optimisations, Support]

Help and Feedback Code Review
#1

Hello everyone,

I have just put together a small part of a larger project of mine, and though it works, I know for a fact there are many ways of improving my code.

My Code 
local FlagData = require(script.Parent.Modules.Data.FlagData)

local Threads = {}

local function SetFlag(Part, FlagType)
	--[[
		Function to set a given flag part to a given flag type- if the part does not have a SurfaceGui setup inside it, it will automatically add
		one for it.
	]]

	if not FlagData then return end

	local function HasInstance(instance, list)
		--[[
			Function used to check if an instance within a list of instances is a certain instance type.
		]]

		for _, V in pairs(list) do
			if not V:IsA(instance) then 
				return false
			else
				return true
			end
		end
	end

	local PartChildren = Part:GetChildren()

	if not HasInstance("SurfaceGui", PartChildren) then
		local SurfaceGui = Instance.new("SurfaceGui")
		SurfaceGui.Face = "Top"
		SurfaceGui.ResetOnSpawn = false
		SurfaceGui.Parent = Part
	end

	local SurfaceGuiChildren = Part.SurfaceGui:GetChildren()

	if not HasInstance("ImageLabel", SurfaceGuiChildren) then
		local ImageLabel = Instance.new("ImageLabel")
		ImageLabel.Size = UDim2.new(1, 0,1, 0)
		ImageLabel.BackgroundTransparency = 1
		ImageLabel.Parent = Part.SurfaceGui
	end

	if FlagData[FlagType] then
		if #FlagData[FlagType] > 1 then
			Threads[Part.Name] = function()
				while true do
					for I, V in pairs(FlagData[FlagType]) do
						Part.SurfaceGui.ImageLabel.Image = "rbxassetid://" .. FlagData[FlagType][I]
						task.wait(0.25)
					end
				end
			end
		else
			Threads[Part.Name] = function()
				while task.wait(.25) do
					Part.SurfaceGui.ImageLabel.Image = "rbxassetid://" .. FlagData[FlagType][1]
					task.wait(0.25)
					Part.SurfaceGui.ImageLabel.Image = "rbxassetid://0"
				end
			end
		end
		
		task.spawn(Threads[Part.Name])
		print(FlagType .. " active on " .. Part.Name)
	elseif FlagType == false then
		print("Turn off")
	else
		warn("FlagSystem Error - Something tried to activate a flag on '" .. Part.Name .. "', but the given flag type does not exist.")
	end	
end

Basically, what this code does is add a SurfaceGui > ImageLabel to a part (if the part doesn’t already have them), then creates a new thread which uses a while true do loop to set the ImageLabel’s Image to the given image ID’s- though if only one image ID is given, the ImageLabels Image will be set to that one ID, then back to 0.

What I’m looking to do with this code, is optimise it, and make it possible to stop the while true do loop when FlagType is equal to false. It’s important that this code is well optimised because multiple of this:

Basically, what this code does is add a SurfaceGui > ImageLabel to a part (if the part doesn’t already have them), then creates a new thread which uses a while true do loop to set the ImageLabel’s Image to the given image ID’s- though if only one image ID is given, the ImageLabels Image will be set to that one ID, then back to 0.

may have to be done simultaneously.

I also need to be able to stop the loop from running, for when I no longer want any image to be shown on the part.

I’d be interested to make this into a more OOP sort of thing, as I feel it may be beneficial and just a better approach overall. This function will also need to be fired from the client.

If anyone can offer any input, I’d greatly appreciate it. I’m still rather new to programming.

Thanks

1 Like
#2

If you are wondering what I am trying to make, it’s shown in this video. I’m trying to make exactly what you see in this video.

1 Like
#3 
task.spawn(function()
	while FlagType do
		--code block
	end
end)

I would spawn that while loop so that it has its own thread, allowing you to stop the loop anytime when FlagType == false

2 Likes