Players rank and aura manager

[Erikmb (HecknIrishMan)]

I developed a system for giving players ranks and auras for this game I’m working on and while it does work. I don’t know if it’s efficient or good on performance

local ReplicatedStorage = game:GetService("ReplicatedStorage")
local ServerStorage = game:GetService("ServerStorage")

local Manager = require(script.Parent.PlayerData.Manager) --functions for getting and setting player data

local Ranks = require(ReplicatedStorage.Databases.Ranks) --dictionary of all the ranks in the game. each rank holds their names and needed power to get them
local remotes = ReplicatedStorage.Remotes

local auraParticles = ServerStorage.AuraParticles --folder that holds the folders for each auras particles

remotes.rankEvent.Event:Connect(function(player: Player)
	--get the players current power value and current rank
	local playerPower = Manager.GetPower(player)
	local playerRank = Manager.GetRank(player)
	
	--if player doesn't have an aura and their current rank has an aura available for it then give aura and enable boolean value (used for players rejoining the game if they had an aura when they previously left)
	if not player.Character.characterStats.hasAura.Value then
		local currentRankAura = getAura(playerRank)
		if currentRankAura then
			player.Character.characterStats.hasAura.Value = true
			giveAura(player.Character, playerRank)
		end
	end

	--loop through all ranks in the ranks dictionary
	for _, newRank in pairs(Ranks) do
		--if player doesn't have enough power for new rank then exit
		if playerPower < newRank.NeededPower then continue end

		--if new rank has a higher needed power than current rank then the player has ranked up
		if newRank.NeededPower > playerRank.NeededPower then
			--play rank up sound and display some gui
			warn("PLAYER HAS RANKED UP")

			--if the new rank has an aura available for it then remove the players current aura if they have one
			local newRankAura = getAura(newRank)
			if newRankAura then
				clearAura(player.Character)
			end

			--give the player the new ranks aura and set players rank to new rank
			giveAura(player.Character, newRank)
			Manager.SetRank(player, newRank)
		end
	end
end)

function clearAura(character: Model)
	--if a part is found with the name AuraPartHolder then destroy it
	local auraPartHolder = character:FindFirstChild("AuraPartHolder")
	if auraPartHolder then 
		auraPartHolder:Destroy()
	end

	--loop through all parts in the players character
	for _, bodyPart in character:GetChildren() do
		--if part found in for loop is not a body part then exit
		if not bodyPart:IsA("MeshPart") then continue end

		--if there is a particle emitter found OR an attachment with the name particleAttachment is found then destroy it
		for _, particle in bodyPart:GetChildren() do
			if particle:IsA("ParticleEmitter") or (particle:IsA("Attachment") and particle.Name == "particleAttachment") then 
				particle:Destroy()
			end
		end
	end

	--disable boolean value for player having an active aura
	character.characterStats.hasAura.Value = false
end

function getAura(rank: SharedTable)
	--loop though the auraParticles table and see if a part of the folders name matches the ranks name
	for _, auraFolder: Folder in auraParticles:GetChildren() do
		if not auraFolder.Name.match(auraFolder.Name, rank.Name) then continue end
		return auraFolder
	end
end

function giveAura(character: Model, rank: SharedTable)
	--if an aura isnt found then exit
	local auraFolder = getAura(rank)
	if not auraFolder then return end

	--loop though the particleHolder parts (that hold the particle emitters) that are in the aura folder
	for _, particleHolder: Part in auraFolder:GetChildren() do

		--loop through all parts in the players character
		for _, bodyPart in character:GetChildren() do

			--loop through all particle emitters or attachments that are in the particleHolder part
			for _, particle: ParticleEmitter in particleHolder:GetChildren() do

				--if the body part already has a particle emitter in it then exit
				if bodyPart:FindFirstChild(particle.Name) then continue end

				--if the character already has a part called AuraPartHolder then exit
				if character:FindFirstChild(particleHolder.Name) then continue end

				if bodyPart:IsA("MeshPart") then 
					if particleHolder.Name == "Everywhere" then
						
						--parents the cloned particles to each body part of the character
						local clone = particle:Clone()
						clone.Parent = bodyPart
					elseif particleHolder.Name == "Torso" then
						
						--if the body part found in the for loop doesn't have the name UpperTorso then exit
						if bodyPart.Name ~= "UpperTorso" then continue end

						--parents the cloned particle only to the characters torso
						local clone = particle:Clone()
						clone.Parent = bodyPart
					elseif particleHolder.Name == "Limbs" then
						
						--if the body part found in the for loop does't have one of these 4 names then exit
						if bodyPart.Name == "LeftLowerArm" or bodyPart.Name == "LeftLowerLeg" or bodyPart.Name == "RightLowerArm" or bodyPart.Name == "RightLowerLeg" then 
							
							--parents the cloned particle only to the characters limbs
							local clone = particle:Clone()
							clone.Parent = bodyPart
						end
					end
				else
					--if the body part found in the for loop is the HumanoidRootPart
					if bodyPart == character.PrimaryPart then

						--if the particle holder part being looped though has the name AuraPartHolder
						if particleHolder.Name == "AuraPartHolder" then

							--parent and weld the clone to the character 
							local holderClone = particleHolder:Clone()
							holderClone.CFrame = bodyPart.CFrame * CFrame.new(0, -2, 0)
							holderClone.Anchored = false
							holderClone.CanCollide = false
							holderClone.Parent = character

							local weld = Instance.new("WeldConstraint")
							weld.Part0 = holderClone
							weld.Part1 = bodyPart
							weld.Parent = holderClone
						end
					end
				end

				--random time for each particle emitter to be cloned to the body parts. adds more visual randomness
				task.wait(math.random(0.05, 0.5))
			end
		end
	end
end

[kiloe2]

Cool! As long as it works and doesn’t lag your game, you’re fine on performance, but I do want to point out a few things relating to code structure:

Firstly, I love the use of comments, but note that if you’re too heavy on them, they can get in the way! A good rule of thumb is to avoid comments that explain what your code does, and add comments that explain why your code does it. Ideally, the code we write should be clear enough to explain the what on its own without needing help from a comment. Most of your code here is plenty clear enough to do that.

Secondly, you’ve got some obese functions here. One of the ways we can mitigate that is by making sure that each function does one job. Big functions are hard to keep up with.

You’ve already done a good job of this, but I think it would be beneficial to go a bit further with it. Here’s an edited version of your script that extracts different responsibilities out into their own functions:

function clearAura(character: Model)
	local auraPartHolder = character:FindFirstChild("AuraPartHolder")
	if auraPartHolder then 
		auraPartHolder:Destroy()
	end

	for _, bodyPart in character:GetChildren() do
		if not bodyPart:IsA("MeshPart") then continue end

		for _, particle in bodyPart:GetChildren() do
			if particle:IsA("ParticleEmitter") or (particle:IsA("Attachment") and particle.Name == "particleAttachment") then 
				particle:Destroy()
			end
		end
	end

	character.characterStats.hasAura.Value = false
end

function giveAura(character: Model, rank: SharedTable)
	-- This is a big function, we'll clean this up in the next step.
end

local function updateAura(player, playerRank)
    local aura = getAura(playerRank)
    if not aura then return end

    if player.Character.characterStats.hasAura.Value then
        clearAura(player.Character)
    end

    giveAura(player.Character, newRank)
    player.Character.characterStats.hasAura.Value = true
end

local function detectRankUp(playerPower, playerRank)
	for _, newRank in pairs(Ranks) do
		if playerPower < newRank.NeededPower then continue end
		if newRank.NeededPower <= playerRank.NeededPower then continue end

		return newRank
	end

	-- (returns nil if no rank up by default)
end

remotes.rankEvent.Event:Connect(function(player: Player)
	-- Notice how much easier it is to understand what this function does at a glance!
	local playerPower = Manager.GetPower(player)
	local playerRank = Manager.GetRank(player)
	local newRank = detectRankUp(playerPower, playerRank)
	
	if not player.Character.characterStats.hasAura.Value then
		updateAura(player, playerRank)
	end

	if newRank then
		warn("PLAYER HAS RANKED UP")
		updateAura(player, newRank)
		Manager.SetRank(player, newRank)
	end
end)

