Repository navigation
[Constraint/Lagrangian] Replace d_index by d_indices in StopperLagrangianConstraint - #6381
Conversation
This is consistant with other LagrangianConstraint.
bakpaul
left a comment
There was a problem hiding this comment.
An by the way I don't like this class at all, it is pretty bad design in term of application.
| if(!d_indices.isSet()) | ||
| { | ||
| auto indices = sofa::helper::getWriteOnlyAccessor(d_indices); | ||
| indices.resize(x.size()); | ||
| for(sofa::Index i=0;i<x.size();i++) | ||
| { | ||
| indices[i] = i; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
This changes the default behavior of the ocmpoent, forcing to apply the constraint to all points if nothing is given. To keep old behavior you should only put it to the first vertex. Otherwise this PR should be marked as breaking
There was a problem hiding this comment.
Thank for your comment,
The PR is actually already flagged as breaking.
That's said, I think the underlying question is more what are the SOFA guidelines regarding this indices/index stuff to increase component consistency.
I searched in the doc, but cannot find guidelines, maybe worth to discuss and formalize (if not done yet).
A quick look in Lagrangian constraint plugin already contains three different pattern.
There was a problem hiding this comment.
I have opened an issue regarding the behavioral in-consistencies on indices/index
#6418
Can you elaborate a bit more on what would be a good design in term of application. On my side I find it useful to implement a collision response through a non-penetration constraint by preventing values to be <0. Is there alternative component for such a thing ? |
|
Anything to change on this PR ? |
|
Last week report: Launch the scenes tests then ready |
|
I restarted Ubuntu other OS seem OK |
This allows to apply the constraint to one or more dofs instead of a single one.
This is consistant with other lagrangian constraint component.
[with-all-tests]
By submitting this pull request, I acknowledge that
I have read, understand, and agree SOFA Developer Certificate of Origin (DCO).
Reviewers will merge this pull-request only if