Attachment Throughout Part

[JerimiahBatres (magnata)]

I’m making a hitbox system which requires attachments to be placed throughout a part. My friend helped me throw together some barebones code, but it’s pretty messy, and I feel could use some rewriting. I’m not too good with vectors though, so I’d like some reviewing.

---

What does your code do?

• Create attachments going from the origin point of the part, going up and down the X axis, Z axis, and Y axis.
  

---

Code

local function makeAttachment(Parent, Position)
	local Attachment = Instance.new("Attachment")
	Attachment.Parent = Parent
	Attachment.Position = Position
	
	return Attachment
end

local Quantity = 4

local Core = makeAttachment(Part, Vector3.new(0, 0, 0))
Core.Name = "0"

-- x axis
for i = 1, 4 do
	if i > 1 then
		local prevAttach = Part:FindFirstChild("X Attachment "..i - 1)
		local Position = Vector3.new((Part.Size.X / 2 / Quantity + prevAttach.Position.X), 0, 0)
		local Attachment = makeAttachment(Part, Position)
		Attachment.Name = "X Attachment "..i

		print(Attachment.Position)
	else
		local Position = Vector3.new((Part.Size.X / 2 / Quantity), 0, 0)
		local Attachment = makeAttachment(Part, Position)
		Attachment.Name = "X Attachment "..i

		print(Attachment.Position)
	end
end

-- minus x axis

for i = 1, 4 do
	if i > 1 then
		local prevAttach = Part:FindFirstChild("-X Attachment "..i - 1)
		local Position = Vector3.new((-Part.Size.X / 2 / Quantity - -prevAttach.Position.X), 0, 0)
		local Attachment = makeAttachment(Part, Position)
		Attachment.Name = "-X Attachment "..i

		print(Attachment.Position)
	else
		local Position = Vector3.new((-Part.Size.X / 2 / Quantity), 0, 0)
		local Attachment = makeAttachment(Part, Position)
		Attachment.Name = "-X Attachment "..i

		print(Attachment.Position)
	end
end

-- y axis

for i = 1, 4 do
	if i > 1 then
		local prevAttach = Part:FindFirstChild("Y Attachment "..i - 1)
		local Position = Vector3.new(0, (Part.Size.Y / 2 / Quantity + prevAttach.Position.Y), 0)
		local Attachment = makeAttachment(Part, Position)
		Attachment.Name = "Y Attachment "..i

		print(Attachment.Position)
	else
		local Position = Vector3.new(0, (Part.Size.Y / 2 / Quantity), 0)
		local Attachment = makeAttachment(Part, Position)
		Attachment.Name = "Y Attachment "..i

		print(Attachment.Position)
	end
end

-- minus y axis

for i = 1, 4 do
	if i > 1 then
		local prevAttach = Part:FindFirstChild("-Y Attachment "..i - 1)
		local Position = Vector3.new(0, (-Part.Size.Y / 2 / Quantity - -prevAttach.Position.Y), 0)
		local Attachment = makeAttachment(Part, Position)
		Attachment.Name = "-Y Attachment "..i

		print(Attachment.Position)
	else
		local Position = Vector3.new(0, (-Part.Size.Y / 2 / Quantity), 0)
		local Attachment = makeAttachment(Part, Position)
		Attachment.Name = "-Y Attachment "..i

		print(Attachment.Position)
	end
end

-- z axis

for i = 1, 4 do
	if i > 1 then
		local prevAttach = Part:FindFirstChild("Z Attachment "..i - 1)
		local Position = Vector3.new(0, 0, (Part.Size.Z / 2 / Quantity + prevAttach.Position.Z))
		local Attachment = makeAttachment(Part, Position)
		Attachment.Name = "Z Attachment "..i

		print(Attachment.Position)
	else
		local Position = Vector3.new(0, 0, (Part.Size.Z / 2 / Quantity))
		local Attachment = makeAttachment(Part, Position)
		Attachment.Name = "Z Attachment "..i

		print(Attachment.Position)
	end
end

-- minus z attachment

for i = 1, 4 do
	if i > 1 then
		local prevAttach = Part:FindFirstChild("-Z Attachment "..i - 1)
		local Position = Vector3.new(0, 0, (-Part.Size.Z / 2 / Quantity - -prevAttach.Position.Z))
		local Attachment = makeAttachment(Part, Position)
		Attachment.Name = "-Z Attachment "..i

		print(Attachment.Position)
	else
		local Position = Vector3.new(0, 0, (-Part.Size.Z / 2 / Quantity))
		local Attachment = makeAttachment(Part, Position)
		Attachment.Name = "-Z Attachment "..i

		print(Attachment.Position)
	end
end

Place File

attachment test.rbxl (22.8 KB)

[ranto828 (Ghomaghity)]

When attempting to clean up code, one of the first things I do is look at what is being unnecessarily repeated, so let’s start with that.

In each loop, you are taking the part’s size, dividing it by 2, then dividing it by quantity. However, all three of these values do not change over the course of the script running. Instead of calculating the same value over each iteration, we can pre calculate it as a constant at the top of the script and replace all instances of the division with the constant. (This works because you can directly divide and multiply Vector3s with numbers)

local offsetConst = Part.Size / 2 / Quantity -- offsetConst (because of Part.Size) is a Vector3

...

    local prevAttach = Part:FindFirstChild("Z Attachment "..i - 1)
    local Position = Vector3.new(prevAttach.Position.X + offsetConst.X, 0, 0)
    local Attachment = makeAttachment(Part, Position)

