How can I reduce the amount of lines in this code?

Help and Feedback Code Review
, ,
#1

Currently, this module has a lot of lines of code just to create a fracture effect for a part. My goal here is to down the amount of lines, since there’s a lot of code that is just repeated. I just don’t know to simplify this.

Some help would be much appreciated as always!

local Module = {}

DebugEnabled = true

Debris = game:GetService("Debris")

function Module.FracturePart(PartToFracture)
	
	local BreakingPointAttachment = PartToFracture:FindFirstChild("BreakingPoint")
	
	-- Settings
	local Configuration = PartToFracture:FindFirstChild("Configuration")
	local DebrisDespawn = false
	local DebrisDespawnDelay = 0
	local AnchorDebri = false
	
	if DebugEnabled then
		local DebugPart = Instance.new("Part")
		DebugPart.Shape = "Ball"
		DebugPart.CanCollide = false
		DebugPart.Anchored = true
		DebugPart.Size = Vector3.new(0.5, 0.5, 0.5)
		DebugPart.Color = Color3.fromRGB(255, 0, 0)
		DebugPart.Position = BreakingPointAttachment.WorldPosition
		DebugPart.Parent = workspace
	end
	
	
	if not BreakingPointAttachment then
		warn("The 'BreakingPoint' attachment is not a valid member of " .. PartToFracture.Name .. ". Please insert an attachment named 'BreakingPoint'")
	end
	
	if Configuration then
		DebrisDespawn = Configuration.DebrisDespawn.Value
		DebrisDespawnDelay = Configuration.DebrisDespawnDelay.Value
		AnchorDebri = Configuration.AnchorDebri.Value
	else
		warn("The 'Configuration' is not a valid member of " .. PartToFracture.Name .. ". Please insert a 'Configuration' with the following values; 'DebrisDespawn' (bool), 'AnchorDebri' (bool), 'DebrisDespawnDelay' (number/int)")
	end
	
	local BreakingPointY = BreakingPointAttachment.Position.Y
	local BreakingPointZ = BreakingPointAttachment.Position.Z
	
	local ShardBottomLeft = Instance.new("WedgePart")
	local ShardBottomRight = Instance.new("WedgePart")
	local ShardTopLeft = Instance.new("WedgePart")
	local ShardTopRight = Instance.new("WedgePart")
	
	local BreakSound = PartToFracture:FindFirstChild("BreakSound")
	
	-- Bottom Left
	ShardBottomLeft.Material = PartToFracture.Material
	ShardBottomLeft.Color = PartToFracture.Color
	ShardBottomLeft.Transparency = PartToFracture.Transparency
	ShardBottomLeft.Size = PartToFracture.Size - Vector3.new(0, (PartToFracture.Size.Y / 2) - BreakingPointY, (PartToFracture.Size.Z / 2) + BreakingPointZ)
	local OldSizeY = ShardBottomLeft.Size.Y
	local OldSizeZ = ShardBottomLeft.Size.Z
	ShardBottomLeft.CFrame = PartToFracture.CFrame * CFrame.new(0, BreakingPointY - (ShardBottomLeft.Size.Y / 2), BreakingPointZ + (ShardBottomLeft.Size.Z / 2))
	ShardBottomLeft.CFrame = ShardBottomLeft.CFrame * CFrame.Angles(math.rad(90), 0, 0)
	ShardBottomLeft.Size = Vector3.new(ShardBottomLeft.Size.X, OldSizeZ, OldSizeY)
	local ShardBottomLeft2 = ShardBottomLeft:Clone()
	ShardBottomLeft2.CFrame = ShardBottomLeft2.CFrame * CFrame.Angles(math.rad(180), 0, 0)
	
	-- Bottom Right
	ShardBottomRight.Material = PartToFracture.Material
	ShardBottomRight.Color = PartToFracture.Color
	ShardBottomRight.Transparency = PartToFracture.Transparency
	ShardBottomRight.Size = PartToFracture.Size - Vector3.new(0, (PartToFracture.Size.Y / 2) + BreakingPointY, (PartToFracture.Size.Z / 2) + BreakingPointZ)
	ShardBottomRight.CFrame = PartToFracture.CFrame * CFrame.new(0, BreakingPointY + (ShardBottomRight.Size.Y / 2), BreakingPointZ + (ShardBottomRight.Size.Z / 2))
	local ShardBottomRight2 = ShardBottomRight:Clone()
	ShardBottomRight2.CFrame = ShardBottomRight2.CFrame * CFrame.Angles(math.rad(180), 0, 0)
	
	-- Top Left
	ShardTopLeft.Material = PartToFracture.Material
	ShardTopLeft.Color = PartToFracture.Color
	ShardTopLeft.Transparency = PartToFracture.Transparency
	ShardTopLeft.Size = PartToFracture.Size - Vector3.new(0, (PartToFracture.Size.Y / 2) + BreakingPointY, (PartToFracture.Size.Z / 2) - BreakingPointZ)
	local OldSizeY = ShardTopLeft.Size.Y
	local OldSizeZ = ShardTopLeft.Size.Z
	ShardTopLeft.CFrame = PartToFracture.CFrame * CFrame.new(0, BreakingPointY + (ShardTopLeft.Size.Y / 2), BreakingPointZ - (ShardTopLeft.Size.Z / 2))
	ShardTopLeft.CFrame = ShardTopLeft.CFrame * CFrame.Angles(math.rad(90), 0, 0)
	ShardTopLeft.Size = Vector3.new(ShardTopLeft.Size.X, OldSizeZ, OldSizeY)
	local ShardTopLeft2 = ShardTopLeft:Clone()
	ShardTopLeft2.CFrame = ShardTopLeft2.CFrame * CFrame.Angles(math.rad(180), 0, 0)
	
	-- Top Right
	ShardTopRight.Material = PartToFracture.Material
	ShardTopRight.Color = PartToFracture.Color
	ShardTopRight.Transparency = PartToFracture.Transparency
	ShardTopRight.Size = PartToFracture.Size - Vector3.new(0, (PartToFracture.Size.Y / 2) - BreakingPointY, (PartToFracture.Size.Z / 2) - BreakingPointZ)
	ShardTopRight.CFrame = PartToFracture.CFrame * CFrame.new(0, BreakingPointY - (ShardTopRight.Size.Y / 2), BreakingPointZ - (ShardTopRight.Size.Z / 2))
	local ShardTopRight2 = ShardTopRight:Clone()
	ShardTopRight2.CFrame = ShardTopRight2.CFrame * CFrame.Angles(math.rad(180), 0, 0)
	
	local ShardDictionary = {ShardBottomLeft, ShardBottomLeft2, ShardBottomRight, ShardBottomRight2, ShardTopLeft, ShardTopLeft2, ShardTopRight, ShardTopRight2}
	
	for i, Shard in ipairs(ShardDictionary) do
		Shard.Velocity = PartToFracture.Velocity
		Shard.RotVelocity = PartToFracture.RotVelocity
		Shard.Anchored = AnchorDebri
		Shard.Name = "Shard"
		Shard.Parent = PartToFracture.Parent
		if DebrisDespawn == true then
			Debris:AddItem(Shard, DebrisDespawnDelay)
		end
	end
	
	PartToFracture:Destroy()
	
