Any recommendations on how to clean code up? How would I install a Maid class?

Hello there,

I am currently unsatisfied with my code. I merged my script with FastCast, and created a bunch of spaghetti code which is very hard for my eyes to handle. On the other hand, it does get the job done, but is very hard to understand at times.

  1. Improvements, I want to upgrade the Organization and purpose of using functions. The script shines runs off of just ONE ModuleScript and can get VERY HARD TO UNDERSTAND.
  2. Maids and Classes, I would like to make everything depend off of classes using ModuleScripts, andeasier to configure.
  3. It is not the best Filtering Enabled script, it needs to be improved greatly and is barely FE.
My Code
local tool = script.Parent
local aiming = false
local meleeing = false
local tagged = nil
local cooldown = false
local reloading = false
local debounce = false
local Player = game.Players.LocalPlayer

local followMouse = false

local recoil = 0

local firing = false

local Handle = tool.Handle

-- FastCast Variables
local FastCast = require(tool:WaitForChild("FastCastRedux"))
local FirePointObject = tool.Emitter:WaitForChild("GunFirePoint")
local ImpactParticle = tool.Emitter:WaitForChild("ImpactParticle")
local Debris = game:GetService("Debris")
local Caster = FastCast.new()

-- Bullet Setup
local CosmeticBullet = Instance.new("Part")
CosmeticBullet.Material = Enum.Material.Neon
CosmeticBullet.Color = Color3.fromRGB(254, 243, 187)
CosmeticBullet.CanCollide = false
CosmeticBullet.Anchored = true
CosmeticBullet.Size = Vector3.new(0.2, 0.2, 2.4)


-- Configuration
local animPath = game.Lighting.Animations.Rifle

local rps = 0.0666666667000
local reloadTime = tool.Handle.reload.TimeLength

local headshotDamage = 25
local defaultDamage = 17

-- Fast Cast Configuration
local RNG = Random.new()							-- Set up a randomizer.
local DEBUG_VISUALIZE = false						-- If true, individual sub-rays will be shown with black cones.
local BULLET_SPEED = 500							-- Studs/second - the speed of the bullet
local BULLET_MAXDIST = 1000							-- The furthest distance the bullet can travel 
local BULLET_GRAVITY = Vector3.new(0, 0, 0)		-- The amount of gravity applied to the bullet in world space (so yes, you can have sideways gravity)
local MIN_BULLET_SPREAD_ANGLE = 0					-- THIS VALUE IS VERY SENSITIVE. Try to keep changes to it small. The least accurate the bullet can be. This angle value is in degrees. A value of 0 means straight forward. Generally you want to keep this at 0 so there's at least some chance of a 100% accurate shot.
local MAX_BULLET_SPREAD_ANGLE = 0.5					-- THIS VALUE IS VERY SENSITIVE. Try to keep changes to it small. The most accurate the bullet can be. This angle value is in degrees. A value of 0 means straight forward. This cannot be less than the value above. A value of 90 will allow the gun to shoot sideways at most, and a value of 180 will allow the gun to shoot backwards at most. Exceeding 180 will not add any more angular varience.
local TAU = math.pi * 2							-- Set up mathematical constant Tau (pi * 2)
local PIERCE_DEMO = true							-- True if the pierce demo should be used. See the CanRayPierce function for more info.

-- Functions
function reload()
	reloading = true
	tool.Handle.reload:Play()
	rld:Play(0.25)
	tool.Mag.Transparency = 1
	wait(reloadTime)
	tool.Mag.Transparency = 0
	reloading = false
	heldammo.Value = heldammo.Value - 1
	ammo.Value = ammo.MaxValue
end

