sofa-framework / sofa

Real-time multi-physics simulation with an emphasis on medical simulation.
https://www.sofa-framework.org
GNU Lesser General Public License v2.1
871 stars 297 forks source link

[all] Deprecates the hard coded interactions in components #4163

Open damienmarchal opened 8 months ago

damienmarchal commented 8 months ago

Hard coding interaction in core component is not a good software design as it couple the behavior of a component and how it is used in the context of a specific interactive scene.

This PR remove this behavior and warns users. To avoid re-implementing things no one use, it is requested in the deprecation message that user need to report if they miss the feature so so we can restore it but with the proper software design.


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

bakpaul commented 7 months ago

This PR is removing some usefull mechanism (VTK and LightManager). In order to pass this PR, those two component would require to provide those controller and not just say "you should code them". One solution would be to remove them from the PR before merging it. (By the way, an issue regarding the mesh loaders and exporters will be created to be discussed and uniformised during the STC)

damienmarchal commented 7 months ago

Hello thank you for the feedback,

I'm strongly reluctant to hard code in c++ trivial keyboard controller because there is a lot of boilerplate code and because this also encourage software obesity by having all the application specific's user interfaces in Sofa Core, think about just changing the keypress "a" to "b" without recompiling sofa.

But I'm ok to implement that in a python controllers, does it would be ok ?

damienmarchal commented 1 month ago

[ci-build]

fredroy commented 3 weeks ago

[ci-build][with-all-tests]

bakpaul commented 2 weeks ago

I think that regarding the last SOFA-Dev meeting, the deprecation date is still missing. When it is added we will be able to merge it !