Is my fireball code good?

this is the first time i made an ability and i was wandering if i did it good and if you have any suggestions (im also gonna add a hit detection for damege)

local script:

local UserInputService = game:GetService("UserInputService")
local repStorage = game:GetService("ReplicatedStorage")

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

local Animation = script.Animation

local remotesFile = repStorage:WaitForChild("Remots")
local SummonRemote = remotesFile:WaitForChild("FireBall"):WaitForChild("Summon")
local ThrowRemote = remotesFile:WaitForChild("FireBall"):WaitForChild("Throw")

local debounce = false

local function OnInputBegan(input)
	if input.KeyCode == Enum.KeyCode.E and debounce == false then
		debounce = true
		
		local char = player.Character or player.CharacterAdded:Wait()
		local humanoid = char:WaitForChild("Humanoid")
		
		local anim = humanoid:LoadAnimation(Animation)
		anim:Play()
		
		local Summoned
		anim:GetMarkerReachedSignal("Summon"):Connect(function()
			Summoned = SummonRemote:InvokeServer(char)
		end)
		
		local Threw
		anim:GetMarkerReachedSignal("Throw"):Connect(function()
			Threw = ThrowRemote:InvokeServer()
		end)
		
		task.wait(3)
		debounce = false
	end
end

UserInputService.InputBegan:Connect(OnInputBegan)

server script:

local RunService = game:GetService("RunService")
local repStorage = game:GetService("ReplicatedStorage")
local Debris = game:GetService("Debris")

local RemotesFile = repStorage:WaitForChild("Remots")
local FireBallRemotes = 
	{
		Summon = RemotesFile:WaitForChild("FireBall"):WaitForChild("Summon"),
		Throw = RemotesFile:WaitForChild("FireBall"):WaitForChild("Throw")
	}

local Debounce = 
	{
		FireBall = {}
	}

local DebounceTime = 
	{
		FireBall = 3
	} 

FireBallRemotes.Summon.OnServerInvoke = function(player, char)
	if not table.find(Debounce.FireBall, player.UserId) then
		table.insert(Debounce.FireBall, player.UserId)
		
		local hrp = char:WaitForChild("HumanoidRootPart")

		local FireBall = repStorage:WaitForChild("Magics"):WaitForChild("FireBall"):Clone()
		FireBall.Parent = workspace.Ignore

		local Hand = char.RightHand
		FireBall.CFrame = Hand.RightGripAttachment.WorldCFrame

		local weld = Instance.new("WeldConstraint", FireBall)
		weld.Part0 = Hand
		weld.Part1 = FireBall

		FireBallRemotes.Throw.OnServerInvoke = function()
			
			Debris:AddItem(weld,0)
			FireBall.Anchored = true

			local direction = hrp.CFrame.LookVector
			local StartTime = os.clock()

			local connection
			connection = RunService.Heartbeat:Connect(function(dt)

				if os.clock() >= StartTime + 2.5 then
					connection:Disconnect()
					Debris:AddItem(FireBall,0)
				end

				FireBall.CFrame = CFrame.new(FireBall.Position) + direction * dt * 60
			end)
			return true
		end
		
		task.wait(DebounceTime.FireBall)
		for i,v in Debounce.FireBall do
			if v == player.UserId then
				table.remove(Debounce.FireBall, i)
			end
		end
		return true
	end
	return false
end

It does seem pretty good to me, it’s not messy, and it has mainly goos practices, it’s a bit advanced for me but I would say you don’t have anything wrong (I think)

1 Like

Way better than when I first wrote my first ability, but if you wanna go an extra mile read this:

To improve the organization and scalability of the code, you can apply object-oriented programming (OOP) concepts such as classes. Here’s a refactored code that shows how the code might look with OOP applied. However, please note that this code was written in a hurry and has never been tested. If you want to use this code, you should delete the RemoteFunction and replace it with a RemoteEvent called “Fireball”. If you want to add new projectiles beside Fireball, I suggest to create a Projectile class and inherit all future projectiles from it. This will allow for easier extension without modifying the code.