function shoot(hitPart)
	while firing and ammo.Value > 0 and aiming do
		recoil = recoil + .5
		ammo.Value = ammo.Value - 1
		fire:Play()
		aim:Play(0.25)
		hold:Stop()
		tool.Emitter.Fire:play()

				
		tool.Emitter.particle.Enabled = true
		tool.Emitter.smoke.Enabled = true
		tool.Emitter.Light.Enabled = true

		local part = hitPart
		if part then
			local humanoid = part.Parent:FindFirstChild("Humanoid")
	 
			if not humanoid and part.Parent.ClassName ~= "Tool" then
				humanoid = part.Parent.Parent:FindFirstChild("Humanoid")
			end
					
			if humanoid then
				if humanoid then
				if part.Name == "Head" then
					workspace.Remotes.SetHealth:FireServer("Damage",headshotDamage,humanoid)
				elseif part.Name == "Torso" then
					workspace.Remotes.SetHealth:FireServer("Damage",defaultDamage,humanoid)
				elseif part.Name == "Handle" and part.Parent.ClassName == "Hat" then
					workspace.Remotes.SetHealth:FireServer("Damage",defaultDamage,humanoid)
				else
					workspace.Remotes.SetHealth:FireServer("Damage",defaultDamage,humanoid)
					end
				end
				elseif Player then
				if part.Name == "Head" then
					workspace.Remotes.SetHealth:FireServer("Damage",0,humanoid)
				elseif part.Name == "Torso" then
					workspace.Remotes.SetHealth:FireServer("Damage",0,humanoid)
				elseif part.Name == "Handle" and part.Parent.ClassName == "Hat" then
					workspace.Remotes.SetHealth:FireServer("Damage",0,humanoid)
				else
					workspace.Remotes.SetHealth:FireServer("Damage",0,humanoid)
					end
				end
		end

		wait(rps)
		tool.Emitter.Light.Enabled = false
		tool.Emitter.particle.Enabled = false
		tool.Emitter.smoke.Enabled = false
		end
	end

function unEquip()
	tool.CLIENT_SCRIPT.Disabled = true
	aim:Stop()
	hold:Stop()
	melee:Stop()
	fire:Stop()
	rld:Stop()
	aiming = false
end

-- FastCast Functions
function VisualizeSegment(castStartCFrame, castLength)
	local adornment = Instance.new("ConeHandleAdornment")
	adornment.Adornee = workspace.Terrain
	adornment.CFrame = castStartCFrame
	adornment.Height = castLength
	adornment.Color3 = Color3.new()
	adornment.Radius = 0.25
	adornment.Transparency = 0.5
	adornment.Parent = workspace.Terrain
end

function MakeParticleFX(position, normal)
	-- This is a trick I do with attachments all the time.
	-- Parent attachments to the Terrain - It counts as a part, and setting position/rotation/etc. of it will be in world space.
	-- UPD 11 JUNE 2019 - Attachments now have a "WorldPosition" value, but despite this, I still see it fit to parent attachments to terrain since its position never changes.
	local attachment = Instance.new("Attachment")
	attachment.CFrame = CFrame.new(position, position + normal)
	attachment.Parent = workspace.Terrain
	local particle = ImpactParticle:Clone()
	particle.Parent = attachment
	Debris:AddItem(attachment, particle.Lifetime.Max) -- Automatically delete the particle effect after its maximum lifetime.
	
	-- A potentially better option in favor of this would be to use the Emit method (Particle:Emit(numParticles)) though I prefer this since it adds some natural spacing between the particles.
	particle.Enabled = true
	wait(0.05)
	particle.Enabled = false
end

function CanRayPierce(hitPart, hitPoint, normal, material)
	if material == Enum.Material.Plastic or material == Enum.Material.Ice or material == Enum.Material.Glass or material == Enum.Material.SmoothPlastic then
		if hitPart.Transparency >= 0.5 then
			return true
		end
	end
	return false
end

