Which Code is More Optomized

Help and Feedback Code Review
, ,
#1

Hey,

Both of these sections of code accomplish the same thing and both are 100% fine. This code runs when a player presses the ‘E’ key. The client checks if the ‘E’ key is enabled, and then this code, on the server side, runs. Essentially, if there are any ‘gems’ within 20 studs, the gem will play a sound.

Both of the below are code samples and do not represent the code in its entirety.

for i, gem in ipairs(game.Workspace.Gems:GetChildren()) do

		local distance = (gem.Position - root.Position).Magnitude

		if distance <= maxDistance then
			
			local gemSound = Instance.new('Sound')
			gemSound.Parent = gem
			gemSound.SoundId = 'rbxassetid://7383525713'
			
			print(distance)
			print('gem')
			
			wait(1)
			gemSound:Play()
			wait(1)
			gemSound:Destroy()
			
		end

	end

Essentially, for every gem that is within the max distance, a sound is inserted into that gem, the sound is played, and then the sound is destroyed.

for i, gem in ipairs(game.Workspace.Gems:GetChildren()) do

		local distance = (gem.Position - root.Position).Magnitude

		if distance <= maxDistance then
			
			for i, child in ipairs(gem:GetChildren()) do
				
				if child.ClassName == 'Sound' then
					
					wait(1)
					child:Play()
					
				end
				
			end
			
			print(distance)
			print('gem')
			
		end

	end

The above code essentially finds a sound that is already placed inside of the gem, and plays it.

My question is, which one is more optimized? As it stands, this code will probably be run hundreds of times in one round, so I thought I should ask.

Thanks for any replies. If you need any more information to answer this, feel free to ask :slight_smile:.

1 Like
#2

Instancing is costlier than referencing. Parenting is costly too. I would say the latter. Though there’s some optimization to be done to that already:

for _, gem in pairs(game.Workspace.Gems:GetChildren()) do
    if (gem.Position - root.Position).Magnitude > maxDistance then continue end

    local sound = gem:FindFirstChildWhichIsA("Sound")
    if sound then 
        task.wait(1)
        sound:Play()
    end
end

pairs is better performant than ipairs for scenarios where the order in which they’re iterated doesn’t actually matter, which yours should not, given the application.
Finding a child of a datatype is a lot easier than iterating all children should you know by its architecture there will only be one sound.
Ideally you should drop the waiting, as it will yield for each iteration of gem.

3 Likes
#3

That’s really awesome! Thanks for replying!

I completely forgot about FindFirstChildWhichIsA().

I agree, but right now the player’s sonar sound runs at the same time as the gem sound, making the sonar useless. That’s why the wait was one second.

#4

If you need these sounds to be inside of each gem, you should call them with the same name so you can do simply gem.Sound to index the sound.

2 Likes