end

return Module

-- System by Ethanthegrand14
1 Like
#2

Why would you want to remove the lines? Wouldn’t you want the script to function the best it could?

#3

Your not getting the point. This code can be easily simplified. It can still work the same way, i just wan’t to down size it.

#4

Instead of creating new instances every time you can make prefabricated ones, with the desired properties, and then just clone them. Also, where its possible group the instances into a local array, for example in the section where you are adding them to the Debris service.

1 Like
#5

Did you just modify the code lol? For the section where you had Debris:AddItem() you changed it to a loop, that was one of my suggestions.

#6

yes i did, iv’e managed to do that before you sent anything actually

1 Like
#7

Well you can use my other suggestion of just make prefabricated instances, for example the DebugPart and shard parts.

1 Like
#8

I’ll consider that other suggestion too, thanks!

#9

Everything looks fine, I recommend your settings variables are left outside the function so resources aren’t spent to redefine it everytime it runs + localize the Debris variable and DebugEnabled variable.

Something like your question is meant to be in #help-and-feedback:code-review by the way.

1 Like
#10

First thing, you are basically returning nothing. You can at least return a function to make the module useful.

Also use Enums instead of strings.

for i, Shard in ipairs(ShardDictionary) do

Remove the “i” and place it with a “_” if you don’t need it and from your code, I guess you don’t need it so there is no use of a index here.

