Scripting Helpers is winding down operations and is now read-only. More info→
Ad
Log in to vote
0

Ways to Improve CFrame Efficiency?

Asked by 6 years ago

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

GIF of how it works

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

1 answer

Log in to vote
0
Answered by 6 years ago

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

Weak keys link

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.

Ad

Answer this question