Added get/setLCPSolver functions to ConstraintSolver

[mkoval]

There is currently no way to use anything other than the default LCP solver in ConstraintSolver. This pull request adds functions that parallel the get/setCollisionDetector for the LCPSolver.

[jslee02]

Passing const reference like std::shared_ptr<T> case?

[mxgrey]

That shouldn't be done for unique_ptr because we want the ownership of the solver to be passed into the ConstraintSolver. Passing by reference wouldn't allow that.

unique_ptr is really weird when it comes to function arguments.

[jslee02]

I see, thanks for the clarification. Good to know!

Also, this pull request looks good to merge.

[mkoval]

I'm not really sure whether this should be managed by smart_ptr or unique_ptr. I changed CollisionDetector from a raw pointer to a unique_ptr in #616 to maintain backwards compatibility and used the same convention here for consistency.

There are no backwards compatibility concerns with LCPSolver, since this was never publicly accessible before this pull request, so I am open to either option. @mxgrey @jslee02 Which do you prefer?

[mxgrey]

I think unique_ptr makes the most sense since sharing an LCPSolver could entail some serious multi-threading issues depending on the implementation of the LCPSolver.

Since unique_ptrs are kind of weird, I would suggest also having a function like

template <typename T, typename... Args>
void setLCPSolver(Args&&... args)
{
  setLCPSolver(std::unique_ptr<T>(new T(std::forward<Args>(args)...)));
}

For the user's convenience. This way an LCPSolver can be set like:

constraintSolver->setLCPSolver<DantzigLCPSolver>(timestep);

Instead of requiring the user to do confusing things with std::move().

[mkoval]

I am not a huge fan of that paradigm for two reasons. First, the function signature is inscrutable to people that are unfamiliar with template argument packs. Second, we have to define this type of overload for every function that accepts an std::unique_ptr.

I would prefer to provide a make_unique function in the common or utils namespace. This exists in C++14 and was omitted from C++11 as "partly an oversight".

[mxgrey]

That sounds like a good alternative to me.

[jslee02]

make_unique sounds good. I would prefer to have it in StlHelpers.h.