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

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

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

damienmarchal
Copy link
Contributor

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

  • 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: status to review To notify reviewers to review this pull-request pr: clean Cleaning the code labels Sep 13, 2023
@damienmarchal damienmarchal changed the title [all] Deprecated the hard coded interaction in components. [all] Deprecates the hard coded interactions in components. Sep 13, 2023
@bakpaul bakpaul changed the title [all] Deprecates the hard coded interactions in components. [all] Deprecates the hard coded interactions in components Sep 13, 2023
@bakpaul
Copy link
Contributor

bakpaul commented Sep 27, 2023

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
Copy link
Contributor Author

damienmarchal commented Sep 27, 2023

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 ?

@bakpaul bakpaul 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 Oct 11, 2023
@damienmarchal damienmarchal force-pushed the pr-deprecate-hardcoded-interaction branch from ee752d0 to 0a103be Compare April 16, 2024 11:39
@damienmarchal damienmarchal 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 Apr 16, 2024
@damienmarchal
Copy link
Contributor Author

[ci-build]

@fredroy
Copy link
Contributor

fredroy commented Apr 22, 2024

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

@bakpaul
Copy link
Contributor

bakpaul commented Apr 29, 2024

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 !

@bakpaul bakpaul 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 May 7, 2024
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 wip Development in the pull-request is still in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants