Is there a better way of doing this code?
local Particles1 = script.Rays1:Clone()
local Particles2 = script.Rays2:Clone()
local Particles3 = script.Glow1:Clone()
Particles1.Parent = char.Head
Particles2.Parent = char.Head
Particles3.Parent = char.Head
wait(2)
char.Head.Rays1:Destroy()
char.Head.Rays2:Destroy()
char.Head.Glow1:Destroy()
for _, v in pairs({script.Rays1, script.Rays2, script.Glow1}) do
local Particles = v:Clone()
Particles.Parent = char.Head
game:GetService("Debris"):AddItem(Particles, 2)
end
Sources: Debris:AddItem, loop for do
Wouldnât use AddItem if I were you, has performance issues as stated in other threads.
for i = 1,3 do
script["Particle"..i]:Clone().Parent = char.Head
end
task.wait(2)
for i = 1,3 do
local particle = char.Head:FindFirstChild("Particle"..i)
if particle then
particle:Destroy()
end
end
Iâd recommend this. Your script would also error since it is trying to index Script with an Instance. I would also suggest you use ipairs over pairs for arrays.
Oh, I did not notice, thank you.
Where did you find this information?
This has one big and one small problem, the big one is the bad practice of initializing the table inside of the pairs function, Iâd rather do it like this for code cleanliness:
local particles = {
script.Rays1,
script.Rays2,
script.Glow1
}
for _, v in ipairs(particles) do
...
end
The second problem which as I said is âsmallâ, is that in this case you should use ipairs instead of pairs. Itâs because your table has integer indexing from 1 to 3 (1 first element, 3 - last element), therefore you should use ipairs which was designed to work best in this case.
for Index, Current in ipairs(script:GetChildren()) do
Current:Clone()
task.wait()
end
------------------------------------------------------------
local Particles = {script.Rays1, script.Rays2, script.Glow1}
local Debris = game:GetService("Debris")
for Index, Particle in pairs(Particles) do
Particle.Parent = char.Head
Debris:AddItem(Particle, 2)
end
print("Finished!")
I hope this helps you at least get an understanding for organizing code better, and utilizing different functions/services!
Why? Is there a Roblox post about this? If a delay occurs, it is most likely due to another issue, right?
As I said this is an issue related to code clarity and expandability.
Of course, it depends on the person who writes it, for me, since it is a small and simple table, it is better that way, that is not why it is a bad practice.
The functionality is what matters, clarity doesnât matter. Itâs not always about aesthetics! If you can distinguish what it does, or is, thatâs good enough if it works. 
Of course, if itâs indistinguishable, then itâs a problem. But I donât believe it matters in this instance, itâs the same as writing what you wrote, just yours is more lines. (It does look cleaner though.)
Doing it like that with small things makes you care less about it when working on large projects. Most of the programmers would agree.
Letâs say that you join a company or a team, and want to quickly get into the project. If the team just wants to make the code âworkâ but not be clear, you will have a hard time getting into it, and you will spend a lot of time trying to understand what they meant.
Then after you finally understand their code, you start to implement more functionalities. But it turns out that the âworkingâ code was implemented so badly and without any plan that in order to add a new functionality you have to fix half of the project. If you had code, that was written clearly with a plan for further expansion, it would take you less time, less effort and less changes.
What? Thatâs the whole point of hiring a team⌠To do what you donât know how to do, completely invalidating that scenario. And plus, if your team has a good programmer, they will know how that works. Itâs not like they were doing this:
local Part = game["Workspace"]:FindFirstChild(['PartThatIWillNeverUseLol']):FindFirstChild("Other", true) or game["Workspace"]:FindFirstChild(['PartThatIWillNeverUseLol']):FindFirstDescendant("Other")
And Iâm sure still people know exactly what thatâs doing, even though that looks like itâs straight from beneath. || Weâre ending this conversation here, have a good day/night.
You completely misunderstood what I said. Anyways, in teams some people join and some people leave. Itâs just a matter of time, thatâs a natural thing. After a good programmer, as you said, leaves the team, there is nobody left to explain his code. And now if the code was written with a plan and preferably according to some standarized practices, then it will be easier to introduce a new programmer into the project.
This would error, youâre attempting to index children named âParticle1â, âParticle2â and âParticle3â when the children are named âRays1â, âRays2â and âGlow1â instead. You can also achieve this in a single loop however the performance difference is likely negligible.
local particles = {script.Rays1, script.Rays2, script.Glow1}
for _, particle in ipairs(particles) do
particle.Parent = char.Head
task.delay(2, function()
particle:Destroy()
end)
end
local particles = {script.Rays1, script.Rays2, script.Glow1}
local debris = game:GetService("Debris")
for _, particle in ipairs(particles) do
particle.Parent = char.Head
debris:AddItem(particle, 2)
end
Debris:AddItem is also fine, itâs arguable better than a task.delay() implementation as calling â:Destroy()â on a non-existent instance will cause the script to error whereas calling â:AddItem()â on a non-existent instance will not cause the script to error.
-- up at the top of the script...
local templates = {script.Rays1, script.Rays2, script.Glow1}
-- ...
-- whenever you need to add the effects...
local head = char:FindFirstChild('Head') -- if there's no head this won't error
local instances = {}
for i, template in ipairs(templates) do -- clone templates into head and record them
local instance = template:Clone()
instance.Parent = head
instances[i] = instance
end
task.delay(2, function() -- destroy all created instances after 2 seconds
for _, instance in ipairs(instances) do
instance:Destroy()
end
end)
Comments on things said by others:
I go with task.delay just to be on the safe side, but Debris:AddItem isnât the end of the world.
This isnât a thing and it can actually be cleaner to initialize a table inside the pairs function if and only if you are only going to use that table once and the only alternative is to duplicate code.
i.e. this is okay:
for _, child in ipairs({x, y, z}) do
-- whatever with x, y and z...
end
However, if you are working with children that are Instances and stick around for the lifetime of the script, and not local variables, you should really declare the table ahead of time (like, way before youâre about to start iterating - maybe even at the top of the script).
This doesnât matter as much because the order of iteration doesnât change the result, but in general yes you should use ipairs for arrays, and pairs for maps (tables with sparse number keys, or string keys).