Are there any improvements i could do to this raycasting code?

notsad2 · June 14, 2024, 11:23am

This script is connected to a remote event that gets called whenever the player fires a weapon (in this case it’s an automatic)

one thing that i know is missing is a shooting cooldown handled on the server

i didn’t feel the need to add something like this since the gun has only 30 bullets, and firing 30 bullets faster than 960RPM (once every 60ms) would just not be worth cheat-proofing (you can scold me if i’m wrong)

i’m gonna be working with scripts like these a lot, so i really wanna know if there are any improvements i could do:

local ShootEvent = game:GetService("ReplicatedStorage"):WaitForChild("GunEvents"):WaitForChild("MP9Folder").ShootEvent
local DamageIndicatorEvent = game:GetService("ReplicatedStorage"):WaitForChild("PlayerEvents").DamageGUI

ShootEvent.OnServerEvent:Connect(function(player, mouse)
	local MP9 = player.Character:FindFirstChildOfClass("Tool")
	local Bullets = MP9:WaitForChild("Bullets")
	
	-- // QUALITY OF LIFE

	coroutine.wrap(function()
		local ShootSound = MP9:WaitForChild("mp9-s_1"):WaitForChild("Receiver"):WaitForChild("Barrel"):WaitForChild("Suppressor"):WaitForChild("ShootSound")
		local MuzzleFlash = MP9:WaitForChild("FlashPart")
		
		ShootSound:Play()
		MuzzleFlash:WaitForChild("Light").Enabled = true
		MuzzleFlash:WaitForChild("FlashImage").Enabled = true
		
		task.wait(0.1)
		
		MuzzleFlash:WaitForChild("Light").Enabled = false
		MuzzleFlash:WaitForChild("FlashImage").Enabled = false
	end)()
	
	-- // RAYCASTING
	
	local MIN_BULLETS = 1
	local MAX_BULLETS = 30
	local MAX_DISTANCE = 200
	local BULLET_DAMAGE = 18
	local HEADSHOT_MULTIPLIER = 1.6
	local HEADSHOT_DAMAGE = 0
	
	if Bullets.Value > MAX_BULLETS then player:Kick("The underworld calls to you.") end
	if Bullets.Value < MIN_BULLETS then player:Kick("Your manipulation techniques are atrocious.") end
	
	local origin = MP9:WaitForChild("FlashPart")
	local raycastParams = RaycastParams.new()
	raycastParams.FilterDescendantsInstances = {player.Character}
	raycastParams.IgnoreWater = true
	raycastParams.FilterType = Enum.RaycastFilterType.Exclude
	
	local direction = (mouse.Direction.Unit).Unit
	local displacement = direction * MAX_DISTANCE
	
	local result = workspace:Raycast(
		origin.Position,
		displacement,
		raycastParams
	)
	
	local tracer = Instance.new("Part")
	tracer.Parent = workspace
	tracer.Anchored = true
	tracer.CFrame = CFrame.lookAt(origin.Position + (displacement / 2), origin.Position + displacement)
	tracer.Size = Vector3.new(0.1, 0.1, displacement.Magnitude)
	tracer.Transparency = 0.6
	tracer.CanCollide = false		
	tracer.Color = Color3.new(0.960784, 0.666667, 0.156863)
	game:GetService("Debris"):AddItem(tracer, 0.2)
	
	
	if result ~= nil then
		local enemy = result.Instance.Parent:FindFirstChild("Humanoid")
		
		if enemy == game:GetService("Players"):GetPlayerFromCharacter(result.Instance.Parent) then return end
		
		if result.Instance == enemy.Parent.Head then	
			HEADSHOT_DAMAGE = math.floor(BULLET_DAMAGE * HEADSHOT_MULTIPLIER)
			enemy:TakeDamage(HEADSHOT_DAMAGE)
			DamageIndicatorEvent:FireClient(player, HEADSHOT_DAMAGE, enemy)
			
		else
			enemy:TakeDamage(BULLET_DAMAGE)
			DamageIndicatorEvent:FireClient(player, BULLET_DAMAGE, enemy)
		end
	end
end)

Eestlane771 · June 14, 2024, 1:17pm

30 bullets at more than 960RPM sounds fine, but what about infinity RPM? An exploiter can just fire the remote 30 times in one tick at one target to instakill it with a stacked superbullet.

This script seems to be huddled away in ServerScriptService and deals with all MP9s in the game. Because it doesn’t know anything about each individual MP9, it spams WaitForChild for every single thing when it should just have all of them prepared in a variable somewhere.
You NEED to stop doing this if you aren’t able to get it right and just have one serverscript and remote event for each individual gun tool. The script should store its firing sound, muzzle flash, Debris service etc. instead of finding everything all over again each time it shoots. This will also allow reusing the RaycastParams between shots, it only needs to set its filter once on equip.
The main downside to this approach is that if the weapon is deleted while it’s in the middle of doing something, it will never complete. The Debris use here is good because the tracer will be removed after 0.2 seconds even if the player leaves right after firing one shot.

The script boldly assumes a LOT of things. It assumes that the player is wielding a MP9. If the other gun scripts are anything like this, then an exploiter can just spam every single weapon remote at once to fire them from this one tool.
It also assumes that the player currently has a Character, but lacking one is rare and the only consequence is an error in the console.