Here’s how I would dry up your whole script.

local Debris = game:GetService("Debris")

local Module = {}

local DebugEnabled = true

 function Module.FracturePart(PartToFracture)
	
	local BreakingPointAttachment = PartToFracture:FindFirstChild("BreakingPoint")
	
	-- Settings
	
	local Configuration = PartToFracture:FindFirstChild("Configuration")
	local DebrisDespawn = false
	local DebrisDespawnDelay = 0
	local AnchorDebris = false
	
	local function createPart(Name, Anchored, CanCollide, Shape, Size, Color, Position, Parent)
		local part = Instance.new("Part")
		part.Name = Name
		part.Shape = Shape 
		part.CanCollide = CanCollide
		part.Size = Size
		part.Color = Color
		part.Position = Position
		part.Parent = Parent
	end
	
	if DebugEnabled then
		createPart("Ball", true, false, Enum.PartType.Ball, Vector3.new(0.5, 0.5, 0.5), Color3.fromRGB(255, 0, 0), BreakingPointAttachment.WorldPosition, workspace)
	end	 
	
	if not BreakingPointAttachment then
		warn("The 'BreakingPoint' attachment is not a valid member of " .. PartToFracture.Name .. ". Please insert an attachment named 'BreakingPoint'")
	end
	
	if Configuration then
		DebrisDespawn = Configuration.DebrisDespawn.Value
		DebrisDespawnDelay = Configuration.DebrisDespawnDelay.Value
		AnchorDebris = Configuration.AnchorDebri.Value
	else
		warn("The 'Configuration' is not a valid member of " .. PartToFracture.Name .. ". Please insert a 'Configuration' with the following values; 'DebrisDespawn' (bool), 'AnchorDebri' (bool), 'DebrisDespawnDelay' (number/int)")
	end
	
	local BreakingPointY = BreakingPointAttachment.Position.Y
	local BreakingPointZ = BreakingPointAttachment.Position.Z
	
	local ShardBottomLeft = Instance.new("WedgePart")
	local ShardBottomRight = Instance.new("WedgePart")
	local ShardTopLeft = Instance.new("WedgePart")
	local ShardTopRight = Instance.new("WedgePart")
	
	local BreakSound = PartToFracture:FindFirstChild("BreakSound")
	
	local function ShardBottomLeft()
	    ShardBottomLeft.Material = PartToFracture.Material
	    ShardBottomLeft.Color = PartToFracture.Color
	    ShardBottomLeft.Transparency = PartToFracture.Transparency
	    ShardBottomLeft.Size = PartToFracture.Size - Vector3.new(0, (PartToFracture.Size.Y / 2) - BreakingPointY, (PartToFracture.Size.Z / 2) + BreakingPointZ)
	    local OldSizeY = ShardBottomLeft.Size.Y
	    local OldSizeZ = ShardBottomLeft.Size.Z
	    ShardBottomLeft.CFrame = PartToFracture.CFrame * CFrame.new(0, BreakingPointY - (ShardBottomLeft.Size.Y / 2), BreakingPointZ + (ShardBottomLeft.Size.Z / 2))
	    ShardBottomLeft.CFrame = ShardBottomLeft.CFrame * CFrame.Angles(math.rad(90), 0, 0)
	    ShardBottomLeft.Size = Vector3.new(ShardBottomLeft.Size.X, OldSizeZ, OldSizeY)
	    local ShardBottomLeft2 = ShardBottomLeft:Clone()
	    ShardBottomLeft2.CFrame = ShardBottomLeft2.CFrame * CFrame.Angles(math.rad(180), 0, 0)
	end
	
	local function ShardBottomRight()
	   ShardBottomRight.Material = PartToFracture.Material
	   ShardBottomRight.Color = PartToFracture.Color
	   ShardBottomRight.Transparency = PartToFracture.Transparency
	   ShardBottomRight.Size = PartToFracture.Size - Vector3.new(0, (PartToFracture.Size.Y / 2) + BreakingPointY, (PartToFracture.Size.Z / 2) + BreakingPointZ)
	   ShardBottomRight.CFrame = PartToFracture.CFrame * CFrame.new(0, BreakingPointY + (ShardBottomRight.Size.Y / 2), BreakingPointZ + (ShardBottomRight.Size.Z / 2))
	   local ShardBottomRight2 = ShardBottomRight:Clone()
	   ShardBottomRight2.CFrame = ShardBottomRight2.CFrame * CFrame.Angles(math.rad(180), 0, 0)
	end
	
	local function ShardTopLeft()
	    ShardTopLeft.Material = PartToFracture.Material
	    ShardTopLeft.Color = PartToFracture.Color
	    ShardTopLeft.Transparency = PartToFracture.Transparency
	    ShardTopLeft.Size = PartToFracture.Size - Vector3.new(0, (PartToFracture.Size.Y / 2) + BreakingPointY, (PartToFracture.Size.Z / 2) - BreakingPointZ)
	    local OldSizeY = ShardTopLeft.Size.Y
  	    local OldSizeZ = ShardTopLeft.Size.Z
	    ShardTopLeft.CFrame = PartToFracture.CFrame * CFrame.new(0, BreakingPointY + (ShardTopLeft.Size.Y / 2), BreakingPointZ - (ShardTopLeft.Size.Z / 2))
	    ShardTopLeft.CFrame = ShardTopLeft.CFrame * CFrame.Angles(math.rad(90), 0, 0)
	    ShardTopLeft.Size = Vector3.new(ShardTopLeft.Size.X, OldSizeZ, OldSizeY)
	    local ShardTopLeft2 = ShardTopLeft:Clone()
		ShardTopLeft2.CFrame = ShardTopLeft2.CFrame * CFrame.Angles(math.rad(180), 0, 0)
	end
	
	local function ShardTopRight()
	    ShardTopRight.Material = PartToFracture.Material
	    ShardTopRight.Color = PartToFracture.Color
	    ShardTopRight.Transparency = PartToFracture.Transparency
	    ShardTopRight.Size = PartToFracture.Size - Vector3.new(0, (PartToFracture.Size.Y / 2) - BreakingPointY, (PartToFracture.Size.Z / 2) - BreakingPointZ)
	    ShardTopRight.CFrame = PartToFracture.CFrame * CFrame.new(0, BreakingPointY - (ShardTopRight.Size.Y / 2), BreakingPointZ - (ShardTopRight.Size.Z / 2))
	    local ShardTopRight2 = ShardTopRight:Clone()
	    ShardTopRight2.CFrame = ShardTopRight2.CFrame * CFrame.Angles(math.rad(180), 0, 0)
	end
	
	local ShardDictionary = {ShardBottomLeft, ShardBottomRight, ShardTopLeft, ShardTopRight}
	
	for _, Shard in ipairs(ShardDictionary) do
		Shard.Velocity = PartToFracture.Velocity
		Shard.RotVelocity = PartToFracture.RotVelocity
		Shard.Anchored = AnchorDebris
		Shard.Name = "Shard"
		Shard.Parent = PartToFracture.Parent
		if DebrisDespawn then
			Debris:AddItem(Shard, DebrisDespawnDelay)
		end
	end
	
	PartToFracture:Destroy()
	
end

return module

-- System by Ethanthegrand14

I wrote it fast so expect a typo or two, and make sure to call some of the functions.

1 Like
#11

But also, remember that optimization is also important. Sometimes having unneeded lines of code could impact your game if you do it way too often.

1 Like
#12

Oh, your right about the category. This thread should now be in #help-and-feedback:code-review

#13

You can do the same thing with very little code

1 Like
#14

This codes really wet, there’s a lot of pointless repitition just with different variables.
Learn to use loops and functions to stop repeating the same steps multiple times in the same place.
Planning your code architecture is also important, I suggest draw.io

1 Like
#15

Thanks, but you did anything but reduce lines. You’ve increased lines. But i do like your functions!

#16

Yeah I did but it is at least more efficient than your previous one. Longer lines don’t matter.

#17

Though one question. What that point of functioning each of the 4 shards if your going to run it once anyways?