-
Notifications
You must be signed in to change notification settings - Fork 287
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
Support spawning during log playback #346
Conversation
Signed-off-by: Nate Koenig <[email protected]>
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 think an alternative implementation would be to increase entityCount
within EntityComponentManagerPrivate::CreateEntityImplementation
whenever the entity being created has a higher number than the current entityCount
.
I think this would cover your use case without the need to hardcode an offset for log playback. It would also avoid exposing the ECM's internal counting mechanism in case we ever want to change it.
src/systems/log/LogPlayback.cc
Outdated
@@ -209,6 +209,10 @@ void LogPlayback::Configure(const Entity &, | |||
// Prepend working directory if path is relative | |||
this->dataPtr->logPath = common::absPath(this->dataPtr->logPath); | |||
|
|||
// Set the entity offset. | |||
// \todo This number should be included in the log file. | |||
_ecm.SetEntityOffset(90000); |
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.
How about ticketing an issue for keeping track of the max entity count during log record?
Also, maybe a number like std::numeric_limits<int64_t>::max() / 2
could be more generic than the current number.
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, I tried that but it fails when the log file spawns a new entity. For example:
|
Signed-off-by: Nate Koenig <[email protected]>
Signed-off-by: Nate Koenig <[email protected]>
Codecov Report
@@ Coverage Diff @@
## ign-gazebo2 #346 +/- ##
================================================
+ Coverage 53.78% 78.12% +24.33%
================================================
Files 121 183 +62
Lines 5838 10025 +4187
================================================
+ Hits 3140 7832 +4692
+ Misses 2698 2193 -505
Continue to review full report at Codecov.
|
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 tried that but it fails when the log file spawns a new entity.
Oh got it, makes sense, I didn't think about the order of operations.
This automatically includes the
usercommands
plugin during log playback in order to support spawning on new models.I added in a
SetEntityCreateOffset
function to prevent entity ID collisions. Logplayback will directly insert entities with their recorded ids, which doesn't alter theentityCount
. As a result, a spawn call will attempt to create an entity with an existing ID.See issue #347
Signed-off-by: Nate Koenig [email protected]