There’s no need for MIN_BULLETS in my opinion, I assume there’s no gun in your game that cannot fire the last 3 or so cartridges(?) in the magazine, so that kind of “customizability” can be removed and the Bullets.Value < MIN_BULLETS be replaced with Bullets.Value <= 0.

(mouse.Direction.Unit).Unit seems wrong to me.
First, there is no check for what mouse actually is, so an exploiter can send {Direction = {Unit = {Unit = Vector3.new(25, 0, 0)}}} to fire along the X axis at 5000 range instead of 200. He can empty all magazines from all weapons into some helpless sod on the other corner of the map with laser precision, turning him into a steaming crater in the blink of an eye.
Second, you can’t send a Mouse instance over a remote event. Well, you can, but the server doesn’t have that instance, so it’ll just pretend it received nil instead.
Third, please reconsider sending a direction instead of the target position over the remote. The client might send an accurate direction, but the server won’t have an accurate character position due to physics interpolation and so the shots won’t hit exactly the point the player clicked unless he has been standing still for a quarter second or so.
Fourth, you don’t need to get Unit more than once.

The tracer is always the length of displacement, so if the raycast actually hits something like a wall instead of terminating in the air, the tracer will poke through it.

HEADSHOT_DAMAGE can just be calculated ahead of time, you don’t need to calculate HEADSHOT_DAMAGE every time a headshot is landed.

if enemy == game:GetService("Players"):GetPlayerFromCharacter(result.Instance.Parent) then return end

enemy is a Humanoid, it cannot be a Player. Consider adding a character variable that’s just result.Instance.Parent and calling the humanoid a humanoid instead of an enemy, you might’ve confused yourself with that variable name.

notsad2 · June 14, 2024, 1:57pm

How can i store the key “ingredients” of a gun inside a script and then have it reused?
how can i use one server script and remote event for every gun in the game? doesn’t this require severe fiddling with code? (which is something i’m hilariously bad at)

how could i check if the player is wielding an MP9?
is there a way to exploit-proof firing multiple events at once? (excluding one server script and one remote event (said above) from this question)
could you elaborate on “assumes that the player currently has a character”?

how exactly does that work, if it pretends it receives nil, how does the rest of the code run without issues?
i’m not really sure how physics interpolation works
i assume you mean local direction = (mouse.Direction.Unit + offset).Unit?

is there a way to get the length of a raycast and then apply it to the size of the tracer? doing result.Distance doesn’t seem to do the trick

Eestlane771 · June 14, 2024, 7:41pm

I was referring to putting a script and a remote event inside each gun the way tools are usually done. You’d also have to make the localscript fire the event inside the gun instead of that MP9 folder.

There’s mentions of three different ways of going about this:

One script and remote for each gun type, which is what you have right now. If there are 4 gun types and all 6 players have all guns, then there are 4 scripts.
One script and remote for each gun tool. That would be 4*6 scripts and remotes. They would each need to ignore remote events from the wrong player and when they’re not equipped.
One script and remote for all guns. Not what I suggested

How can i store the key “ingredients” of a gun inside a script and then have it reused?

If you have one script inside each tool, then you can just move most of the local variables that get the tool’s parts (which never change) out of the ShootEvent handler. If there isn’t a single WaitForChild in there, then that’s good enough.

That’s also possible with one script per gun type or one script per game, but it’s some trouble to clean up and avoid memory leaks that way.

could you elaborate on “assumes that the player currently has a character”?

This is the first line of the ShootEvent handler:
local MP9 = player.Character:FindFirstChildOfClass("Tool")
If player.Character is nil, then it will error. It blindly assumes that the player has a character. This is fine because it doesn’t actually enable any exploits in this case.

how could i check if the player is wielding an MP9?

In your original code, you’d just have to move that MP9:WaitForChild("mp9-s_1") out of the muzzle flash code, make it a FindFirstChild so that you can exit right away if it’s not a MP9 (otherwise exploiters can just spam the remotes, leave a million threads waiting for something that will never show up and crash the server), then just do nothing if it’s not found because that instance is (probably) only in the MP9 tool. This will make the MP9 script only fire if you’re wielding a MP9, every script will need to do that to make sure you can’t shoot one gun while actually holding another.

is there a way to exploit-proof firing multiple events at once? (excluding one server script and one remote event (said above) from this question)

Preventing individual gun firerate exploits and firing a gun you’re not holding should be enough. If you’re talking about generally ‘exploit-proofing’ multiple events, any rate limit mechanism/debounce will work as long as they all share one per player.

how exactly does that work, if it pretends it receives nil, how does the rest of the code run without issues?

Your code looks like it gets an actual Mouse from a client and punches tracers through walls, so I assumed that you’ve never run or tested the code even once, sorry about that. If that actually works, then it’s some big mystery.

i’m not really sure how physics interpolation works

You can’t be sure because you’ve probably never been tripped up by it before.

i assume you mean local direction = (mouse.Direction.Unit + offset).Unit?

No, that’s not what was in the original post.
This doesn’t make any sense either, if you fired in this direction (and if offset were the muzzle position and not the muzzle part) then you would always fire roughly away from 0,0,0.

is there a way to get the length of a raycast and then apply it to the size of the tracer? doing result.Distance doesn’t seem to do the trick

There’s a result.Distance or result.Length property, don’t remember which it was called. If there’s a result, then make the tracer this length, otherwise make the tracer the length of displacement.