What OOP? Why should I use it?

  • Better readibility
  • Can add more stuff later, without having to spend more time

local script

Warning! The code below does not follow the best practices (like SOLID principles)

local UserInputService = game:GetService("UserInputService")
local ReplicatedStorage = game:GetService("ReplicatedStorage")
local Players = game:GetService("Players")

local FireballThrower = {} --name sucks but it's fine
FireballThrower.__index = FireballThrower

function FireballThrower.new()
    local self = setmetatable({}, FireballThrower)
    self.Animation = script.Animation
    self.Remotes = ReplicatedStorage.Remots.Fireball
    return self
end

function FireballThrower:Cast()
    if self.Debounce then
        return
    end
    
	self.Remotes.FireballThrower:Fire("Create")
    self.Debounce = true
    
    local character = Players.LocalPlayer.Character or Players.LocalPlayer.CharacterAdded:Wait()
    local humanoid = character:WaitForChild("Humanoid")
    local animation = humanoid:LoadAnimation(self.Animation)
    animation:Play()

    animation:GetMarkerReachedSignal("Summon"):Once(function()
        self.Remotes.Fireball:Fire("Summon")
    end)
    
    animation:GetMarkerReachedSignal("Throw"):Once(function()
        self.Remotes.Fireball:Fire("Throw")
    end)
    
    task.wait(3)
    
    self.Debounce = false
end

local FireBall = FireballThrower.new()

UserInputService.InputBegan:Connect(function(input)
    if input.KeyCode == Enum.KeyCode.E then
        FireBall:Cast()
    end
end)




server script

Warning! The code below does not follow the best practices (like SOLID principles)

local RunService = game:GetService("RunService")
local ReplicatedStorage = game:GetService("ReplicatedStorage")

local RemotesFile = ReplicatedStorage:WaitForChild("Remots")

local PlayerFireballs = {}

local Fireball = {}
Fireball.__index = Fireball

function Fireball.new(Player)
    local self = setmetatable({}, Fireball)

    self.Player = Player
    self.Debounce = false

    self.FireballModel = ReplicatedStorage.Magics.Fireball
    self.CurrentFireball = nil

    self.Weld = nil
    self.Character = nil

    return self
end

function Fireball:Summon(Player: Player, Character: Model)
    if self.Debounce then return end
    self.Debounce = true
    self.Character = Character

    self.CurrentFireball = self.FireballModel:Clone()
    self.Weld = Instance.new("WeldConstraint", self.CurrentFireball)
    self.Weld.Part0 = Character.RightHand
    self.Weld.Part1 = self.CurrentFireball
end

function Fireball:Throw()
    self.Weld:Destroy()
    self.CurrentFireball.Anchored = true

    local Direction = self.Character.HumanoidRootPart.CFrame.LookVector
    local StartTime = os.clock()

    local Connection
    Connection = RunService.Heartbeat:Connect(function(DeltaTime)
        if os.clock() >= StartTime + 2.5 then
            Connection:Disconnect()
            self.CurrentFireball:Destroy()
        end

        self.CurrentFireball.CFrame = CFrame.new(self.CurrentFireball.Position) + Direction * DeltaTime * 60
    end)

    self:Destroy()
end

function Fireball:Destroy()
    table.clear(self)
    PlayerFireballs[Player.UserId] = nil
end

RemotesFile.Fireball:OnServerEvent(function(Player: Player, Action: string) --code is kinda meh below
    if not Player then return end --ensure nonsensical value isn't passed.
    
    if Action == "Create" then
        if PlayerFireballs[Player.UserId] then return end

        PlayerFireballs[Player.UserId] = Fireball.new(Player)
    end
    
    if not PlayerFireballs[Player.UserId] then return end

    if Action == "Summon" then
        PlayerFireballs[Player.UserId]:Summon(Player, Player.Character or Player.CharacterAdded:Wait())
    elseif Action == "Throw" then
        PlayerFireballs[Player.UserId]:Throw()
    end
end)
3 Likes