It’s not too inefficient in the first place, but here’s how I would optimize this.
(note: I have not and can not test this code, but any errors should be simple to resolve)
Original script:
local trees = script.Parent:GetChildren()
for _, v in ipairs(trees) do
if v.Name == "Tree" then
for _, aa in ipairs(v:GetChildren()) do
if aa:IsA('BasePart') then
if aa.Material == Enum.Material.Grass then
task.delay(0, function()
local pos = aa.Position
pos = Vector3.new(pos.x, pos.y-0.2, pos.z)
local x = 0
local z = 0
local T = -99999
local tall = aa.Size.Y / 2
math.randomseed(tick())
local rand = (math.random(0,20))/10
while task.wait() do
x = pos.x + (math.sin(T + (pos.x/5)) * math.sin(T/9))/3
z = pos.z + (math.sin(T + (pos.z/6)) * math.sin(T/12))/4
aa.CFrame = CFrame.new(x, pos.y, z) * CFrame.Angles((z-pos.z)/tall, 0,(x-pos.x)/-tall)
T = T + 0.12
end
end)
end
end
end
end
end
I move the function out of the loop so that it doesn’t get recreated for each part. Also use coroutine.wrap instead of task.delay to be able to supply an argument. (Both task.delay and coroutine.wrap have the same effect here: putting the function in a coroutine so that the wait does not stop the whole loop).
local trees = script.Parent:GetChildren()
local function animate(aa)
local pos = aa.Position
pos = Vector3.new(pos.x, pos.y-0.2, pos.z)
local x = 0
local z = 0
local T = -99999
local tall = aa.Size.Y / 2
math.randomseed(tick())
local rand = (math.random(0,20))/10
while task.wait() do
x = pos.x + (math.sin(T + (pos.x/5)) * math.sin(T/9))/3
z = pos.z + (math.sin(T + (pos.z/6)) * math.sin(T/12))/4
aa.CFrame = CFrame.new(x, pos.y, z) * CFrame.Angles((z-pos.z)/tall, 0,(x-pos.x)/-tall)
T = T + 0.12
end
end
-- NOTE: the nested loop remains. It's not a problem at all. The real work actually isn't and never has been inside a deeply nested loop.
for _, v in ipairs(trees) do
if v.Name == "Tree" then
for _, aa in ipairs(v:GetChildren()) do
if aa:IsA('BasePart') then
if aa.Material == Enum.Material.Grass then
coroutine.wrap(animate)(aa)
end
end
end
end
end
Some pedantic cleanups/tweaks, but the structure stays the same. It’s mostly variable removal.
local trees = script.Parent:GetChildren()
-- EDIT: changed variable name (aa -> part)
local function animate(part)
-- EDIT: calculated pos in one step
local pos = part.Position + Vector3.new(0, -0.2, 0)
-- EDIT: x, z are now locals inside the loop
local tall = part.Size.Y / 2
-- EDIT: removed rand and randomseed because it was not being used at all
while true do
-- EDIT: replaced T with os.clock(), renamed to t and placed inside the loop
local t = os.clock()
local x = pos.x + (math.sin(t + (pos.x/5)) * math.sin(t/9))/3
local z = pos.z + (math.sin(t + (pos.z/6)) * math.sin(t/12))/4
-- EDIT: added a space after a comma
part.CFrame = CFrame.new(x, pos.y, z) * CFrame.Angles((z-pos.z)/tall, 0, (x-pos.x)/-tall)
-- EDIT: moved task.wait() into the loop
task.wait()
end
end
for _, v in ipairs(trees) do
if v.Name == "Tree" then
for _, aa in ipairs(v:GetChildren()) do
-- EDIT: merged the two ifs using `and`
if aa:IsA('BasePart') and aa.Material == Enum.Material.Grass then
coroutine.wrap(animate)(aa)
end
end
end
end
Now this is the real optimization: I store information on each leaf part in a table and animate all of them in one loop.
The earlier approach did the same thing, but accessing a table is somewhat cheaper than resuming a coroutine (that is, the task scheduler timing each animate() individually and having to resume each of them).
local trees = script.Parent:GetChildren()
local parts = {} -- array of parts
local positions = {} -- array of positions
-- this could've also been done with an array of {part, position}s for easier expansion, but this reduces the amount of tables, saving memory somewhat
-- discover parts
for _, v in ipairs(trees) do
if v.Name == "Tree" then
for _, aa in ipairs(v:GetChildren()) do
if aa:IsA('BasePart') and aa.Material == Enum.Material.Grass then
table.insert(parts, aa)
table.insert(positions, aa.Position + Vector3.new(0, -0.2, 0))
end
end
end
end
-- animate parts
while true do
local t = os.clock()
for i = 1, #parts do
local part = parts[i]
local pos = positions[i]
local tall = part.Size.Y / 2
local x = pos.x + (math.sin(t + (pos.x/5)) * math.sin(t/9))/3
local z = pos.z + (math.sin(t + (pos.z/6)) * math.sin(t/12))/4
part.CFrame = CFrame.new(x, pos.y, z) * CFrame.Angles((z-pos.z)/tall, 0,(x-pos.x)/-tall)
end
task.wait()
end
In addition, I seriously hope you’re running this on the client! If this loop is running on the server, it will create a lot of pointless network traffic updating all the parts. If the network jitters a little, then the trees will also animate with some stutter.