function Fire(direction)
	-- Called when we want to fire the gun.
	if tool.Parent:IsA("Backpack") then return end -- Can't fire if it's not equipped.
	-- Note: Above isn't in the event as it will prevent the CanFire value from being set as needed.
	
	-- UPD. 11 JUNE 2019 - Add support for random angles.
	local directionalCF = CFrame.new(Vector3.new(), direction)
	-- Now, we can use CFrame orientation to our advantage.
	-- Overwrite the existing Direction value.
	local direction = (directionalCF * CFrame.fromOrientation(0, 0, RNG:NextNumber(0, TAU)) * CFrame.fromOrientation(math.rad(RNG:NextNumber(MIN_BULLET_SPREAD_ANGLE, MAX_BULLET_SPREAD_ANGLE)), 0, 0)).LookVector
	
	-- UPDATE V6: Proper bullet velocity!
	-- IF YOU DON'T WANT YOUR BULLETS MOVING WITH YOUR CHARACTER, REMOVE THE THREE LINES OF CODE BELOW THIS COMMENT.
	-- Requested by https://www.roblox.com/users/898618/profile/
	-- We need to make sure the bullet inherits the velocity of the gun as it fires, just like in real life.
	local humanoidRootPart = tool.Parent:WaitForChild("HumanoidRootPart", 1)	-- Add a timeout to this.
	local myMovementSpeed = humanoidRootPart.Velocity							-- To do: It may be better to get this value on the clientside since the server will see this value differently due to ping and such.
	local modifiedBulletSpeed = (direction * BULLET_SPEED) + myMovementSpeed	-- We multiply our direction unit by the bullet speed. This creates a Vector3 version of the bullet's velocity at the given speed. We then add MyMovementSpeed to add our body's motion to the velocity.
	
	-- Prepare a new cosmetic bullet
	local bullet = CosmeticBullet:Clone()
	bullet.CFrame = CFrame.new(FirePointObject.WorldPosition, FirePointObject.WorldPosition + direction)
	bullet.Parent = workspace
	
	-- NOTE: It may be a good idea to make a Folder in your workspace named "CosmeticBullets" (or something of that nature) and use FireWithBlacklist on the descendants of this folder!
	-- Quickly firing bullets in rapid succession can cause the caster to hit other casts' bullets from the same gun (The caster only ignores the bullet of that specific shot, not other bullets).
	-- Do note that if you do this, you will need to remove the Equipped connection that sets IgnoreDescendantsInstance, as this property is not used with FireWithBlacklist
	
	-- Fire the caster
	if PIERCE_DEMO then
		Caster:Fire(FirePointObject.WorldPosition, direction * BULLET_MAXDIST, modifiedBulletSpeed, bullet, tool.Parent, false, BULLET_GRAVITY, CanRayPierce)
	else
		Caster:Fire(FirePointObject.WorldPosition, direction * BULLET_MAXDIST, modifiedBulletSpeed, bullet, tool.Parent, false, BULLET_GRAVITY)
	end
end

function OnRayHit(hitPart, hitPoint, normal, material, cosmeticBulletObject)
	-- This function will be connected to the Caster's "RayHit" event.
	cosmeticBulletObject:Destroy() -- Destroy the cosmetic bullet.
	if hitPart and hitPart.Parent then -- Test if we hit something
		local humanoid = hitPart.Parent:FindFirstChildOfClass("Humanoid") -- Is there a humanoid?
		if humanoid then
			--humanoid:TakeDamage(10) -- Damage.
		end
		MakeParticleFX(hitPoint, normal) -- Particle FX
	end
end

function OnRayUpdated(castOrigin, segmentOrigin, segmentDirection, length, cosmeticBulletObject)
	-- Whenever the caster steps forward by one unit, this function is called.
	-- The bullet argument is the same object passed into the fire function.
	local bulletLength = cosmeticBulletObject.Size.Z / 2 -- This is used to move the bullet to the right spot based on a CFrame offset
	local baseCFrame = CFrame.new(segmentOrigin, segmentOrigin + segmentDirection)
	cosmeticBulletObject.CFrame = baseCFrame * CFrame.new(0, 0, -(length - bulletLength))
	
	if DEBUG_VISUALIZE then VisualizeSegment(baseCFrame, length) end
end

-- Misc
assert(MAX_BULLET_SPREAD_ANGLE >= MIN_BULLET_SPREAD_ANGLE, "Error: MAX_BULLET_SPREAD_ANGLE cannot be less than MIN_BULLET_SPREAD_ANGLE!")
if (MAX_BULLET_SPREAD_ANGLE > 180) then
	warn("WARNING - FASTCAST: MAX_BULLET_SPREAD_ANGLE is over 180! This will not pose any extra angular randomization. The value has been changed to 180 as a result of this.")
	MAX_BULLET_SPREAD_ANGLE = 180
end
	
