THREE.IK icon indicating copy to clipboard operation
THREE.IK copied to clipboard

General API Design

Open jsantell opened this issue 6 years ago • 1 comments

Currently, THREE.IK is comprised of a root IK system, IK, which can contain several root IKChains, each that may contain a tree of IKChains, which store multiple IKJoints that wrap THREE.Bone and an optional constraint.

Some general open questions:

IK/IKChain structure

  • Is this class necessary? It can contain several root chains, but they're all independently solved, so what's the benefit of having multiple root chains? And if that's the case, can an IKChain just contain multiple chains without being wrapped in an IK system? If other solvers (#2) are implemented, would that be handled in an IK class, with chains being implementation independent?
  • Currently IKChains attach new chains via connect(). The subchain's base must be a joint in the parent chain to determine where this subchain is connected. The fullik library's API for this has you specify which joint index to connect the chain to, and the subchain's base has a separate joint from the parent's. This is probably the better solution.
  • Each IKChain must have a target, or subchains on its last joint. Is this a necessary restriction? Why solve if neither of these scenarios apply?

Constraints

  • There's only one type of constraint currently, an IKBallConstraint. There are open issues for additional constraints (https://github.com/jsantell/THREE.IK/issues/3), multiple constraints per joint (#5) and handling constraints with subbases (#6), but currently they are just instances that can take a joint, and transform it by some way. Because of this, they can be shared between IKJoints.
  • Is the IKJoint-containing-a-constraint model a good one?

General API

In general, the API is difficult to use and easy to get wrong. Issues:

  • ikchain.connect(otherChain) probably should change to index-based and not share joints between chains
  • new IKJoint(bone, { constraints }) is this how constraints should be defined?
  • ikchain.add(joint, { target: target }) when adding a joint to a chain, the last joint needs a target, which makes this the end effector. This is a weird API, surely something could be better.
  • Should we support when there are dynamic changes to the bone structure? Or once it's set, there's nothing you can do? Maybe there's something that can be done where it requires setting a needsUpdate flag.

jsantell avatar Apr 23 '18 05:04 jsantell

I'm new to IK in general (learning about it, and planning to use it on a project), but I have a keen interest in API design, so let me see if I can help.

I think the current API could be simpler. Building a chain seems to always involve the same steps (adding bones, creating joints, adding chain to ik system) and they don't seem to change. I'd suggest consolidating them inside a ik.createChain() method. As an advantage, it would not prevent the developer from going full-advanced mode if they so desire.

// Going from the README example, the code below assumes 
//   we already have a `scene` with `pivot`, `movingTarget`, and 
//   a `bones` array with 10 bones.
const ik = new THREE.IK()

// Create chain from the `ik` instance
// Internally creates joints for each bone
// Automatically adds chain to IK system so `ik.add(chain)` isn't necessary
var chain = ik.createChain(...bones)

// Add a target to the chain
// If jointIndex is undefined, automatically uses last bone
chain.setTarget(movingTarget, jointIndex)

// Set a constraint for a given joint index
// If jointIndex is undefined, set constraint for the whole chain
chain.setConstraint(new THREE.IKBallConstraint(90), jointIndex)

chain.setTarget() would also be called internally when connecting chains, as if I'm understanding correctly, the target of the parent chain is the other chain's first joint. I think chain1.connect(chain2) looks good. If a different parent joint index is necessary, I don't think chain1.connect(chain2, jointIndex) would be too difficult to understand.

I'd also replace ik.getRootBone() with ik.root, where root is a getter. But that's purely semantic: scene.add(ik.root). Short and sweet. 😎

Let me know what you think.

pauldps avatar Jul 02 '18 18:07 pauldps