So I recently made joints that attach to a part, and by pressing T the selected joint rotates clockwise and sets the NEW joint as the PrimaryPart (allowing for the model to be re-positioned accordingly)
So the problem with my code is that it's super inefficient (it simply guesses the CFrame based on the variable number
Any suggestions on how to improve the code would be appreciated, and more of the code (if necessary) will be provided
Code (give me a break it's summer and I know it looks ugly):
local function GetClockwiseJoint() local target = Mouse.Target if target == nil or target.Name ~= 'Joint' then return end local j = SetJoint local cfx, cfy, cfz = j.Position.x, j.Position.y, j.Position.z local newpos if number == 1 then newpos = vec(cfx - 4, cfy, cfz + 4) elseif number == 2 then newpos = vec(cfx - 4, cfy, cfz - 4) elseif number == 3 then newpos = vec(cfx + 4, cfy, cfz - 4) elseif number == 4 then newpos = vec(cfx + 4, cfy, cfz + 4) number = 0 end number = number + 1 for nm, prt in pairs(Joints) do if prt.Position == newpos then SetJoint = prt UpdateJointPosition(SetJoint, target) --Updates the position accordingly end end end
GetClockwiseJoint doesn't seem particularly inefficient on its own. However, it seems to me that you would benefit from keeping track of the joints in a table so you don't need to "guess their location", since your entire objective seems to be to "get the next joint in a clockwise direction". If I understand your situation correctly, you'd need a dictionary that maps central brick parts to a list of their 4 joints, and maybe other information (like the last joint you looked at). You might use such a table like this:
jointData = {} --Initialize: jointData[centralPart] = {joint1, joint2, joint3, joint4, number=1} -- Note: I'm changing what your function is doing a bit, but you get the idea local function GetClockwiseJoint(target) local data = jointData[target] or error("Did not correctly initialize " .. tostring(target)) local joint = data[data.number] data.number = (data.number % 4) + 1 return joint end local joint = GetClockwiseJoint(centralPart) -- will return "joint1", then "joint2", etc -- When you no longer need centralPart, you must remove it from jointData or you will create a memory leak. If you don't know when you no longer need centralPart, let jointData be a table with weak keys
Note: An important programming principle is to not optimize before you know you need to. Often it doesn't make a noticeable difference to your users/players, and it saves you significant time. Sometimes people will try to sacrifice code quality for efficiency in places that don't end up mattering in terms of speed, but do make it harder to maintain.