...

else
    local Position = Vector3.new(offsetConst.X, 0, 0)
    local Attachment = makeAttachment(Part, Position)

Now that we’ve done this, something’s been revealed. prevAttach.Position looks to just be the sum of all previous additions with offsetConst! Does that sound familiar? Yep, multiplication. Adding offsetConst to prevAttach.Position ends up being equivalent to multiplying offsetConst by the loop iteration we are currently in! This means that we can get rid of prevAttach and instead multiply by “i”.

for i = 1, 4 do
    if i > 1 then
	    local Position = Vector3.new(offsetConst.X * i, 0, 0)
	    local Attachment = makeAttachment(Part, Position)
	    Attachment.Name = "X Attachment "..i

	    print(Attachment.Position)
    else
	    local Position = Vector3.new(offsetConst.X, 0, 0)
	    local Attachment = makeAttachment(Part, Position)
	    Attachment.Name = "X Attachment "..i

	    print(Attachment.Position)
    end
end

Once again, something has shown up! Both sides of the if statement seem almost identical, and in fact, they both act functionally identically. Because of this, we can combine both sides of the if statement, making this loop as clean as currently possible.

for i = 1, Quantity do -- I'm assuming this is supposed to be Quantity
	local Position = Vector3.new(offsetConst.X * i, 0, 0)
	local Attachment = makeAttachment(Part, Position)
	Attachment.Name = "X Attachment "..i

	print(Attachment.Position)
end

Looking back at the entire script, this does make everything look a lot cleaner. However, there is still more that can be done.

Something that has become more apparent after cleaning is how similar all the code looks. The only difference between each loop is the axis that the attachments are being made for. Luckily, there is a solution for this. Each loop deals with an axis, one deals with +X (offsetConst.X), another deals with -Z (-offsetConst.Z), etc. Now imagine instead of looking at this as “offsetConst.X” or “-offsetConst.Z”, we looked at it as

offsetConst * Vector3.new(1, 0, 0) -- X
offsetConst * Vector3.new(0, 0, -1) -- -Z

which ends up being the same as

Vector3.new(offsetConst.X * 1, offsetConst.Y * 0, offsetConst.Z * 0)  -- X
Vector3.new(offsetConst.X * 0, offsetConst.Y * 0, offsetConst.Z * -1) -- -Z

Which is the same as

Vector3.new(offsetConst.X, 0, 0)  -- X
Vector3.new(0, 0 -offsetConst.Z) -- -Z

Notice how similar this is to the current code! Having a Vector3 with only a 1 or -1 in the desired axis acts as a sort of “and” with the other Vector3 we’re multiplying it with. We can use this and create a list of axis we want to create the attachments for, iterate through the axis list, and multiply the value with offsetConst. This behavior acts the same as having 6 different for loops with each axis!

local axis = {
    ["X"] = Vector3.new(1, 0, 0), -- "X"
    ["-X"] = Vector3.new(-1, 0, 0), -- "-X"
    ["Y"] = Vector3.new(0, 1, 0), -- "Y"
    ["-Y"] = Vector3.new(0, -1, 0), -- "-Y"
    ["Z"] = Vector3.new(0, 0, 1), -- "Z"
    ["-Z"] = Vector3.new(0, 0, -1), -- "-Z"
}

Hold up! You might notice the name of each axis is actually the key of the Vector3 in the table. The reason to do this is that you can get the name of the axis for free without requiring a separate table. You can then use “pairs” and get both key and value!

for key, currentAxis in pairs(axis) do
    for i = 1, Quantity do
	    local Position = (offsetConst * currentAxis) * i -- in the case of "X", this is the same as
                                                         -- Vector3.new(offsetConst.X, 0, 0) * i. 
                                                         -- The "i" can be multiplied with the Vector3, as explained earlier
	    local Attachment = makeAttachment(Part, Position)
        Attachment.Name = key .. " Attachment " .. i -- in the case of "X", "key" will also equal "X"
                                                     -- i.e., the name will be "X Attachment 1"
	    print(Attachment.Position)
    end
end

And that’s it for major cleaning. All that’s left is some house keeping stuff, like moving variables together, moving function declaration below variables, etc… Something to note is when using Instance.new(), avoid parenting the instance until you are done setting it’s properties, as it affects performance.

Finally, the new, cleaned code would look something like this:

Code spoilers!

local part = script.Parent
local quantity = 4
local offsetConst = part.Size / 2 / quantity;
local axis = {
	["X"] = Vector3.new(1, 0, 0),
	["-X"] = Vector3.new(-1, 0, 0),
    ["Y"] = Vector3.new(0, 1, 0),
    ["-Y"] = Vector3.new(0, -1, 0),
    ["Z"] = Vector3.new(0, 0, 1),
    ["-Z"] = Vector3.new(0, 0, -1),
}

local function makeAttachment(Parent, Position)
    local Attachment = Instance.new("Attachment")
    Attachment.Position = Position
    Attachment.Parent = Parent

    return Attachment
end

local core = makeAttachment(part, Vector3.new(0, 0, 0))
core.Name = "0"

for name, currentAxis in pairs(axis) do
    for i = 1, quantity do
        local Position = offsetConst * currentAxis * i
	    local Attachment = makeAttachment(part, Position)
	    Attachment.Name = name .. " Attachment " .. i

	    print(Attachment.Position)
    end
end

And here’s the result in game:

If you have any questions, don’t be afraid to ask! I’m not that great at explaining stuff, so I’d be happy to try and clarify.