| |
|
»
»
|
|
| |
Re: Increasing the efficiency of this function
|
|
|
|
| 14 Sep 2016 07:04 AM |
I think I've gone as far as I can but if any-one knows any more efficient ways of scripting this please give me a shout. Basically the premise is the function applies grass to blocks and spheres (spheres are unions).
function Foliage(part) local Area = part.Size.X*part.Size.Z local Model = Instance.new("Model", FoliageModel) local val = Instance.new("ObjectValue", Model) val.Name = "ParentTerrain" val.Value = part for i = 1,math.floor(Area*Density) do local Grass = script.Grass:clone() Grass.BrickColor = part.BrickColor local x = math.random(math.floor(3-(part.Size.X)/2), math.ceil((part.Size.X-3)/2)) local z = math.random(math.floor(3-(part.Size.Z)/2), math.ceil((part.Size.Z-3)/2)) local objectSpace = CFrame.new(x, (part.Size.Y/2)-1, z) if part:IsA("UnionOperation") then -- for spheres, not too bothered about this atm local Rsq = ((part.Size.Y/2)^2) local R = math.sqrt(Rsq) local determiner = math.random(0,1) local x,y if determiner < 0.5 then x = math.random(-R, R) local new = math.sqrt(Rsq - math.abs(x)) z = math.random(-new, new) else z = math.random(-R, R) local new = math.sqrt(Rsq - math.abs(z)) x = math.random(-new, new) end local y = math.sqrt(Rsq - ((x^2)+(z^2))) objectSpace = CFrame.new(x, y, z) end Grass.CFrame = part.CFrame:toWorldSpace(objectSpace) * CFrame.Angles(0,math.rad(math.random(0,360)),0) Grass.Parent = Model end end
Any help will always be appreciated! Thanks, UK. |
|
|
| Report Abuse |
|
|
|
| 14 Sep 2016 07:33 AM |
Iterations: 30
Original: Average time: 11.001107931137
Optimisation #1: Average time: 10.167498588562
Not much of a difference Also, that's some crappy indenting
Will try and improve it more once I fix up the indentation |
|
|
| Report Abuse |
|
|
|
| 14 Sep 2016 08:03 AM |
Add these variables to the top of your script: local mathceil = math.ceil local mathfloor = math.floor local CFramenew = CFrame.new local mathrandom = math.random local mathsqrt = math.sqrt local mathabs = math.abs local mathrad = math.rad local CFrameAngles = CFrame.Angles
and change the function calls to the variables z = math.random(-R, R) becomes z = mathrandom(-R, R)
For some reason, removing this line reduced the execution time by half: Grass.Parent = Model |
|
|
| Report Abuse |
|
|
|
| 14 Sep 2016 08:07 AM |
local mathceil = math.ceil local mathfloor = math.floor local CFramenew = CFrame.new local mathrandom = math.random local mathsqrt = math.sqrt local mathabs = math.abs local mathrad = math.rad local CFrameAngles = CFrame.Angles local Instancenew = Instance.new
function Foliage(part) FoliageModel:ClearAllChildren() local Area = part.Size.X*part.Size.Z local Model = Instancenew("Model", FoliageModel) local val = Instancenew("ObjectValue", Model) val.Name = "ParentTerrain" val.Value = part local xMin = mathfloor(3-(part.Size.X)/2) local xMax = mathceil((part.Size.X-3)/2) local yMin = mathfloor(3-(part.Size.Z)/2) local yMax = mathceil((part.Size.Z-3)/2) local y = (part.Size.Y/2)-1 local isUnion = part:IsA("UnionOperation") for i = 1, mathfloor(Area*Density) do local Grass = game.Workspace.Grass:Clone() Grass.BrickColor = part.BrickColor local x = mathrandom(xMin, xMax) local z = mathrandom(yMin, yMax) local objectSpace = CFramenew(x, y, z) if isUnion then -- for spheres, not too bothered about this atm local Rsq = ((part.Size.Y/2)^2) local R = mathsqrt(Rsq) local determiner = mathrandom(0,1) local x,y if determiner < 0.5 then x = mathrandom(-R, R) local new = mathsqrt(Rsq - mathabs(x)) z = mathrandom(-new, new) else z = mathrandom(-R, R) local new = mathsqrt(Rsq - mathabs(z)) x = mathrandom(-new, new) end local y = math.sqrt(Rsq - ((x^2)+(z^2))) objectSpace = CFrame.new(x, y, z) end local cframe = part.CFrame:toWorldSpace(objectSpace) local angle = CFrameAngles(0,mathrad(mathrandom(0,360)),0) Grass.CFrame = cframe * angle Grass.Parent = Model end end
|
|
|
| Report Abuse |
|
|
|
| 14 Sep 2016 08:07 AM |
| Yes, sorry for the indentation, it doesn't come across well from studio, thank you for your efforts but likewise anything I seem to do produces negligible effects, I think the bolster of the activity goes into the CFraming which is what I really wanted some help with, removing the randomization and creating a grid pattern doesn't give any significant improvements either. |
|
|
| Report Abuse |
|
|
|
| 14 Sep 2016 08:14 AM |
You can see which part of the code takes the most time Open a new place in studio and run this in the command bar
It doesn't show the total execution time for the benchmarks inside the function, but it still shows the general efficiency You will see that 'Benchmark 8' takes the longest, which is where Grass.Parent is changed
local mathceil = math.ceil local mathfloor = math.floor local CFramenew = CFrame.new local mathrandom = math.random local mathsqrt = math.sqrt local mathabs = math.abs local mathrad = math.rad local CFrameAngles = CFrame.Angles local Instancenew = Instance.new
local Density = 1 local baseplate = game.Workspace.Baseplate baseplate.Size = Vector3.new(20, 2, 20) baseplate.Position = Vector3.new(0, 0, 0)
local grassTemplate = Instance.new("Part", game.Workspace) grassTemplate.Name = "Grass" grassTemplate.Size = Vector3.new(1, 2, 1)
local FoliageModel = game.Workspace:FindFirstChild("FoliageModel") if not FoliageModel then FoliageModel = Instancenew("Model", game.Workspace) FoliageModel.Name = "FoliageModel" end
local benchmarks = {0, 0, 0, 0, 0, 0, 0, 0}
function Foliage(part) start = tick() FoliageModel:ClearAllChildren() local Area = part.Size.X*part.Size.Z local Model = Instancenew("Model", FoliageModel) local val = Instancenew("ObjectValue", Model) val.Name = "ParentTerrain" val.Value = part benchmarks[1] = benchmarks[1] + (tick() - start) start = tick() local xMin = mathfloor(3-(part.Size.X)/2) local xMax = mathceil((part.Size.X-3)/2) local yMin = mathfloor(3-(part.Size.Z)/2) local yMax = mathceil((part.Size.Z-3)/2) local y = (part.Size.Y/2)-1 local isUnion = part:IsA("UnionOperation") benchmarks[2] = benchmarks[2] + (tick() - start) for i = 1, mathfloor(Area*Density) do start = tick() local Grass = game.Workspace.Grass:Clone() Grass.BrickColor = part.BrickColor local x = mathrandom(xMin, xMax) local z = mathrandom(yMin, yMax) local objectSpace = CFramenew(x, y, z) benchmarks[3] = benchmarks[3] + (tick() - start) start = tick() if isUnion then -- for spheres, not too bothered about this atm local Rsq = ((part.Size.Y/2)^2) local R = mathsqrt(Rsq) local determiner = mathrandom(0,1) local x,y if determiner < 0.5 then x = mathrandom(-R, R) local new = mathsqrt(Rsq - mathabs(x)) z = mathrandom(-new, new) else z = mathrandom(-R, R) local new = mathsqrt(Rsq - mathabs(z)) x = mathrandom(-new, new) end local y = math.sqrt(Rsq - ((x^2)+(z^2))) objectSpace = CFrame.new(x, y, z) end benchmarks[4] = benchmarks[4] + (tick() - start) start = tick() local cframe = part.CFrame:toWorldSpace(objectSpace) benchmarks[5] = benchmarks[5] + (tick() - start) start = tick() local angle = CFrameAngles(0,mathrad(mathrandom(0,360)),0) benchmarks[6] = benchmarks[6] + (tick() - start) start = tick() Grass.CFrame = cframe * angle benchmarks[7] = benchmarks[7] + (tick() - start) start = tick() Grass.Parent = Model benchmarks[8] = benchmarks[8] + (tick() - start) end end
local start = tick() local averageTime = 0 local iterations = 30
for i = 1, iterations do Foliage(baseplate) averageTime = averageTime + (tick() - start) end print("Iterations: "..iterations) print("Average time: "..averageTime) for i,v in pairs(benchmarks) do print("Benchmark "..i.." : "..v) end
|
|
|
| Report Abuse |
|
|
|
| 14 Sep 2016 08:18 AM |
Whoops, forgot to divide averageTime by iterations
Iterations: 1000
Original: Average time: 12.203981934786
Optimized: Average time: 10.699549471617 |
|
|
| Report Abuse |
|
|
|
| 14 Sep 2016 08:28 AM |
Even more optimized Average time: 8.0798643918037
local mathceil = math.ceil local mathfloor = math.floor local CFramenew = CFrame.new local mathrandom = math.random local mathsqrt = math.sqrt local mathabs = math.abs local mathrad = math.rad local CFrameAngles = CFrame.Angles local Instancenew = Instance.new
function Foliage(part) local Area = part.Size.X*part.Size.Z local Model = Instancenew("Model") local val = Instancenew("ObjectValue", Model) val.Name = "ParentTerrain" val.Value = part local xMin = mathfloor(3-(part.Size.X)/2) local xMax = mathceil((part.Size.X-3)/2) local yMin = mathfloor(3-(part.Size.Z)/2) local yMax = mathceil((part.Size.Z-3)/2) local y = (part.Size.Y/2)-1 local isUnion = part:IsA("UnionOperation") for i = 1, mathfloor(Area*Density) do local Grass = game.Workspace.Grass:Clone() Grass.BrickColor = part.BrickColor local x = mathrandom(xMin, xMax) local z = mathrandom(yMin, yMax) local objectSpace = CFramenew(x, y, z) if isUnion then -- for spheres, not too bothered about this atm local Rsq = ((part.Size.Y/2)^2) local R = mathsqrt(Rsq) local determiner = mathrandom(0,1) local x,y if determiner < 0.5 then x = mathrandom(-R, R) local new = mathsqrt(Rsq - mathabs(x)) z = mathrandom(-new, new) else z = mathrandom(-R, R) local new = mathsqrt(Rsq - mathabs(z)) x = mathrandom(-new, new) end local y = math.sqrt(Rsq - ((x^2)+(z^2))) objectSpace = CFrame.new(x, y, z) end local cframe = part.CFrame:toWorldSpace(objectSpace) local angle = CFrameAngles(0,mathrad(mathrandom(0,360)),0)
Grass.CFrame = cframe * angle Grass.Parent = Model end Model.Parent = FoliageModel end |
|
|
| Report Abuse |
|
|
|
| 14 Sep 2016 08:29 AM |
Thank you kindly for your efforts, but FoliageModel:ClearAllChildren() not only removes all the foliage generated meaning only one is 'rendered' at a time, but also requires that it be regenerated on the next cycle, which spikes the activity.
The reason removing Grass.Parent = model cuts the execution time is because it is being placed into workspace which is probably where most of the efforts are going.
I've worked on putting some well placed wait()s into the script to help spread the activity so it isn't suddenly spiked because I don't honestly thing there's any other way to improve it. |
|
|
| Report Abuse |
|
|
UFAIL2
|
  |
