Double Jump Script Criticize me

Help and Feedback Code Review
#1

I want to be criticized till I lose hope of life

Here’s a simple Double Jump Script I wrote in 20 mins

--[[ --> // Pseudocode for Double Jump Script
	Event for detecting when the player wants to jump
	When detected, (With UserInputService ofc)
	Let the player Jump and play jump sound
	Record this in a variable (+1 Jump)
	Check if humanoid state is either FreeFall, Jumping
	If JumpRequest called again, force jump (With ChangeState() function of Humanoid) and play sound
	After Humanoid Floor Material is NOT air, reset the jump to 0
]]
local Player = game.Players.LocalPlayer
local Humanoid = Player.Character:WaitForChild("Humanoid")
local UIS = game:GetService("UserInputService")
local JSound = script:WaitForChild("jump")
local HumanoidStateChanged
local CanJump = true
local Jumps = 0
local Jumped = false

local function HasReachedGround() --determines if player has reached the ground
	if Humanoid.FloorMaterial == Enum.Material.Air then
		return false
	else
		return true
	end
end

UIS.JumpRequest:Connect(function() -- Detects whether player wants to jump (Works in all platforms)
	if CanJump == false then return end --Dismisses function if player has used up all jumps
	if Jumps >= 2 then return end --Dismisses function when player used all jumps too
	Jumps = Jumps + 1 
	CanJump = true --Think of this as 'CanDoubleJump'
	
	if Jumped == true then --If jumped the first time, run this part of the code
		if JSound.Playing then
			JSound:Stop()
		end
		JSound:Play() --Play the jump sound
		Humanoid:ChangeState(Enum.HumanoidStateType.Jumping)
		Jumps = Jumps + 1
		Jumped = false --We dont want this part of the code to run again
		HumanoidStateChanged:Disconnect() --Disconnect the event below this code, player cant jump again
		print("Cannot Jump again")
		CanJump = false
		repeat wait() until(HasReachedGround() == true) --Wait until player has reached the ground
		CanJump = true
		Jumps = 0 --Reset jump count
		return --Dismiss function since we don't want the code below to run again
	end
	
	Jumped = true
	JSound:Play()
	HumanoidStateChanged = Humanoid.StateChanged:Connect(function(Old, New)
		if New == Enum.HumanoidStateType.Landed or Enum.HumanoidStateType.Jumping then --If player is in air or has landed, he can jump again
			CanJump = false --Cannot jump till cooldown finishes
			wait(.1) --Prevent excessive spam
			CanJump = true --Can jump again after short cooldown
			print("Can jump again!")
		end
	end)
	
end)
4 Likes
#3

cracks fingers Ask and you shall receive.

I actually recently did a code review for someone else asking about a triple jump script and I refactored it to be an x jump script where it’s as easy as setting the maximum jumps. So, I’ll first run through the issues I see in your code, point out the good stuff, and then leave you with that refactored script for reference.

The Good

You’re using events

Thank god you’re using events (I believe the roblox scripting lingo term is a signal connection). It’s amazing how many people wrap their stuff in large wait loops; it’s abysmal both in performance and maintainability.

You’re keeping your code consistently styled

To me this is a no-brainier, but I’ve been surprised how many people change their style on a day-to-day basis; what’s even worse, a codebase with several conflicting styles. I don’t see that here. Everything you’re doing seems to be PascalCase, and as long as you’re consistent then that’s perfectly fine.

You used #help-and-feedback:code-review

Genuinely, this category is underused. The fact you’re open to criticism puts you beyond most others in my eyes. So definitely keep it up.

The bad

Your comments do no good

A lot of your comments explain what your code is doing, not explaining why you are having to do something a certain way. You and anyone else reviewing your Lua code should know Lua and, thus, can be perfectly capable of reading what it’s doing.

Also, I believe your pseudo code falls under this category. I would omit that.

A case in point is the following line:

Next time, call the variable CanDoubleJump.

Don’t over-complicate Roblox’s API

That’s a lot of code to achieve a double jump. Instead of rebuilding a lot of the native API from the ground up, research the official documentation to find more efficient ways of doing what you wish to achieve.

Write idiomatic Lua code

I suppose this takes more experience than anything, but write idiomatic Lua code. For instance, a function that looks like so:

…could just be written like so:

local function HasReachedGround()
    return Humanoid.FloorMaterial ~= Enum.Material.Air;
end

The Ugly

Syke! Why would I post ugly code? Here is my rendition of a x jump script. It is a LocalScript placed under StarterCharacterScripts which can be found under StarterPlayer:

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

local character = script.Parent;
local humanoid = character:WaitForChild("Humanoid");

local canJump = true;
local currentJump = 0;

local CHECK_DELAY_IN_SECONDS = 0.2;
local MAX_JUMPS = 2;

--- An event listener for Humanoid.StateChanged that manages if and when a
--- Player's character can achieve more than one consecutive jump.
--- @param _ HumanoidStateType (just in case)
--- @param newState HumanoidStateType
--- @return nil
local function manageConsecutiveJumps(_, newState)
    if newState == Enum.HumanoidStateType.Jumping then
		canJump = false;
		wait(CHECK_DELAY_IN_SECONDS);
        currentJump = currentJump + 1;
		canJump = currentJump < MAX_JUMPS;
    elseif newState == Enum.HumanoidStateType.Landed then
        currentJump = 0;
		canJump = true;
	end
end

--- An event listener for UserInputService.InputBegan that dispatches the jump
--- input to the character when appropriate.
--- @param inputObject InputObject
--- @return nil
local function dispatchConsecutiveJumps(inputObject)
    local shouldDispatch = (
        inputObject.KeyCode == Enum.KeyCode.Space
		and humanoid:GetState() ~= Enum.HumanoidStateType.Jumping
        and canJump
    );

    if shouldDispatch then
        humanoid:ChangeState(Enum.HumanoidStateType.Jumping);
    end
end

humanoid.StateChanged:Connect(manageConsecutiveJumps);
UserInputService.InputBegan:Connect(dispatchConsecutiveJumps);

As always, feel free to ask any questions as to my rationales on why I did something the way I did! Also, keep in mind my code is by no means a prescriptive solution, but just a perspective and reference you can utilize to better inform your own style and code.

8 Likes
#4

First of all, thank you for taking your precious quarantine time to look at that ugly code snippet I made. Not many people actually write in-depth reviews as this. I was quite surprised when I saw this post was more than the usual 3 line-replies in most code reviews. Anyways, here are my comments on this.

My Thoughts on

1. “… Roblox’s API” part

I must admit, I’ve spent so much of my time as a developer developing my decision-making skills, but on the other hand, disregarding learning more of the API itself. Definitely will take this into consideration in any future anything I code.

2. Idiomatic Lua code

This is my first time to encounter this, but I’m glad I did nonetheless. I guess this also is kind of related to my limited knowledge of the Roblox API capabilities.

Final Comments

From your code, I can tell that you have experience in much more other languages. Other than the obvious fact that you use semi-colons at the end of each line of code, you’ve fully utilized the capabilities of RLua like the and for initializing shouldDispatch, which I almost never use. I shall therefore take insipiration from this to improve my code.

The fact that this made me realize how ugly my code is doesn’t change the fact that this review was something I definitely needed. And no, I have no questions, your code is already too concise to be questioned.

Thanks for reply :sweat_smile:

1 Like