-
Notifications
You must be signed in to change notification settings - Fork 311
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[SofaConstraint] Issue in GenericConstraintCorrection #567
[SofaConstraint] Issue in GenericConstraintCorrection #567
Conversation
…and if not, search in Local then Root.
Thanks for your PR @EulalieCoevoet :-) |
You know which vector and which string by looking at the using xD |
My 2 cents... May I suggest two strategy:
Strategy 2 would produce a very consistent and readable code making it obvious we we are using external object or lib; EDIT: I forgot to say: 👍 Eulalie for the bugtracking, the fixing, cleaning and PRing |
@@ -36,78 +36,97 @@ namespace component | |||
namespace constraintset | |||
{ | |||
|
|||
using helper::vector; | |||
using std::string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will depend on which rules we use...but at least don't import std::string and sofa::helper::vector
I agree with your suggestion 2 @damienmarchal. We should add this somewhere in the rules. |
Sounds good to me :) |
Yup, I guess it only concerns the two lines highlighted by Damien :-) |
…ternal type eg 'using std::vector' or 'using std::string' is forbidden
if(d_ODESolverName.isSet()) | ||
context->get(m_ODESolver, d_ODESolverName.getValue()); | ||
|
||
if(m_ODESolver == NULL) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fjourdes if you have some time can you drop an eye on the code changes made by Eulalie...before it was fetching the Local only...now Global is a fallback. Could it have bad consequence ?
[ci-build][with-scene-tests] |
In the PR #484, the way of searching the ODESolver in GenericConstraintCorrection changed from
c->get(odesolver, core::objectmodel::BaseContext::SearchRoot);
to
c->get(odesolver, core::objectmodel::BaseContext::Local);
In some of our simulations, the GenericConstraintCorrection is located in the root node. And so no ODE solver can be found, and the user can not set it manually.
In this PR:
This PR:
Reviewers will merge only if all these checks are true.