| Joined: 14 Aug 2010 |
| Total Posts: 6905 |
|
|
| 14 Sep 2016 08:45 AM |
bat did pretty much every optimization I did except he forgot to do this
local partCFrame = part.CFrame local toWorldSpace = partCFrame.toWorldSpace
-- for loop -- code to gen object space
local cframe = toWorldSpace(partCFrame, objectSpace) |
|
|
| Report Abuse |
|
|
|
| 14 Sep 2016 08:47 AM |
When I was removing the benchmarks code, I accidently left in FoliageModel:ClearAllChildren() Had to use it or my studio would crash |
|
|
| Report Abuse |
|
|
|
| 14 Sep 2016 08:47 AM |
why declare those math values by hand? you can do this local ignore={noise=true;} local ke,va={},{} for k in next,math do if not ignore[k] then ke[#ke+1],va[#va+1] = k,"math." .. k end end
print("local " .. table.concat(ke,", ") .. " = " .. table.concat(va,", ")) --prints declaration of math vars
also thats a lot of code you got there |
|
|
| Report Abuse |
|
|
|
| 14 Sep 2016 08:50 AM |
local CFrameAngles = CFrame.Angles local Instancenew = Instance.new local CFrametoWorldSpace = CFramenew(0, 0, 0).toWorldSpace
local cframe = CFrametoWorldSpace(part.CFrame, objectSpace) local angle = CFrameAngles(0,mathrad(mathrandom(0,360)),0)
Grass.CFrame = cframe * angle Grass.Parent = Model |
|
|
| Report Abuse |
|
|
|
| 14 Sep 2016 08:59 AM |
You're right Fluffy A lot easier, but I'm not sure which I wanna use
local Instancenew = Instance.new local CFrametoWorldSpace = CFramenew(0, 0, 0).toWorldSpace
local _math = { abs, ceil, floor, rad, random, sqrt } local _CFrame = { new, Angles } for i,v in pairs(_math) do _math[i] = v _math[i] = math[i] end for i,v in pairs(_CFrame) do _CFrame[i] = v _CFrame[i] = CFrame[i] end
------------------------------
local mathceil = math.ceil local mathfloor = math.floor local CFramenew = CFrame.new local mathrandom = math.random local mathsqrt = math.sqrt local mathabs = math.abs local mathrad = math.rad local CFrameAngles = CFrame.Angles local Instancenew = Instance.new |
|
|
| Report Abuse |
|
|
|
| 14 Sep 2016 09:01 AM |
| all yours does is remove a "." but i don't see how thats helpful but ok |
|
|
| Report Abuse |
|
|
|
| 14 Sep 2016 09:02 AM |
| They both do the same thing |
|
|
| Report Abuse |
|
|
|
| 14 Sep 2016 09:03 AM |
no lol mine will convert a table into shorthand
cos(x) pi huge sin(x) |
|
|
| Report Abuse |
|
|
| |
|
|
| 14 Sep 2016 09:12 AM |
| um well duh you gotta run my code and copy the output into a script |
|
|
| Report Abuse |
|
|
|
| 14 Sep 2016 09:14 AM |
local ignore={noise=true;} local ke,va={},{} for k in next,math do if not ignore[k] then ke[#ke+1],va[#va+1] = k,"math." .. k end end
print("local " .. table.concat(ke,", ") .. " = " .. table.concat(va,", "))
>> local log, acos, ldexp, huge, pi, rad, tanh, deg, tan, cosh, cos, random, sinh, frexp, ceil, floor, randomseed, abs, sqrt, modf, asin, min, pow, fmod, max, atan2, exp, sin, atan = math.log, math.acos, math.ldexp, math.huge, math.pi, math.rad, math.tanh, math.deg, math.tan, math.cosh, math.cos, math.random, math.sinh, math.frexp, math.ceil, math.floor, math.randomseed, math.abs, math.sqrt, math.modf, math.asin, math.min, math.pow, math.fmod, math.max, math.atan2, math.exp, math.sin, math.atan
|
|
|
| Report Abuse |
|
|
| |
|
|
| 14 Sep 2016 09:15 AM |
| But what do you do if you need Instance.new and CFrame.new ?_? |
|
|
| Report Abuse |
|
|
|
| 14 Sep 2016 09:18 AM |
convert both and ignore "new" or don't convert then just add like CF() or Create() |
|
|
| Report Abuse |
|
|
|
| |
|
|
| |
|
»
»
|
|
|
|
|