Any bad habits spotted in V1.2 of my Inverse Kinematics FABRIK module?

Help and Feedback Code Review
#1

So I was reading this, then I was quite alarmed as I believe I went a bit overboard after discovering the power of objects oriented programming.

https://devforum.roblox.com/t/what-are-bad-programming-habits-that-you-see-often-by-scripters/761033/5?u=dthecoolest

I’m particularly concerned with using objects within objects, and how these two objects are sharing a table between them and methods as well…


--So I store these variables in the original LimbChain Object
    obj.LimbVectorTable = LimbVectorTable
    -- Stores the limb vector table in the iterated version
    obj.IteratedLimbVectorTable = IteratedLimbVectorTable

--Then I pass them into another object....is that ok?
--The other object is the FabrikSolver object which does the math stuff

--Once the limb vectors are initialized store them in a FabrikSolver object which does the Fabrik iteration
    local LimbFabrikSolver = FabrikSolver.new(IteratedLimbVectorTable,LimbLengthTable,LimbConstraintTable,obj)
--yeah obj the LimbChain itself but it's a different issue so ignore it for now
--due to the constraint system and IterateUntil and the Parts CFrames...
--A lot of stuff

--Then I store that object in this LimbChain object
    obj.LimbFabrikSolver = LimbFabrikSolver


--Inside the iterate once method
--function LimbChain:IterateOnce(parameters stuff)
--Do the iteration in the object
--Then the LimbChain copies the homework from this other object
    self.IteratedLimbVectorTable = self.LimbFabrikSolver:IterateOnce(originJointCF,targetPosition, tolerance)

Is this ok, it works but it feels a bit redundant…

I was mainly trying to pass this iteration variable back and forth through functions like in my old module before I used OOP on it like the limbLengthTable is a constant variable for a single chain. You can see this in my IKAnimation folder in replicated storage as Fabrik.lua.

--A lot of the same parameters passed back and forth

local function FabrikAlgo(tolerance, originCF, targetPos, limbVectorTable,
                          limbLengthTable, limbConstraintTable)
--do stuff
end

local function Backwards(originCF, targetPos, limbVectorTable,
                            limbLengthTable)
end

local function Forwards(originCF, targetPos, limbVectorTable, limbLengthTable)
end

You can see all the code here since it’s open source:

https://github.com/datlass/fabrik-ik-motor6d/tree/master/src/ReplicatedStorage/ObjectFolder

Plus any other issues, primarily:

  1. Code readability and structure

  2. Some other optimization issue?

1 Like
#2

Later in the day I’ll provide some more in-depth feedback—would you rather this feedback through GitHub or itemized on these forums?

Just by reviewing a random script I can see that your comments are essentially just clutter; remember, comments should describe some rationale or intention, not what the code is doing. The only people that should be reading the source code should also be able to understand the Lua language, the Luau derivative, and the Roblox APIs.

Also, be sure to remove any trailing whitespace from lines. It ruins git diffs. I saw a few lines had some.

As an aside, good job on using GitHub! Open source or not, I would conside any VCS such as Git non-optional.

1 Like
#3

I would like to learn Git workflow please, seems important if I want to work in a larger scripting team.

But I’m fine if it’s on the dev forumn since I well posted on the dev forumn.

Edit: the comments are mostly for me lol definitely a bad habit for readability issues

1 Like