Skip to content
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

[SofaCore] FIX SingleLink clear/set behavior #1749

Merged

Conversation

damienmarchal
Copy link
Contributor

@damienmarchal damienmarchal commented Jan 27, 2021

When using Single with/without storepath the way a link should behave is "unclear" and inconsitant with multlink. A multilink of size 1 can contains a nullptr while a single link containing a nullptr has size zero (which has side effect on path attributes). Now the size of a SingleLink is either 0 or 1 and a link can contain either nullptr or a path. Said differently, a SingleLink with a path set now report a size of 1.

To fix the bug it was needed to change the SinglePtr signature (which is breaking but as this is an internal feature of SingleLink should be problematic)


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

  • it builds with SUCCESS for all platforms on the CI.
  • it does not generate new warnings.
  • it does not generate new unit test failures.
  • it does not generate new scene test failures.
  • it does not break API compatibility.
  • it is more than 1 week old (or has fast-merge label).

@damienmarchal damienmarchal added pr: fast merge Minor change that can be merged without waiting for the 7 review days pr: fix Fix a bug pr: status to review To notify reviewers to review this pull-request labels Jan 28, 2021
@jnbrunet
Copy link
Contributor

Ok to me once the CI has finished.

@damienmarchal damienmarchal added issue: bug (minor) Bug affecting only some users or with no major impact on the framework pr: breaking Change possibly inducing a compilation error labels Jan 28, 2021
@damienmarchal
Copy link
Contributor Author

As it change the behavior of a core component i prefer to validate on regression files.
[ci-build][with-scene-tests][with-regression-tests]

@jnbrunet jnbrunet added pr: status ready Approved a pull-request, ready to be squashed and removed pr: status to review To notify reviewers to review this pull-request labels Jan 28, 2021
@jnbrunet jnbrunet merged commit 5006ffc into sofa-framework:master Jan 28, 2021
@guparan guparan added this to the v21.06 milestone Feb 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
issue: bug (minor) Bug affecting only some users or with no major impact on the framework pr: breaking Change possibly inducing a compilation error pr: fast merge Minor change that can be merged without waiting for the 7 review days pr: fix Fix a bug pr: status ready Approved a pull-request, ready to be squashed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants