Improved bullet EntityManagementFeatures GetEntities #365
Conversation
Signed-off-by: ahcorde <[email protected]>
Signed-off-by: ahcorde <[email protected]>
Signed-off-by: ahcorde <[email protected]>
Signed-off-by: ahcorde <[email protected]>
Signed-off-by: ahcorde <[email protected]>
Signed-off-by: ahcorde <[email protected]>
Signed-off-by: ahcorde <[email protected]>
Signed-off-by: ahcorde <[email protected]>
Signed-off-by: ahcorde <[email protected]>
Signed-off-by: ahcorde <[email protected]>
Signed-off-by: ahcorde <[email protected]>
Signed-off-by: Alejandro Hernández <[email protected]>
Codecov Report
@@ Coverage Diff @@
## main #365 +/- ##
==========================================
+ Coverage 76.77% 77.44% +0.66%
==========================================
Files 128 128
Lines 5628 5724 +96
==========================================
+ Hits 4321 4433 +112
+ Misses 1307 1291 -16
Continue to review full report at Codecov.
|
| return modelName; | ||
| } | ||
|
|
||
| static const std::string modelName = this->models.at(_modelID)->name; |
There was a problem hiding this comment.
I know it was this way before, but why the use of static here?
There was a problem hiding this comment.
I think this must be a copy/paste error from line 335. It kind of makes sense on line 335 but seems definitely wrong to use here since it means every single valid model will always report having the same name, and that name will be whichever was the first one to be asked for it.
There was a problem hiding this comment.
This method return a string reference, when modelName is out of scope then the reference doesn't exists anymore. That's the only way I found to make it work. Suggestions ?
There was a problem hiding this comment.
I think this should work: #385
Basically, if it's in the map, return the reference, otherwise return a static std::string that will have a stable reference. We should be able to follow this pattern in the other places where we do this.
Returning a string reference is not a great API for this reason, but I suppose it's a bit late for that.
Signed-off-by: Michael Carroll <[email protected]>
Signed-off-by: Michael Carroll <[email protected]>
Signed-off-by: Michael Carroll <[email protected]>
Signed-off-by: Michael Carroll <[email protected]>
Signed-off-by: Michael Carroll <[email protected]>
Signed-off-by: Michael Carroll <[email protected]> Co-authored-by: Alejandro Hernández Cordero <[email protected]>
Signed-off-by: ahcorde <[email protected]>
Signed-off-by: ahcorde <[email protected]>
…into ahcorde/6/improveEntityManagerFeatures_getentities
Signed-off-by: ahcorde <[email protected]>
Signed-off-by: Michael Carroll <[email protected]>
| myMotionState, collisionShape, body}); | ||
| } | ||
|
|
||
|
|
| return modelName; | ||
| } | ||
|
|
||
| static const std::string modelName = this->models.at(_modelID)->name; |
There was a problem hiding this comment.
I think this should work: #385
Basically, if it's in the map, return the reference, otherwise return a static std::string that will have a stable reference. We should be able to follow this pattern in the other places where we do this.
Returning a string reference is not a great API for this reason, but I suppose it's a bit late for that.
|
this PR is targeting |
Signed-off-by: ahcorde [email protected]
🎉 New feature
Summary
Improved bullet EntityManagementFeatures GetEntities
Checklist
codecheckpassed (See contributing)Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining
Signed-off-bymessages.