Should I change my code or is it good?

Hello, I am wondering if my fall damage script should be more optimized in any way so no lag happens in-game, if so, any to all support helps

if new == Enum.HumanoidStateType.Freefall then
		freeFallPos = HRP.Position.Y
		velocity = HRP.Velocity.Y
		
		connection = RS.Stepped:Connect(function()
			task.wait()
			local newPos = HRP.Position.Y
			print(freeFallPos)
			
			if freeFallPos > newPos then
				print("Disconnecting")
				print(freeFallPos)
				connection:Disconnect()
			end
			freeFallPos = newPos
		end)
		
		fallStart = tick()

	elseif new ~= Enum.HumanoidStateType.Landed then
		local fallStop = tick()
		local fallTime = fallStart - fallStop
		
		local newPos = HRP.Position.Y
		local magnitudePos = (freeFallPos - newPos)
		
		if magnitudePos >= 20 then
			takeDamage = true
			if takeDamage == true then
				Hum:TakeDamage(math.abs((magnitudePos - fallTime) - (.25 * velocity)))
				print(math.abs((magnitudePos - fallTime) - (.25 * velocity))*-1)
				takeDamage = false
				freeFallPos = 0
			end
		  end
            end
     end)

Also if you’re wondering, yes, the script does work.

I do want to mention firstly, it looks great but second I want to comment that different people have different ways of approaching problems and so, of course, each result may have different speeds but however unless you’re programming a server application handling with lots of data, it doesn’t really matter if you say 1ms or 2. Anyways it looks great don’t worry too much about script speeds.

2 Likes

Thanks for the support, glad to see it won’t terribly affect my game. Thanks for the support. Just wanted to make sure no memory leaks or lag will happen because of this!

Not too shabby.

Is there a reason why you have a wait statement within your Stepped event listener function though? That function will still be executed every frame regardless of the wait statement, so it’s kind of redundant to have that in there. (the event is internally handled and all of the attached listener functions to said event fire independently, or as separate coroutines so they don’t rely on the previous call finishing execution first, if that makes sense)

1 Like

I just have a habit of adding waits for loops, I know I don’t particularly need it there, I just added it just in case I don’t crash studio.

1 Like

Understandable.

Just note though, Stepped is not actually a loop. It’s an event, calling :Connect(function f) on an event object registers that function to be executed/ran when the event occurs.

Specifically, the Stepped event is fired every frame. (makes sense as to why you thought it was a loop.)

Hope it helps.

1 Like

Oh I see, thanks for the help! This helped me a lot more!

1 Like

I did my fall damage differently tell me if you want me to show you the code ? :wink:

I would love to see in how both would compare!

1 Like

Here the code :wink: :

--Variable--

local Player = game.Players.LocalPlayer
local Character = Player.Character
local Humanoid = Character:WaitForChild("Humanoid")
local RootPart = Character:WaitForChild("HumanoidRootPart")
local RunService = game:GetService("RunService")
local TouchWater = false
local Y
local YF
local YL

--Variable--

--CheckIfThePlayerIsInWater--

RunService.Heartbeat:Connect(function()
	local BodyPos = Player.Character.HumanoidRootPart.CFrame
	local x, y, z = BodyPos.X, BodyPos.Y, BodyPos.Z

	local min, max = Vector3.new(x + 0.01, y + 0.01, z + 0.01), Vector3.new(x - 0.01, y - 0.01, z - 0.01)
	local region = Region3.new(max, min)
	region = region:ExpandToGrid(4)

	if region then
		local material = workspace.Terrain:ReadVoxels(region, 4)
		if material[1][1][1] == Enum.Material.Water then
			TouchWater = true
		else
			TouchWater = false
		end
	end
end)

--CheckIfThePlayerIsInWater--

--SetVariable/ApplyDamage--

Humanoid.StateChanged:Connect(function (oldState, newState)
	if newState == Enum.HumanoidStateType.Freefall then
		YF = Y
	end
end)

Humanoid.StateChanged:Connect(function (oldState, newState)
	if newState == Enum.HumanoidStateType.Landed then
		YL = Y
		if YF - YL > 100 and TouchWater then --MinEightForTakeDamageIfThePlayerTouchWater--
			Humanoid:TakeDamage(100) --ApplyDamage--
		else
			if YF - YL > 30 then  --MinEightForTakeDamage--
				Humanoid:TakeDamage(YF-YL) --ApplyDamage--
			end
		end
	end
end)

----SetVariable/ApplyDamage--

--GetY--

while true do
	Y = Character:WaitForChild("HumanoidRootPart").Position.Y
	wait()
end

--GetY--
1 Like

Why do you call the terrain like a table? And do it 3 times?

material[1][1][1]
1 Like

I just know that’s how you get the material, if you don’t put them on you won’t get the material!
Try printing it in the output, you’ll see what I’m talking about! :wink:

1 Like

The reason is because Terrain is in Voxels. Basically it’s divided up into lots of evenly spaced points. When you are getting data about a terrain, it sends you a list of those points. Since you are working in 3 dimensions you can think of it as the list like this (I didn’t check if this is the actual order because I’m lazy, but it’s the general concept)

material[x][y][z]

You essentially are giving it coordinates. They aren’t actually game coordinates though. You are essentially just going through all of the nodes one by one that it returned so your really asking for it’s relative location.

Personally for this I would probably just try to use a raycast but this method will work.

As for your original question (though it’s 11 days old). Your code really doesn’t have any excessive loops. Computers are so fast that a few steps aren’t really an issue. If your code took 100 times longer to run than it does now it probably still wouldn’t noticeably impact performance. The things that tend to impact performance are large loops. Specifically large nested loops with no yields are really bad. Since you aren’t looping though, you’re not even getting close to what a computer is capable of. Sure there are very small optimizations that can be made to your code, but the effect of those since you are only running each line once per frame max are negligible. It would just be a waste of time to try optimizing this. You should really only optimize code if you need to.

1 Like