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

DataFileNameVector and spaces #21

Closed
matthieu-nesme opened this issue Sep 22, 2016 · 6 comments
Closed

DataFileNameVector and spaces #21

matthieu-nesme opened this issue Sep 22, 2016 · 6 comments
Labels
issue: bug (major) Critical bug affecting everyone: not working, performances or accuracy degraded
Milestone

Comments

@matthieu-nesme
Copy link
Member

DataFileNameVector is a Data<vector> where the strings are separated by spaces in the string-serialization. So how to include spaces in one of the paths? (right now it is splitting this path into two separated paths).

@matthieu-nesme matthieu-nesme added the issue: bug (major) Critical bug affecting everyone: not working, performances or accuracy degraded label Sep 22, 2016
@matthieu-nesme
Copy link
Member Author

Problem demonstrated in the failing tests DataFileNameVector_test.

@matthieu-nesme
Copy link
Member Author

In fact the problem is there for any Data<vector>.
For such vectors, the string-serialization could(should) separate strings with a specific character that cannot be on a path, but it would break existing scenes.

@JeremieA
Copy link
Contributor

There was a SVector class added in sofa::helper simply to change the stream operators to use commas to delimit values and [ ] to delimit the vector itself (allowing for vectors inside vectors), but I don't like this design because the type itself is different affecting all the code that uses it...

@matthieu-nesme
Copy link
Member Author

A solution could be to replace the concerned Data with new Data with different names using SVector.

The backward compatibility could be ensure by having a specific "parse" function, looking for the old data names, and filling the new SVector by splitting at every spaces (the old way). W/o forgetting the deprecated message.

A similar process has been done in RigidMapping (Data 'rigidIndexPerPoint' replacing 'repartition').

@damienmarchal
Copy link
Contributor

The approach proposed by matthieu sounds ok to me.
I strongly support the "W/o forgetting the deprecated message".

@matthieu-nesme
Copy link
Member Author

Fixed in a99443a.

I finally changed DataFileNameVector that is now a SVector, so be aware its string serialization changed.

I made this choice because only OglShader is using DataFileNameVector in sofa-public, so the consequences of this change are small, and at least future usage of DataFileNameVector will be ok.
Of course if your private code is using DataFileNameVector, you have to update it.

@guparan guparan modified the milestone: v16.12 Jun 29, 2017
guparan added a commit that referenced this issue Feb 12, 2020
[SofaKernel] Partial FIX of the warning related to invalid reading of…
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
issue: bug (major) Critical bug affecting everyone: not working, performances or accuracy degraded
Projects
None yet
Development

No branches or pull requests

4 participants