Support world reset #1249
Merged
Support world reset #1249
Conversation
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
chapulina
reviewed
Dec 14, 2021
mjcarroll
force-pushed
the
reset
branch
2 times, most recently
from
5ee4836 to
9433bd0
Compare
|
This will be cleaner when we get #1340 landed and forward ported |
Co-authored-by: Addisu Z. Taddese <[email protected]> Signed-off-by: Michael Carroll <[email protected]>
Co-authored-by: Addisu Z. Taddese <[email protected]> Signed-off-by: Michael Carroll <[email protected]>
Signed-off-by: Addisu Z. Taddese <[email protected]>
Signed-off-by: Michael Carroll <[email protected]>
mjcarroll
approved these changes
Mar 14, 2022
Signed-off-by: Michael Carroll <[email protected]>
Codecov Report
@@ Coverage Diff @@
## main #1249 +/- ##
==========================================
+ Coverage 62.22% 62.34% +0.12%
==========================================
Files 316 317 +1
Lines 24317 24419 +102
==========================================
+ Hits 15131 15224 +93
- Misses 9186 9195 +9
Continue to review full report at Codecov.
|
|
@osrf-jenkins retest this please |
chapulina
reviewed
Mar 23, 2022
There was a problem hiding this comment.
I believe this PR broke compilation on Windows:
[1830.187s] EntityComponentManager_TEST.obj : error LNK2001: unresolved external symbol "public: void __cdecl ignition::gazebo::v7::EntityComponentManagerDiff::ClearRemovedEntities(void)" (?ClearRemovedEntities@EntityComponentManagerDiff@v7@gazebo@ignition@@QEAAXXZ) [C:\Jenkins\workspace\ign_gazebo-pr-win\ws\build\ignition-gazebo7\src\UNIT_EntityComponentManager_TEST.vcxproj]
[1830.187s] EntityComponentManager_TEST.obj : error LNK2001: unresolved external symbol "public: class std::vector<unsigned __int64,class std::allocator<unsigned __int64> > const & __cdecl ignition::gazebo::v7::EntityComponentManagerDiff::AddedEntities(void)const " (?AddedEntities@EntityComponentManagerDiff@v7@gazebo@ignition@@QEBAAEBV?$vector@_KV?$allocator@_K@std@@@std@@XZ) [C:\Jenkins\workspace\ign_gazebo-pr-win\ws\build\ignition-gazebo7\src\UNIT_EntityComponentManager_TEST.vcxproj]
[1830.187s] EntityComponentManager_TEST.obj : error LNK2001: unresolved external symbol "public: class std::vector<unsigned __int64,class std::allocator<unsigned __int64> > const & __cdecl ignition::gazebo::v7::EntityComponentManagerDiff::RemovedEntities(void)const " (?RemovedEntities@EntityComponentManagerDiff@v7@gazebo@ignition@@QEBAAEBV?$vector@_KV?$allocator@_K@std@@@std@@XZ) [C:\Jenkins\workspace\ign_gazebo-pr-win\ws\build\ignition-gazebo7\src\UNIT_EntityComponentManager_TEST.vcxproj]
[1830.187s] C:\Jenkins\workspace\ign_gazebo-pr-win\ws\build\ignition-gazebo7\bin\Release\UNIT_EntityComponentManager_TEST.exe : fatal error LNK1120: 3 unresolved externals [C:\Jenkins\workspace\ign_gazebo-pr-win\ws\build\ignition-gazebo7\src\UNIT_EntityComponentManager_TEST.vcxproj]
@azeey / @mjcarroll , can you take a look?
| // Inline bracket to help doxygen filtering. | ||
| inline namespace IGNITION_GAZEBO_VERSION_NAMESPACE { | ||
|
|
||
| class EntityComponentManagerDiff |
There was a problem hiding this comment.
This class should be documented
|
I can take a look. Windows CI has been pretty flaky lately, so I merged without it. |
7 tasks
mjcarroll
added a commit
that referenced
this pull request
May 17, 2022
This builds upon the reset interface for systems introduced in #1249 by implementing the interface for the Physics system. Upon receiving a reset command, this allows the physics plugin to rewind to the initial state of the world without removing and re-adding all of the entities. Signed-off-by: Michael Carroll <[email protected]> Co-authored-by: Addisu Z. Taddese <[email protected]> Co-authored-by: Louise Poubel <[email protected]>
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
🎉 New feature
Toward #1107
Summary
This adds a new phase in which systems can implement special Reset functionality. The approach taken is to make an initial copy of the ECM after the entities have been created. When reset is requested, a diff is computed between the current ECM and the initial copy. This is necessary to ensure that entities that were added later on are marked as removed after reset. Likewise, entities that were removed later on are marked as newly added after reset.
The physics system has been updated to make use of the Reset phase. The changes were nontrivial because we keep a lot of state in the physics system to help with performance. Hopefully, updating other systems will be more straightforward.
The Reset API allows systems to make changes to the ECM. This can be used to, for example, change the poses of objects in the scene at every reset without having to reload the the world. As an example, I have created a small and very trivial joint position randomizer.
There things that still need to be cleaned up and tests that need to be added, but I wanted open this as a draft PR to get feedback on the approach.
Test it
Checklist
codecheckpassed (See contributing)Note to maintainers: Remember to use Squash-Merge