Skip to content

[Collision] Set Data & Links to public in Collision models. Adopt state accessor for mstate. - #5969

Open
th-skam wants to merge 8 commits into
sofa-framework:masterfrom
th-skam:point-coli-public
Open

[Collision] Set Data & Links to public in Collision models. Adopt state accessor for mstate.#5969
th-skam wants to merge 8 commits into
sofa-framework:masterfrom
th-skam:point-coli-public

Conversation

@th-skam

@th-skam th-skam commented Feb 25, 2026

Copy link
Copy Markdown
Contributor

A bit of an inconsistency since the topology link (and Data) are mostly public in other model classes. Same with Sphere coli model.
Was this by design? Let's keep one consistent style for all models.
What do you think? should they be public or private?

@th-skam th-skam added pr: status to review To notify reviewers to review this pull-request pr: clean Cleaning the code labels Feb 25, 2026

@alxbilger alxbilger left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, Links and Data should be public. In any case, they can be accessed publicly (for example using findData, or in Python), so it makes sense that they are also public in C++.

As a side note, mstate should be a link too.

@th-skam th-skam added pr: status wip Development in the pull-request is still in progress and removed pr: status to review To notify reviewers to review this pull-request labels Feb 25, 2026
@alxbilger
alxbilger force-pushed the point-coli-public branch from 30a2e49 to 3b562e4 Compare July 29, 2026 09:04
@th-skam
th-skam force-pushed the point-coli-public branch from 3b562e4 to 9458431 Compare August 10, 2026 13:11
@th-skam th-skam added pr: status to review To notify reviewers to review this pull-request and removed pr: status wip Development in the pull-request is still in progress labels Aug 10, 2026
@th-skam

th-skam commented Aug 10, 2026

Copy link
Copy Markdown
Contributor Author

Ended up with a bit of cleaning:

  • Data and Links were moved to public
  • topology pointer and Link coexisting in LineCollisionModel
  • Use SingleStateAccessor for the mstate

@th-skam th-skam changed the title [Collision] Set Data and SingleLinks to public in PointCollisionModel [Collision] Set Data & Links to public in Collision models. Adopt state accessor for mstate. Aug 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr: clean Cleaning the code pr: status to review To notify reviewers to review this pull-request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants