Any way to clean up this script?

One M1 in my game currently is nearly 100 lines of code.

What are you aiming for?

I want to know if there’s any more efficient ways to check player status, AVOIDING ATTRIBUTES AT ALL COSTS or any better way to make hitboxes in general?

Why did you post this?

I want to maintain the integrity of the script, because it works perfectly fine. But the messiness of it just yells at me that theres a much better way to go about making it.

Have you tried any other solutions?

I have attempted to use remote functions to pass on the contact table do all the hitlist humanoid stuff, then sending it back to the client checking their current status and seeing if they are able to be hurt. Then of course, dealing the damage and applying stun.
This did NOT work :skull:


(Sorry about all the blabbering, just trying to provide as much Info as possible to make this simpler.)

CURRENT CODE

HitRemote.OnServerEvent:Connect(function(Player, Contact)
	task.wait(0.3)
	local hitlist = {}
	
	for i,v in Contact do
		local contactsound = Instance.new("Sound")
		contactsound.RollOffMode = Enum.RollOffMode.Linear
		contactsound.RollOffMaxDistance = 15
		contactsound.RollOffMinDistance = 0
		contactsound.Parent = v.Parent
			contactsound.SoundId = 'rbxassetid://5955549918'
		contactsound.PlaybackSpeed = 2
		contactsound.Volume = 0.35
		contactsound:Play()
		contactsound.Ended:Connect(function()
			contactsound:Destroy()
		end)
		local EChar = v.Parent
		local EHuman = EChar:FindFirstChildOfClass("Humanoid")
		local EPlayer = Players:GetPlayerFromCharacter(EChar)
		if EHuman then
		if not table.find(hitlist, EHuman) then
			table.insert(hitlist, EHuman)
			if EPlayer == nil then
				if IsBlocking[EChar] then
					local blockedsound = Instance.new("Sound")
						blockedsound.SoundId = 'rbxassetid://7029643469'
						blockedsound.Parent = EChar
						blockedsound.PlaybackSpeed = math.random(1,1.6)
						blockedsound:Play()
						blockedsound.Ended:Connect(function()
							blockedsound:Destroy()
						end)
							elseif IsParrying[EChar] then
							print("parried")
							local parrysound = Instance.new("Sound")
						parrysound.SoundId = 'rbxassetid://8132494511'
						parrysound.PlaybackSpeed = math.random(1,1.3)
						parrysound.Parent = EChar
						parrysound:Play()
						parrysound.Ended:Connect(function()
							parrysound:Destroy()
						end)
						StunRemote:FireClient(Player, 2.5)
				elseif IFrames[EChar] then 
					print("Cannot Connect...")
						elseif not IFrames[EChar] and not IsParrying[EChar] and not IsBlocking[EChar] then
							local stunanim = Instance.new("Animation")
						stunanim.AnimationId = 'rbxassetid://18341528103'
						local stuntrack = EHuman:FindFirstChildOfClass("Animator"):LoadAnimation(stunanim)
						stuntrack:Play()
						stuntrack.Ended:Connect(function()
							stunanim:Destroy()
						end)
						EHuman:TakeDamage(math.random(2,5))
				end
			elseif EPlayer ~= nil then
					if IsBlocking[EChar] then
						local blockedsound = Instance.new("Sound")
						blockedsound.SoundId = 'rbxassetid://7029643469'
						blockedsound.Parent = EChar
						blockedsound.PlaybackSpeed = math.random(1,1.6)
						blockedsound:Play()
						blockedsound.Ended:Connect(function()
							blockedsound:Destroy()
						end)
					elseif IsParrying[EChar] then
						local parrysound = Instance.new("Sound")
						parrysound.SoundId = 'rbxassetid://8132494511'
						parrysound.PlaybackSpeed = math.random(1,1.3)
						parrysound.Parent = EChar
						parrysound:Play()
						parrysound.Ended:Connect(function()
							parrysound:Destroy()
						end)
						StunRemote:FireClient(Player, 2.5)
					elseif IFrames[EChar] then
						print(EChar.Name, "Is in IFrames")
					elseif not IFrames[EChar] and not IsParrying[EChar] and not IsBlocking[EChar] then
						local stunanim = Instance.new("Animation")
						stunanim.AnimationId = 'rbxassetid://18341528103'
						local stuntrack = EHuman:FindFirstChildOfClass("Animator"):LoadAnimation(stunanim)
						stuntrack:Play()
						stuntrack.Ended:Connect(function()
							stunanim:Destroy()
						end)
						EHuman:TakeDamage(math.random(2,5))
						StunRemote:FireClient(EPlayer, 1)
					end
				end
			end
		end
	end
end)

This is one singular M1

Maybe move a lot of tedious reusable logic, into module functions, then require that in each M1 file, to have less clutter going on in the code.

I highly recommend splitting it up into functions like so:

local function playSound(parent, soundId, playbackSpeed, volume, maxDistance)
	local sound = Instance.new("Sound")
	sound.RollOffMode = Enum.RollOffMode.Linear
	sound.RollOffMaxDistance = maxDistance or 15
	sound.RollOffMinDistance = 0
	sound.Parent = parent
	sound.SoundId = soundId
	sound.PlaybackSpeed = playbackSpeed
	sound.Volume = volume or 0.35
	sound:Play()
	sound.Ended:Connect(function()
		sound:Destroy()
	end)
end

local function playAnimation(humanoid, animationId)
	local animator = humanoid:FindFirstChildOfClass("Animator")
	if animator then
		local animation = Instance.new("Animation")
		animation.AnimationId = animationId
		local animationTrack = animator:LoadAnimation(animation)
		animationTrack:Play()
		animationTrack.Ended:Connect(function()
			animation:Destroy()
		end)
	end
end

local function handleHit(player, eChar, eHuman, isPlayer)
	if IsBlocking[eChar] then
		playSound(eChar, 'rbxassetid://7029643469', math.random(1, 1.6))
	elseif IsParrying[eChar] then
		playSound(eChar, 'rbxassetid://8132494511', math.random(1, 1.3))
		StunRemote:FireClient(player, 2.5)
	elseif IFrames[eChar] then
		print("Cannot Connect...")
	else
		playAnimation(eHuman, 'rbxassetid://18341528103')
		eHuman:TakeDamage(math.random(2, 5))
		if isPlayer then
			StunRemote:FireClient(Players:GetPlayerFromCharacter(eChar), 1)
		end
	end
end

HitRemote.OnServerEvent:Connect(function(player, contact)
	task.wait(0.3)
	local hitlist = {}

	for _, v in pairs(contact) do
		local eChar = v.Parent
		local eHuman = eChar:FindFirstChildOfClass("Humanoid")
		local ePlayer = Players:GetPlayerFromCharacter(eChar)

		if eHuman and not table.find(hitlist, eHuman) then
			table.insert(hitlist, eHuman)
			playSound(v.Parent, 'rbxassetid://5955549918', 2, 0.35)

			handleHit(player, eChar, eHuman, ePlayer ~= nil)
		end
	end
end)

Yeah I forgot Modules existed for one of the sole reasons of using tedious functions, thank you both for reminding me that functions exist :skull:, also. If you do, do stuff with hitboxes. How do you go about handling them?

Just do not even bother, I highly recommend using an opensource hitbox module, you can find them in #resources, and running it on the client and sending over the targets to the server.