Help making this script more efficient (sorry if wrong flair, will correct if needed)

[Asteralx (Taushima)]

You can write your topic however you want, but you need to answer these questions:

1. What do you want to achieve? Keep it simple and clear!
   making this script a little more efficient
2. What is the issue? Include screenshots / videos if possible!
   well I want more efficiency + not have errors for some of the if statements (like if the model doesn’t have a primary part)
3. What solutions have you tried so far? Did you look for solutions on the Developer Hub?
   I’m sorta thinking I should add debounce for the whole area (debounce when activated) because the animation will keep playing even if DB
   After that, you should include more details if you have any. Try to make your topic as descriptive as possible, so that it’s easier for people to help you!

--variables 
	local player = tool.Parent 
	local range = game.ServerStorage.Random.GBhitr
	local slice = hum:LoadAnimation(amm2)
	--running 
	slice:Play()
	local cpy = range:Clone() -- range is the hit range of the gear, this is about where it should be 
	
	local offset = Vector3.new(0, 0, -5)
	cpy.CFrame = player.PrimaryPart.CFrame*CFrame.new(offset)
	cpy.Parent = player
	-- weld range and player
	local weld = Instance.new("WeldConstraint", cpy)
	
	weld.Part0 = cpy
	weld.Part1 = player.PrimaryPart
	
	local takedam = true 

		
	cpy.Touched:Connect(function(hit) -- if detects if anything is in the hit range
			if hit.Parent:IsA("Model") then -- if model
				if hit.Parent.PrimaryPart.Name == ("HumanoidRootPart") then -- has a root
					if takedam == true then -- sorta a debounce i think?
						takedam = false
						local target = hit.Parent:FindFirstChild("Humanoid")
						target.Health = target.Health / 2 
						print(target.Name)
						wait(5) 
						takedam = true
						canhit = true
					elseif takedam == false then return end 
				elseif not hit.Parent.PrimaryPart.Name == ("HumanoidRootPart") then return end
			elseif not hit.Parent:IsA("Model") then return end

edit: forgot to mention the script is an inside tool.Activated
Please do not ask people to write entire scripts or design entire systems for you. If you can’t answer the three questions above, you should probably pick a different category.

[Eestlane771]

Let’s get right into it.

if hit.Parent.PrimaryPart.Name == ("HumanoidRootPart") then -- has a root
elseif not hit.Parent.PrimaryPart.Name == ("HumanoidRootPart") then return end

Constructs like this aren’t really necessary.
The entire second line can just be replaced with end.
This is because the elseif will only run when Name ~= "HumanoidRootPart" anyway, it is completely redundant.

Furthermore, you can turn that pyramid of ifs into a row of sentinels.
This isn’t really an optimization, it’s just common code style.

cpy.Touched:Connect(function(hit) -- if detects if anything is in the hit range
	if not takedam then return end -- sorta a debounce i think?
	if not hit.Parent:IsA("Model") then return end -- if model
	if hit.Parent.PrimaryPart.Name ~= "HumanoidRootPart" then return end -- has a root
	
	takedam = false
	local target = hit.Parent:FindFirstChild("Humanoid")
	target.Health = target.Health / 2 
	print(target.Name)
	wait(5)
	takedam = true
	canhit = true
end)

Here, I turned the if/elses into sentinels, and moved the debounce to the start.
It’s just a variable in the same script, so it’s really quick to check.

You can do a lot better with error checking, too.

	local target = hit.Parent:FindFirstChild("Humanoid")
	target.Health = target.Health / 2 

You use FindFirstChild to get the Humanoid, and then use it with the assumption that it isn’t nil.
This will error if there is no Humanoid, defeating the point of the FindFirstChild.

cpy.Touched:Connect(function(hit) -- if detects if anything is in the hit range
	if not takedam then return end -- sorta a debounce i think?
	-- if hit.Parent == nil then return end -- I'm not sure if this is needed anymore, but a lot of old sword scripts had this to suppress errors from e.g. paintballs hitting swords
	
	local target = hit.Parent:FindFirstChild("Humanoid")
	if target == nil then return end -- didn't hit a living thing
	
	takedam = false
	target.Health = target.Health / 2
	print(target.Name)
	wait(5)
	takedam = true
	canhit = true
end)

I removed the checks for Model and HumanoidRootPart because they aren’t as important as the Humanoid check.
If you uncomment the line that checks hit.Parent, then this function should never raise any errors ever again.

I’m also concerned that the “range part” is never removed, though I can only assume this because I can’t see the rest of your script.
If they don’t get removed, then eventually, after attacking a lot, there will be a LOT of attack parts, each trying to deal damage.

Also do something about that canhit variable. It seems to have no purpose.

The code above that Touched connection is fine IMO.

[Asteralx (Taushima)]

thanks for the help i never though you could turn ifs into such simple lines instead of return ending

[Asteralx (Taushima)]

if not canhit then return end  -- if canhit then cant hit and run script
	canhit = false 
	--variables 
	local player = tool.Parent 
	local range = game.ServerStorage.Random.GBhitr
	local slice = hum:LoadAnimation(amm2)
	slice:Play()
	local cpy = range:Clone() -- range is the hit range of the gear, this is about where it should be 
	local offset = Vector3.new(0, 0, -5)
	cpy.CFrame = player.PrimaryPart.CFrame*CFrame.new(offset)
	cpy.Parent = player
	
	-- weld range and player
	local weld = Instance.new("WeldConstraint", cpy)
	
	weld.Part0 = cpy
	weld.Part1 = player.PrimaryPart
	
	local takedam = true 

		
	cpy.Touched:Connect(function(hit) -- if detects if anything is in the hit range
		if not takedam then return end -- if take dam is true then make false
		takedam = false
		local target = hit.Parent:FindFirstChild("Humanoid")
		if target == nil then return end -- no humanoid 
		
		print("Hit takedam is allowed and it has a humanoid")
			target.Health = target.Health / 2
		wait()
		takedam = true
	end)
	
	wait(1)
	cpy:Destroy()
	wait(4)
	print("You can hit now")

so here’s the first draft edit I made. it works fine and doesn’t error but it hits twice (printing the “hit takedam…” twice) . I could just get over it and make it do less damage but idk.

[Eestlane771]

@Asteralx

it hits twice

That might be because the wait() in that Touched event is too short. The debounce is in effect for only 1/30 of a second, after which it can hit again. It totally can hit another body part in that amount of time. You can probably make it hit thrice if you do it exactly right.

In addition, that takedam = false should be moved two lines downward. If the sword hits a non-living thing, then the debounce is still activated, meaning it can’t hit any living thing it was supposed to hit to afterward.

If you do both of these, then it should deal damage only once and even if it bonks into a wall.
In fact, you can simply remove that wait() takedam = true if an attack should only hit once in its lifetime.
You can get maximum efficiency by stopping listening for touch events entirely:

local connection
connection = cpy.Touched:Connect(function(hit) -- if detects if anything is in the hit range
	local target = hit.Parent:FindFirstChild("Humanoid")
	if target == nil then return end -- no humanoid 
	
	print("Hit takedam is allowed and it has a humanoid")
	target.Health = target.Health / 2
	
	connection:Disconnect() -- don't run this event handler again. touching the effect will no longer do anything
end)

The drawback is that you don’t get to hit more than one enemy, which would be very disappointing if your weapon is a greatsword or something.