Am I sanity checking correctly?

Heya there.

Working on a 1:1 recreation of a game called “Noobs Vs Zombies: Relish Reborn” if I made it. Anyway, wondering if I am sanity checking correctly since whenever I tried changing the Ammo_Hold attribute, it kept saying "It's valid.".

Local

--//Tool
local Tool = script.Parent
local FireEvent = Tool:WaitForChild("FireEvent")
--Attributes
local AmmoHold = Tool:GetAttribute("Ammo_Hold")
local AmmoContain = Tool:GetAttribute("Ammo_Contain")
local Cooldown = Tool:GetAttribute("Cooldown")

--//Functionality
--Getting player's mouse
local Player = game:GetService("Players").LocalPlayer
local Mouse = Player:GetMouse()

--Firing the weapon.
local CD = false
Tool.Activated:Connect(function()
	if not CD then
		CD = true
		
		if AmmoHold < AmmoContain then
			Tool.FireEvent:FireServer(Mouse.Hit.Position)
			Tool:SetAttribute("Ammo_Hold", Tool:GetAttribute("Ammo_Hold") - 1)
		end
		
		task.wait(Cooldown)
		CD = false
	end
end)

Server

--//Tool
local Tool = script.Parent
local FireEvent = Tool:WaitForChild("FireEvent")
--Attributes
local AmmoHold = Tool:GetAttribute("Ammo_Hold")
local AmmoContain = Tool:GetAttribute("Ammo_Contain")
local Cooldown = Tool:GetAttribute("Cooldown")

--//Functionality
FireEvent.OnServerEvent:Connect(function(Player, MouseHit)
	--//Sanity Checking
	if (AmmoHold < AmmoContain) then
		print("It's valid.")
		Tool:SetAttribute("Ammo_Hold", Tool:GetAttribute("Ammo_Hold") - 1)
	else
		print("Not valid. Stop hacking, cheater.")
	end
end)

These are now the values held at those attributes at that point in time. They don’t update. You would want to call GetAttribute() within the callback.

Also attributes replicate from server to client so you don’t need to do the client sided Set().

1 Like

Also just to clarify,

print(“Not valid. Stop hacking, cheater.”)

I wouldn’t assume cheating off the back of this specifically, just in-case you have plans on using it as an exploit detection method. There will be a certain amount of latency between ‘shooting’, firing the event, adjusting the attribute, then having it replicate to the client again. It’s very possible they click twice during that time (if the cooldown is short), which means your script will print ‘Stop hacking, cheater’ when they aren’t.

What do you mean exactly?

(Filler)

local AmmoHold = Tool:GetAttribute("Ammo_Hold")

print(AmmoHold) -- > 5

Tool:SetAttribute("Ammo_Hold", 4)

print(AmmoHold) -- > 5

print(Tool:GetAttribute("Ammo_Hold")) -- > 4

The variable AmmoHold is just the value that the attribute had when you called GetAttribute. Calling SetAttribute doesn’t update that value, you need to get the new value again.

Ah, gotcha. Anyway, so I’d assuming I’m sanity checking incorrectly?

Yeah you’d want to check the value in the attribute every time the remote fires.

So I’d assume something like this?

--//Tool
local Tool = script.Parent
local FireEvent = Tool:WaitForChild("FireEvent")
--Attributes
local AmmoHold = Tool:GetAttribute("Ammo_Hold")
local AmmoContain = Tool:GetAttribute("Ammo_Contain")
local Cooldown = Tool:GetAttribute("Cooldown")

--//Functionality
FireEvent.OnServerEvent:Connect(function(Player, MouseHit)
	--//Sanity Checking
	if (Tool:GetAttribute("Ammo_Hold") < Tool:GetAttribute("Ammo_Contain")) then
		print("It's valid.")
		Tool:SetAttribute("Ammo_Hold", Tool:GetAttribute("Ammo_Hold") - 1)
	elseif not (Tool:GetAttribute("Ammo_Hold") < Tool:GetAttribute("Ammo_Contain")) then
		print("Not valid. Stop hacking, cheater.")
	end
end)
1 Like

Yeah kinda like that, but no need to wrap things in parenthesis. You can also simplify it a little; The Ammo_Contain isn’t going to change (magazine size right?), so you can get that outside of the event. Also only need to call GetAttribute one time if you know the value isn’t going to change, like this

local ammoContain = Tool:GetAttribute("Ammo_Contain")

FireEvent.OnServerEvent:Connect(function(Player, MouseHit)
    local ammoHold = Tool:GetAttribute("Ammo_Hold")
	--//Sanity Checking
	if ammoHold <= ammoContain then
		print("It's valid.")
		Tool:SetAttribute("Ammo_Hold", ammoHold - 1)
	else -- No need to elseif unless you're chaining a bunch of conditions. Either they have ammo or they don't here. 
		print("Not valid. Stop hacking, cheater.")
	end
end)

Alright, thanks for the help!!

1 Like