How can i improve this diving code

local player = game:GetService("Players").LocalPlayer
local Character = player.Character
local humanoid = Character:WaitForChild("Humanoid")
local RootPart = Character:WaitForChild("HumanoidRootPart")

local ContextActionService = game:GetService("ContextActionService")
local UserInputService = game:GetService("UserInputService")

local DivingAnimation = script:WaitForChild("Animation")
local DivingLoad = humanoid.Animator:LoadAnimation(DivingAnimation)

local RollingAnimation = script:WaitForChild("RollingAnim")
local RollingLoad = humanoid.Animator:LoadAnimation(RollingAnimation)


local function ChangeJumpPower(jumppower)
	if Character ~= nil then
		local humanoid = Character:FindFirstChild("Humanoid")
		if humanoid ~= nil then
			humanoid.JumpPower = jumppower
		end
	end
end



local function MidAirLongJumpHandler(actionName,userInputState,inputObject)
	if userInputState == Enum.UserInputState.Begin then
		if humanoid:GetState() == Enum.HumanoidStateType.Freefall then
			if not Debounce then
				LongJumpDebounce = true
				inDiving = true
				Debounce = true
				ChangeJumpPower(0)
				DivingLoad:Play()
				
				RootPart:ApplyImpulse(RootPart.CFrame.LookVector * (1050))
				workspace.Gravity = 145.8
				RootPart:ApplyImpulse(RootPart.CFrame.YVector * (200))
				task.wait(0.25)
				workspace.Gravity = 180.2
				ChangeJumpPower(50)
				
				humanoid.FreeFalling:Connect(function(active)
					if inDiving then
						if active then
							return
						else
							DivingLoad:Stop()
							RollingLoad:Play()
							inDiving = false
							task.wait(0.6)
							LongJumpDebounce = false
							Debounce = false
						end
					end
				end)
			end
              end
       end
end
ContextActionService:BindAction("Mid Air Long Jump", MidAirLongJumpHandler, true, Enum.KeyCode.X)
ContextActionService:SetPosition("Mid Air Long Jump", UDim2.new(0.100, 0,0.700, 0))

it works but how i make it better
for like example make it smother or use something other than applyimpulse

im not a good developer and im trying to learn how to make good code for the game

1 Like

One thing you can do is make this script run once and work with the player on an input-basis, making it more robust. For example, you’re ChangeJumpPower function could be changed to:

local LocalPlayer = Players.LocalPlayer

local function ChangeJumpPower(Value)
	local Character = LocalPlayer.Character
	-- This is like a short-hand if statement
	-- See: https://luau-lang.org/syntax#if-then-else-expressions
	local Humanoid = if Character then Character:FindFirstChildWhichIsA("Humanoid") else nil
	
	if Character and Humanoid then
		Humanoid.JumpPower = Value
	end
end

And it will change the JumpPower when the Character and Humanoid exists, without needing to reload the script.

The same could be done with the MidAirLongJumpHandler as well, where it checks if the Character and Humanoid exist rather than just expecting them to exist.

Nesting

This might be a preference but nesting conditional statements can get annoying to look at or deal with over time and it adds unnecessary indentation. One rule you’ll see other’s suggest is not nesting more than 3 layers deep as it provides better legibility in most cases. (Although yours doesn’t go beyond 3 deep)

Turning

if userInputState == Enum.UserInputState.Begin then
		if humanoid:GetState() == Enum.HumanoidStateType.Freefall then
			if not Debounce then

Into

if userInputState ~= Enum.UserInputState.Begin then return end
if humanoid:GetState() ~= Enum.HumanoidStateType.Freefall then return end
if Debounce then return end

It’s better doing this when you don’t have anything proceeding the conditional statements that need to be ran. But you could turn that into a function itself if you did. You could probably read up or watch videos about why over nesting isn’t that great.


Disposing/Disconnecting Unneeded Used Connections

I’d also like to note that you create a connection but never store the connection in a variable for later disposal/disconnect, so each time you do this MidAirLongJump it’s creating a new connection for the exact same thing.


Case Consistency / Naming Conventions

You might or might not have a naming convention you already decided, but I noticed you have mismatching cases for instances like humanoid (lowercase h) over your normal scheme like Character (capital C). I’m definitely not a professional but keeping how you name variables consistent across variable types is very much appreciated by your future self or other people that might be working with you. If you want to dig more into this, you can look up Programming naming conventions. This should be one of the hardest parts about programming is just naming your variables & functions something that makes sense.


Type Annotation / Commenting

This is definitely a preference and I know a lot of people don’t annotate when dealing with personal projects but it’s an absolute life saver for your future self or co-workers. It allows autocomplete to be used and lets anyone know what the value should be. And about comments, personally it makes it easier to understand what the Function should do. Commenting on easy-to-understand tasks isn’t unnecessary.

I personally strongly suggest doing this for complicated processes as I personally had to re-read my non-commented/documented code regarding math and it took a while to jot down and understand it again.

-- // Allows auto-complete to work it's wonders
-- // even without the variable being set
local humanoid : Humanoid

-- // Allows the coder to know what the variables should be
-- // when using the function either in that same script or another (modules)
local function foo(var1 : string, var2 : number?)
end

Parenthesis In Conditional Statements

This isn’t completely necessary 100% of the time, but adding parenthesis can help increase legibility sometimes for you and others.

-- // Multi-lined so it doesn't need scrolling
if userInputState ~= Enum.UserInputState.Begin 
    and humanoid:GetState() ~= Enum.HumanoidStateType.Freefall
    and Debounce then return end

-- // into this
if not (userInputState == Enum.UserInputState.Begin
    and humanoid:GetState() == Enum.HumanoidStateType.Freefall)
    or (Debounce) then return end

Minor Convenience

Random convenience for UDims, if you’re only using scale or pixel offset, you could use UDim2.fromScale() or UDim2.fromOffset() as it only needs 2 parameters.

4 Likes

This topic was automatically closed 14 days after the last reply. New replies are no longer allowed.