-
Notifications
You must be signed in to change notification settings - Fork 41
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
Remove nested models #230
Merged
Merged
Remove nested models #230
Changes from all commits
Commits
Show all changes
44 commits
Select commit
Hold shift + click to select a range
b54cd35
Fix error caused when the same link name is used in different models
azeey 6c59b58
Make LoadWorld return WorldPtr
azeey d7257a4
Handle BodyNodes moving to other skeletons
azeey bcfc983
Codecheck
azeey baed701
Remove unnecessary functions
azeey 372cd27
Ensure Link and Model APIs continue to work after joint creation
azeey 353410c
Add empty nested model construction and nested model entity management
azeey 96f46c8
Add empty nested model construction and nested model entity managemen…
azeey f9af478
Remove nested models in TPE
azeey 6bb3c6c
Remove nested models in dartsim
azeey 8515664
Merge remote-tracking branch 'upstream/main' into keep_track_of_links
azeey 0bb7d9e
Address reviewer feedback
azeey 16da5e1
Fix typo, add const ref
azeey 19b0768
Merge branch 'keep_track_of_links' into dartsim_nested_entities
azeey ab95922
Merge remote-tracking branch 'upstream/main' into dartsim_nested_enti…
azeey 6e25256
Merge branch 'dartsim_nested_entities' into tpe_nested_entities
azeey dca717a
Merge remote-tracking branch 'upstream/main' into dartsim_nested_enti…
azeey b836a0a
Address reviewer feedback
azeey d137bba
Make nested model API more explicit by using "nested" in function names
azeey 212c941
Fix bug in nested model construction
azeey a652ef2
Add a test for AddNestedModel
azeey 50f491d
Merge remote-tracking branch 'upstream/main' into dartsim_nested_enti…
azeey a9cc99c
Merge branch 'main' into dartsim_nested_entities
adlarkin 983aee1
Merge branch 'dartsim_nested_entities' of github.com:azeey/ign-physic…
azeey 3f84880
Merge branch 'dartsim_nested_entities' into tpe_nested_entities
azeey f0a8492
Fix function names after merge
azeey e17aed3
Address reviewer feedback
azeey 11407c5
Merge remote-tracking branch 'upstream/main' into tpe_nested_entities
azeey ca61e24
Merge branch 'tpe_nested_entities' into remove_nested_models
azeey 0b5e9b6
Fix function renames after merge
azeey 7012c67
Fix test failures due to separate bookkeeping of links and nested mod…
azeey e5679df
Fix entity by index lookup, address reviewer feedback
azeey adccd2a
Merge branch 'tpe_nested_entities' into remove_nested_models
azeey 31474ee
Fix windows warning
azeey 2254dc5
Merge branch 'main' into tpe_nested_entities
adlarkin ae53c57
Merge branch 'tpe_nested_entities' into remove_nested_models
azeey 62abc8b
Reviewer feedback
azeey 641d342
Merge remote-tracking branch 'upstream/main' into remove_nested_models
azeey 27ba015
Remove unnecessary header
azeey ef11291
Merge remote-tracking branch 'upstream/main' into remove_nested_models
azeey b3b9ae7
Rename RemoveModel to RemoveNestedModel in the RemoveNestedModelFromM…
azeey ea42d0c
Small fixes addressing reviewer feedback
azeey 27daecb
Refactor entity removal code
azeey 5c87a07
Add a TODO about de-duplicating model removal code in dartsim
azeey File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic here is pretty similar to
RemoveNestedModelByIndex
, I think this function could call that one.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it would be trivial to be able to call
RemoveNestedModelByIndex
because we have to find the index based on the name first. Ideally the linescould go into
RemoveModelImpl
, but that would involve movingGetFilterPtr
intoBase.hh
. How about we revisit this later? I can leave a TODO for myself in the code.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah later is fine, it was just a small suggestion