How could I organize this code better?

Help and Feedback Code Review
1

How could I organize this code better

LocalScript inside of Tool:

local Player = game:GetService("Players").LocalPlayer

local Character = Player.Character or Player.CharacterAdded:Wait()
local Humanoid = Character:WaitForChild("Humanoid")

local Animator = Humanoid:WaitForChild("Animator")

local AnimationsFolder = script:WaitForChild("Animations")

local R15Folder = AnimationsFolder:WaitForChild("R15")
local R6Folder = AnimationsFolder:WaitForChild("R6")

local R15ToolEquipAnimation = R15Folder:WaitForChild("R15ToolEquip")
local R15ToolIdleAnimation = R15Folder:WaitForChild("R15ToolIdle")

local R6ToolEquipAnimation = R6Folder:WaitForChild("R6ToolEquip")
local R6ToolIdleAnimation = R6Folder:WaitForChild("R6ToolIdle")

local ToolEquipAnimation
local ToolIdleAnimation

local Tool = script.Parent

if Humanoid.RigType == Enum.HumanoidRigType.R15 then
	ToolEquipAnimation = Animator:LoadAnimation(R15ToolEquipAnimation)
	ToolIdleAnimation = Animator:LoadAnimation(R15ToolIdleAnimation)
elseif Humanoid.RigType == Enum.HumanoidRigType.R6 then
	ToolEquipAnimation = Animator:LoadAnimation(R6ToolEquipAnimation)
	ToolIdleAnimation = Animator:LoadAnimation(R6ToolIdleAnimation)
end

Tool.Equipped:Connect(function()
	ToolEquipAnimation:Play()

	ToolEquipAnimation.Stopped:Connect(function()
		ToolIdleAnimation:Play()
	end)
end)

Tool.Unequipped:Connect(function()
	for _, AnimationTrack in pairs(Animator:GetPlayingAnimationTracks()) do
		if AnimationTrack.Name == "R15ToolIdle" or AnimationTrack.Name == "R6ToolIdle" then
			AnimationTrack:Stop()
		end
	end
end)
2

For this, you can try to indent more, instead of having the wall of code look like a wall, and space it up a little


if Humanoid.RigType == Enum.HumanoidRigType.R15 then
	ToolEquipAnimation = Animator:LoadAnimation(R15ToolEquipAnimation)
		ToolIdleAnimation = Animator:LoadAnimation(R15ToolIdleAnimation)
	elseif Humanoid.RigType == Enum.HumanoidRigType.R6 then 
		ToolEquipAnimation = Animator:LoadAnimation(R6ToolEquipAnimation)
	ToolIdleAnimation = Animator:LoadAnimation(R6ToolIdleAnimation)
end

You could also try to make the variables a bit more compact (If you want to) or, try to put it within notes like this


--VARIABLES--

local Player = game:GetService("Players").LocalPlayer
local Character = Player.Character or Player.CharacterAdded:Wait()
local Humanoid = Character:WaitForChild("Humanoid")
local Animator = Humanoid:WaitForChild("Animator")
local AnimationsFolder = script:WaitForChild("Animations")
local R15Folder = AnimationsFolder:WaitForChild("R15")
local R6Folder = AnimationsFolder:WaitForChild("R6")
local R15ToolEquipAnimation = R15Folder:WaitForChild("R15ToolEquip")
local R15ToolIdleAnimation = R15Folder:WaitForChild("R15ToolIdle")
local R6ToolEquipAnimation = R6Folder:WaitForChild("R6ToolEquip")
local R6ToolIdleAnimation = R6Folder:WaitForChild("R6ToolIdle")
local ToolEquipAnimation
local ToolIdleAnimation
local Tool = script.Parent

--END--

all of these can help to make the code look shorter, and to help you understand it better, but if you are trying to make it so that the code IS shorter, then you have done a pretty good job

3

Please DO NOT indent like this. This is not standard and does not make it more legible.

2 Likes
4

Thats just personally how I learned, by seperating it in the beggining

But of course, one to ones own

5

Thank you.

I have left the code like this:

if Humanoid.RigType == Enum.HumanoidRigType.R15 then
	ToolEquipAnimation = Animator:LoadAnimation(R15ToolEquipAnimation)
	ToolIdleAnimation = Animator:LoadAnimation(R15ToolIdleAnimation)
elseif Humanoid.RigType == Enum.HumanoidRigType.R6 then
	ToolEquipAnimation = Animator:LoadAnimation(R6ToolEquipAnimation)
	ToolIdleAnimation = Animator:LoadAnimation(R6ToolIdleAnimation)
end

Should I have a for loop that infinetely checks if the character type is R15 or R6? Will this put more memory on the server? Is the code fine the way it is?

1 Like
6

I’d leave the code as is, and only make adjustments if you see any problems arise.

1 Like
7

The point of indentation is to group scopes together. Each scope is a really big deal with its own flow control and variables, and so it’s important to be able to identify them at a glance and quickly tell which scope a particular line is in. Indentation can also be used to group multi line statements:

part.CFrame= CFrame.new(x, y, Z)
    * CFrame.Angles(a, b, c)
    * CFrame.new(m, n, o);

if a == b and b == c
    or a > b and c == nil then

This is also somewhat important to be able to identify at a glance.
Indentation is not a style, it is quite purposeful. There is only one correct way to do it.

If you want to break up the code, add comments or new lines. Or new scopes so you can minimize it and hide it in the script editor.

local redParts do
    redParts = {}
    for i, v in pairs(workspace:GetDescendants()) do
        if v:IsA("BasePart") and v.BrickColor == BrickColor.red() then
            redParts[#redParts + 1] = v
        end
    end
end
2 Likes
8

didnt do much but yes (should work idk i didnt test lol

local Player = game:GetService("Players").LocalPlayer
local Character = Player.Character or Player.CharacterAdded:Wait()
local Humanoid = Character:WaitForChild("Humanoid")
local Animator = Humanoid:WaitForChild("Animator")

local AnimationsFolder = script:WaitForChild("Animations")
local R15Folder, R6Folder = AnimationsFolder:WaitForChild("R15"), AnimationsFolder:WaitForChild("R6")
local R15ToolEquipAnimation, R15ToolIdleAnimation = R15Folder:WaitForChild("R15ToolEquip"), R15Folder:WaitForChild("R15ToolIdle")
local R6ToolEquipAnimation, R6ToolIdleAnimation = R6Folder:WaitForChild("R6ToolEquip"), R6Folder:WaitForChild("R6ToolIdle")

local ToolEquipAnimation
local ToolIdleAnimation

if Humanoid.RigType == Enum.HumanoidRigType.R15 then
	ToolEquipAnimation = Animator:LoadAnimation(R15ToolEquipAnimation)
	ToolIdleAnimation = Animator:LoadAnimation(R15ToolIdleAnimation)
elseif Humanoid.RigType == Enum.HumanoidRigType.R6 then
	ToolEquipAnimation = Animator:LoadAnimation(R6ToolEquipAnimation)
	ToolIdleAnimation = Animator:LoadAnimation(R6ToolIdleAnimation)
end

script.Parent.Equipped:Connect(function()
	ToolEquipAnimation:Play()
	ToolEquipAnimation.Stopped:Connect(function()
		ToolIdleAnimation:Play()
	end)
end)

script.Parent.Unequipped:Connect(function()
	for _, v in pairs(Animator:GetPlayingAnimationTracks()) do
		if v.Name == "R15ToolIdle" or v.Name == "R6ToolIdle" then
			v()
		end
	end
end)
1 Like
9

I see

Well, thanks for teaching me about it

Guess i should change my ways