I wanted to improve my Roblox scripting skills, and the first thing that I have done after learning all the basic concepts is inspecting scripts/codes in the Toolbox. The first item that I have inspected its code is the ordinary and normal Roblox sword tool.
When I inspected the code, I notice some problems:
-
The code itself is poorly organised, meaning the file structure of the code is really messy which makes the code less readable.
-
There are many unnecessary checks for the code, and I believe these checks are meant to ensure the sword works properly under certain circumstances.
function CheckIfAlive()
return (((Player and Player.Parent and Character and Character.Parent and Humanoid and Humanoid.Parent and Humanoid.Health > 0 and Torso and Torso.Parent) and true) or false)
end
if not Tool.Enabled or not ToolEquipped or not CheckIfAlive() then
return
end
function Blow(Hit)
if not Hit or not Hit.Parent or not CheckIfAlive() or not ToolEquipped then
return
end
local RightArm = Character:FindFirstChild("Right Arm") or Character:FindFirstChild("RightHand")
if not RightArm then
return
end
local RightGrip = RightArm:FindFirstChild("RightGrip")
if not RightGrip or (RightGrip.Part0 ~= Handle and RightGrip.Part1 ~= Handle) then
return
end
local character = Hit.Parent
if character == Character then
return
end
local humanoid = character:FindFirstChildOfClass("Humanoid")
if not humanoid or humanoid.Health == 0 then
return
end
local player = Players:GetPlayerFromCharacter(character)
if player and (player == Player or IsTeamMate(Player, player)) then
return
end
UntagHumanoid(humanoid)
TagHumanoid(humanoid, Player)
humanoid:TakeDamage(Damage)
end
- Poor management on performance as there is unnecessary memory leaks, mainly caused by events not being disconnected when they are no longer used in the code.
- The sword does damage when the player doesn’t activate (click) it. Also, the sword does additional damage to other targets even if the targets were hit by the sword already.
So after figuring out all of the problem addressed above, I have decided to test my Roblox scripting skills by creating a brand new script which is the exact same as the old script but better, which solves all the problems mentioned above. First, I have made a simple flowchart which describes the whole sword system:
After that, I have coded the sword system by following the flowchart via creating one server script:
local Players = game:GetService("Players")
local DAMAGE_TABLE = {
SLASH_DAMAGE = 10,
LUNGE_DAMAGE = 50
}
local ANIMATIONS_TABLE = {
Slash = "rbxassetid://16935567961",
Lunge = "rbxassetid://16938434031"
}
local lastAttacked = 0
local attackCooldown = false
local handleTouchedDebounce = {}
local swordTool = script.Parent
local swordHandle = swordTool.Handle
local attackAnim = swordTool.AttackAnim
local character = swordTool.Parent.Parent.Character
local humanoid = character:WaitForChild("Humanoid")
local animator : Animator = humanoid:WaitForChild("Animator")
local swordActivatedConnection : RBXScriptConnection = nil
local playAnimationConnection : RBXScriptConnection = nil
local function playAnimation(animationType)
if animationType == "Slash" then
attackAnim.AnimationId = ANIMATIONS_TABLE.Slash
elseif animationType == "Lunge" then
attackAnim.AnimationId = ANIMATIONS_TABLE.Lunge
else
error("function playAnimation received an unknown type: "..animationType)
end
local animTracks : AnimationTrack = animator:LoadAnimation(attackAnim)
animTracks:Play()
return animTracks
end
local function onActivated()
if not attackCooldown then
attackCooldown = true
local touchedEvent : RBXScriptConnection = nil
local attackAnimTracks : AnimationTrack = nil
local attackType
local targetDamaged
local damage
local currentTick = tick()
local timeDifference = currentTick - lastAttacked
if timeDifference > 2 then -- SLASH
attackType = "Slash"
attackAnimTracks = playAnimation("Slash")
damage = DAMAGE_TABLE.SLASH_DAMAGE
else -- LUNGE
attackType = "Lunge"
attackAnimTracks = playAnimation("Lunge")
damage = DAMAGE_TABLE.LUNGE_DAMAGE
end
touchedEvent = swordHandle.Touched:Connect(function(hit)
local attackTarget
attackTarget = hit.Parent
if attackTarget.ClassName == "Model" and (not handleTouchedDebounce[attackTarget]) then
handleTouchedDebounce[attackTarget] = true
local humanoid = attackTarget:FindFirstChildWhichIsA("Humanoid")
if humanoid and not Players:GetPlayerFromCharacter(attackTarget) then
humanoid:TakeDamage(damage)
targetDamaged = true
end
end
if targetDamaged then
attackAnimTracks.Ended:Wait()
handleTouchedDebounce[attackTarget] = nil
end
end)
playAnimationConnection = attackAnimTracks:GetMarkerReachedSignal(attackType):Connect(function()
local attackSound : Sound = swordHandle[attackType]
attackSound:Play()
attackSound.Ended:Wait()
playAnimationConnection:Disconnect()
end)
attackAnimTracks.Ended:Wait()
touchedEvent:Disconnect()
lastAttacked = currentTick
attackCooldown = false
end
end
local function onUnequipped()
if swordActivatedConnection ~= nil then
swordActivatedConnection:Disconnect()
end
end
local function onEquipped()
if humanoid.Health ~= 0 then
swordHandle.Unsheath:Play()
swordActivatedConnection = swordTool.Activated:Connect(onActivated)
end
end
swordTool.Equipped:Connect(onEquipped)
swordTool.Unequipped:Connect(onUnequipped)
After I have conducted some testing, the code works well. Here’s why I believe my code works better than the old script:
- The sword now only does damage ONCE to each different target when the sword handle touches different targets. After the sword attacking animation is complete, the sword can later deal damage to those different targets again, without dealing additional unnecessary damage in one single hit which could result an instant death of the target.
- The sword is better in terms of performance, as the script makes sure that, when the sword is unequipped (or not equipped), it disconnects the
Activatedevent which is responsible for dealing damage to targets. - The script/code itself is more organised, which increases readability.
Here’s a video demonstrating how the new sword works.
However, I believe there are still some rooms for improvement:
- Is my code organised? Where should I improve in order to improve its readability?
- Is my script optimised? In which part of my script should I pay attention and try to optimise?
- What are the other disadvantages that my script can’t do? How should I approach to solving it?
- Are there any other suggestions which I can add in my script?
- Is there any other decomposition diagram maker or flowchart maker for me to create flowcharts like the one above?
I hope my script can be thoroughly analysed and interpreted by anyone for feedbacks and constructive criticisms. Should there be any questions regarding the script itself, please let me know!
Thanks.