-- Main Script
tool.Equipped:connect(function(mouse)
	character = tool.Parent
	local hum = character.Humanoid
	local Player = game:GetService("Players").LocalPlayer
	
	local rlMouse = Player:GetMouse()
	
	ammo = script.Parent.Ammo
	heldammo = script.Parent.Stock
	
	mouse.Icon = "http://www.roblox.com/asset/?id=106491038"
	
	hold = hum:LoadAnimation(animPath.idle)
	aim = hum:LoadAnimation(animPath.aim)
	fire = hum:LoadAnimation(animPath.fire)
	melee = hum:LoadAnimation(animPath.melee)
	rld = hum:LoadAnimation(animPath.reload) 
	
	hold:Play(0.25)
	debounce = true
	
	tool.Emitter.Touched:connect(function(hit)
		if meleeing and hit and hit.Parent and hit.Parent ~= tagged and hit.Anchored == false then
			tagged = hit.Parent
			hit.Velocity = character.Torso.CFrame.lookVector * 60
			wait(.5)
			tagged = nil
		end
	end)
	
	mouse.KeyDown:connect(function(key)
		if key:lower() == "r" and ammo.Value < ammo.MaxValue and not reloading and heldammo.Value > 0 then
			reload()
elseif key:lower() == "e" and not meleeing and not reloading then
			if aiming == false then
				-- Enabled
				aim:Play(0.25)
				tool.CLIENT_SCRIPT.Disabled = false
				tool.CLIENT_TORSOSCRIPT.Return:Fire(true)
				hold:Stop()
				print('Animation success.')
				tool.GripForward = Vector3.new(-0.1, 0, -0.9)
				tool.GripRight = Vector3.new(0.9, 0, -0.1)
				aiming = true
			elseif aiming == true then
				-- Disabled
				aim:Stop()
				tool.CLIENT_SCRIPT.Disabled = true
				tool.CLIENT_TORSOSCRIPT.Return:Fire(false)
				hold:Play(0.25)
				print('Animation success.')
				tool.GripForward = Vector3.new(0, 0, -1)
				tool.GripRight = Vector3.new(1, 0, 0)
				aiming = false
			end
		end
	end)

	mouse.Button1Down:connect(function(clientThatFired, mouseDirection)
		if aiming and not meleeing and not cooldown and ammo.Value > 0 and not reloading then
			aim:Play(0.25)
			hold:Stop()
			tool.CLIENT_SCRIPT.Disabled = false
			firing = true
			cooldown = true
			Fire(mouseDirection)
			shoot()
			wait(.01)
			cooldown = false
			fire:Stop()
		elseif not meleeing and not aiming and not reloading and not firing then
		end
	end)
	
Caster.LengthChanged:Connect(OnRayUpdated)
Caster.RayHit:Connect(OnRayHit)
	
	mouse.Button1Up:connect(function()
		firing = false
		tool.Emitter.smoke.Enabled = false
		recoil = 0
	end)
	
	hum.Died:connect(function()
		if tool.Parent == character then
			local gunModel = Instance.new("Model", game.Workspace)
			gunModel.Name = tool.Name
			local faketool = tool:clone()
			faketool.Parent = gunModel
			local parts = faketool:GetChildren()
			for i = 1,#parts do
				if parts[i].ClassName == "UnionOperation" then
					parts[i].Parent = gunModel
					parts[i].CanCollide = true
				elseif parts[i].ClassName == "Part" then
					parts[i].Parent = gunModel
					parts[i].CanCollide = true
				elseif parts[i].Name == "Stock" then
					parts[i].Parent = gunModel
				end
			end
			local ammoScript = game.Lighting.AmmoDrop:clone()
			ammoScript.Parent = gunModel
			faketool:remove()
			gunModel:MoveTo(tool.Handle.Position)
			tool:remove()
		end
	end)
end)

tool.Unequipped:connect(function()
	unEquip()
end)

Thanks,
Rook.

You just need to have a way to organize the script.

What I usually do is to get the services first, then I index anything that are in the services, then I get the players, then any other objects. Then I would write the actual code. So, it would look somewhat like this:

local RP = game:GetService("ReplicatedStorage")
local RF = game:GetService("ReplicatedFirst")
local LG = game:GetService("Lighting")

local Events = RP:WaitForChild("Events")
local Coin = RF:WaitFrChild("Coin")

local Player = game.Players
local LocalPlayer = Players.LocalPlayer
local Character = LocalPlayer.Character

if Character.Name = "TOP_Crundee123" then
-- rest of code

You just need t find out a way that works best fr you, this works best for me

3 Likes

i personally tend to avoid modules altogether if it isnt repeat code that is used by different scripts at a time; but you can sort of categorize your scripts (i.e. services first, events second, values third etc.)

1 Like

Your code is already pretty good, all I can suggest is to downsize your code.