Am I reloading correctly?

[imnotaguest1121 (Diabolical_Subspace)]

Heya there.

I’ve created a simple pistol for a fan recreation of a game called “Noobs vs Zombies: Relish Reborn”. Anyway, I’ve added a reload script alongside sanity checking on the server. I am just wondering if I am doing it correctly and what I should instead improve on.

(Local Script)

--//Reloading
local ReloadCD = false
local function Reloading()
	local ammoContain = Tool:GetAttribute("Ammo_Contain")
	local ammoHold = Tool:GetAttribute("Ammo_Hold")

	--//Checking to see if AmmoHold is less.
	if Tool:GetAttribute("Ammo_Hold") <= AmmoHold then

		--//We're also checking to see if they have enough ammo to even reload it.
		if Tool:GetAttribute("Ammo_Contain") >= 1 then
			--//Reloading Animation
			local ReloadAnim = Player.Character.Humanoid:FindFirstChild("Animator"):LoadAnimation(AnimationFolder.Reload)
			ReloadAnim:Play()

			ReloadAnim:GetMarkerReachedSignal("RackedGunIndication"):Connect(function()
				Tool.Pistol.Reload:Play()
			end)

			ReloadAnim.Stopped:Wait()

			FireEvent:FireServer("Reload")

			Tool:SetAttribute("Ammo_Hold", 12)
			Tool:SetAttribute("Ammo_Contain", ammoContain - 12)
			print("Ammo Remaining: "..ammoContain.."... Better use it wisely.")
		else
			warn("You don't got ammo!! Go near an ammo box or find an ammo noob to refill!!")
			return
		end
	else
		print("Your gun is already loaded! Silly!!")
		return
	end
end
CAS:BindAction("Reload",Reloading,false,Enum.KeyCode.R)

(Server Script)

---It's inside of a OnServerEvent.
if Type == "Reload" then
		local ammoHold = Tool:GetAttribute("Ammo_Hold")
		local ammoContain  = Tool:GetAttribute("Ammo_Contain")
		
		--//Sanity checking
		if ammoHold <= AmmoHold then
			Tool:SetAttribute("Ammo_Hold", 12)
			Tool:SetAttribute("Ammo_Contain", ammoContain - 12)
		else
			warn("Too much ammo. Stop hacking, cheater.")
			return
		end
	end

[PowFPS1]

Server-Side: The ammo checks should be fixed, for instance, Ammo_Hold must be less than 12, while Ammo_Contain should be at least 12. Also, the AmmoHold variable should have a value corresponding to the tool’s attributes.

Cooldown: Add a cooldown for the reload action to prevent spamming in the Local Script.

Animator Check: The script should always check the animator on the player’s character before it tries to set any animation.

Ammo Updates: Both Ammo_Hold and Ammo_Contain must be updated after a reload.

Error Handling: Some handling of events when there is insufficient ammo.

With these adjustments, your reload system should be somewhat better

[imnotaguest1121 (Diabolical_Subspace)]

For the Server-Side, wouldn’t this be the solution?

local ammoHold = Tool:GetAttribute("Ammo_Hold")
		local ammoContain  = Tool:GetAttribute("Ammo_Contain")
		
		--//Sanity checking
		if ammoHold <= AmmoHold then
			
			if ammoHold < 12 then
				if ammoContain > 12 then
					Tool:SetAttribute("Ammo_Hold", 12)
					Tool:SetAttribute("Ammo_Contain", ammoContain - 12)
				end
			end
			
		else
			warn("Too much ammo. Stop hacking, cheater.")
			return
		end

[PowFPS1]

You’re close, but there are a couple of issues

You’re comparing ammoHold to AmmoHold, but AmmoHold isn’t stated anywhere. It should be a fixed value (e.g., 12), as that’s the max capacity of the gun.

In your second if statement, ammoContain > 12 should be ammoContain >= 12 to make sure there’s enough ammo for a full reload.

local ammoHold = Tool:GetAttribute("Ammo_Hold")
local ammoContain = Tool:GetAttribute("Ammo_Contain")

-- Sanity checking
if ammoHold < 12 then  -- Making sure ammo hold is less than 12
    if ammoContain >= 12 then
        Tool:SetAttribute("Ammo_Hold", 12)
        Tool:SetAttribute("Ammo_Contain", ammoContain - 12)
    else
        warn("Not enough ammo to reload!")
    end
else
    warn("Your gun is already fully loaded!")
end

This is the fixed script.