Will this cause any performance issues?

Help and Feedback Code Review
#1

I have a thread that is suppose to ONLY RUN if a certain a value is set true (Let’s say, burn) and when that thread is running it loops through the character baseparts and starts a touched function and if it touches another player character it fires a bindable event. Now I was wondering, will this cause any issues or not?

local __TOUCHBURN = coroutine.create(function()
	for _,v in ipairs(Character:GetChildren()) do
		if v:IsA("BasePart") then
			v.Touched:Connect(function(__PART)
				if __CHARSTATS:WaitForChild("__BURNING").Value == true then
					local __VICTIMPLR = game:GetService("Players"):GetPlayerFromCharacter(__PART.Parent)
					if __VICTIMPLR ~= nil then
						local __CHARACTER = __VICTIMPLR.Character
						__CHARACTER:FindFirstChild("__EVENTS"):FindFirstChild("__BURN"):Fire(2,4)
					end
				end
			end)
		end
	end
end)
__CHARSTATS:FindFirstChild("__BURNING").Changed:Connect(function(__VA)
	if __VA == false then
		Burn:Stop()
		coroutine.yield(__TOUCHBURN) -- STOP THEAD BECAUSE PLAYER IS NO LONGER BURNING
	else
		coroutine.resume(__TOUCHBURN) -- START THREAD BECAUSE PLAYER IS BURNING
	end
end)
#2

It’s very hard to read what’s even going because every variable and argument starts with an underscore and is all in capital letters. An _ means that the variable is private, which it isn’t in this scenario.

2 Likes
#3

Apologies, I can paste a snippet with readable variables right now.
The reason I name my variables that is because of an old habit of mine I’ve got.

local TouchBurn = coroutine.create(function()
	for _,v in ipairs(Character:GetChildren()) do
		if v:IsA("BasePart") then
			v.Touched:Connect(function(Part)
				if CharacterStats:WaitForChild("IsBurning").Value == true then
					local VictimPlayer = game:GetService("Players"):GetPlayerFromCharacter(Part.Parent)
					if VictimPlayer ~= nil then
						local CharacterVictim = VictimPlayer.Character
						CharacterVictim:FindFirstChild("Events"):FindFirstChild("Burn"):Fire(2,4)
					end
				end
			end)
		end
	end
end)
CharacterStats:WaitForChild("IsBurning").Changed:Connect(function(Value)
	if Value == false then
		Burn:Stop()
		coroutine.yield(TouchBurn) -- STOP THEAD BECAUSE PLAYER IS NO LONGER BURNING
	else
		coroutine.resume(TouchBurn) -- START THREAD BECAUSE PLAYER IS BURNING
	end
end)
1 Like
#4

You could probably make PlayerService a variable instead of calling a function every time, and probably remove the find first child statements as im certain it’s the same as just indexing it.

#5

Edit: Found an issue with this, for some reason the bindable event still fires even though IsBurning is set to false meaning the thread is still running, why is that?