Now let’s tackle that giveAura function. That thing is a CHONKER, and it has three nested loops, which could be a sign of overcomplication. It appears that you’re trying to allow for simple implementation in the instance hierarchy. That’s good, but there’s no graceful way to check for custom names like “Limbs” without implementing an elseif chain that gets longer with the amount of names you want to detect. Perhaps you could try making it so that the names of particle holder parts directly correspond to names of body parts. The code would look as follows:

local function weld(part0, part1, offset) -- Extracted from giveAura
	holderClone.CFrame = part1.CFrame * offset
	part0.Anchored = false
	part0.CanCollide = false

	local weld = Instance.new("WeldConstraint")
	weld.Part0 = part0
	weld.Part1 = part1
	weld.Parent = part0
end

function clearAura(character: Model)
	local auraPartHolder = character:FindFirstChild("AuraPartHolder")
	if auraPartHolder then 
		auraPartHolder:Destroy()
	end

	for _, bodyPart in character:GetChildren() do
		if not bodyPart:IsA("MeshPart") then continue end

		for _, particle in bodyPart:GetChildren() do
			if particle:IsA("ParticleEmitter") or (particle:IsA("Attachment") and particle.Name == "particleAttachment") then 
				particle:Destroy()
			end
		end
	end

	character.characterStats.hasAura.Value = false
end

function giveAura(character: Model, rank: SharedTable)
	-- Still kinda big, but not as big, and a LOT less complicated!

	local auraFolder = getAura(rank)
	if not auraFolder then return end

	for _, particleHolder: Part in auraFolder:GetChildren() do
		local bodyPart = character:FindFirstChild(particleHolder.Name)
		-- (we can cut out the guard clauses detecting if the player already has an aura there because we clear the aura before we call giveAura outside of this function)

		if not bodyPart and particleHolder.Name == "AuraPartHolder" then
			local holderClone = particleHolder:Clone()
			weld(holderClone, character.HumanoidRootPart, CFrame.new(0, -2, 0)) -- Uh oh, another job detected, you know what that means! We extract it to its own function B)
			holderClone.Parent = character
		else
			for _, particle in particleHolder:GetChildren() do
				particle:Clone().Parent = bodyPart
			end
		end

		--random time for each particle emitter to be cloned to the body parts. adds more visual randomness
		-- (Note that the above is a GOOD comment because it explains the WHY and not the WHAT)
		task.wait(math.random(0.05, 0.5))
	end
end

local function updateAura(player, playerRank)
    local aura = getAura(playerRank)
    if not aura then return end

    if player.Character.characterStats.hasAura.Value then
        clearAura(player.Character)
    end

    giveAura(player.Character, newRank)
    player.Character.characterStats.hasAura.Value = true
end

local function detectRankUp(playerPower, playerRank)
	for _, newRank in pairs(Ranks) do
		if playerPower < newRank.NeededPower then continue end
		if newRank.NeededPower <= playerRank.NeededPower then continue end

		return newRank
	end
end

remotes.rankEvent.Event:Connect(function(player: Player)
	local playerPower = Manager.GetPower(player)
	local playerRank = Manager.GetRank(player)
	local newRank = detectRankUp(playerPower, playerRank)
	
	if not player.Character.characterStats.hasAura.Value then
		updateAura(player, playerRank)
	end

	if newRank then
		warn("PLAYER HAS RANKED UP")
		updateAura(player, newRank)
		Manager.SetRank(player, newRank)
	end
end)

Notice: The above code is untested!

I did this all in the draft compose box. This might have a few errors or inconsistencies with the original, so you might have to modify it a little.

But with that, hopefully this helps point you in a good direction. Remember that everything I’ve said is just a suggestion, and if you absolutely must do it a different way, there’s no rule against that. These are just tips for a bit more readable and manageable code.

[Erikmb (HecknIrishMan)]

This worked almost perfectly! (had to do some small tweaks to the giveAura function)
Thanks a bunch!

Also the abundance of comments was done mostly for the purpose of making this post just to make things as clear as possible. On an average day I don’t do nearly as much commenting as I probably should. Might try change that…

[sonic_848 (killer)]

@kiloe2

obese function

I don’t know why but i find it funny you used the word obese lol

[kiloe2]

I’m glad this helped

@sonic_848

I don’t know why but i find it funny you used the word obese lol

Always beware obese functions…