[Citadel] Add Base and EntityManagement to tpeplugin #32
Conversation
claireyywang
changed the title
Signed-off-by: claireyywang <[email protected]>
claireyywang
changed the base branch from
claire/tpelib_add_fns
to
ign-physics2
Signed-off-by: claireyywang <[email protected]>
Signed-off-by: claireyywang <[email protected]>
Signed-off-by: claireyywang <[email protected]>
Codecov Report
@@ Coverage Diff @@
## ign-physics2 #32 +/- ##
================================================
+ Coverage 82.16% 82.21% +0.05%
================================================
Files 92 90 -2
Lines 3095 3025 -70
================================================
- Hits 2543 2487 -56
+ Misses 552 538 -14
Continue to review full report at Codecov.
|
Signed-off-by: claireyywang <[email protected]>
| { | ||
| modelName = it->second->model->GetName(); | ||
| } | ||
| return modelName; |
There was a problem hiding this comment.
I think we could use ReferenceInterface in this function to avoid lookups?
static std::string modelName =
this->ReferenceInterface<ModelInfo>(_modelID)->model->GetName();
return modelName;
I now understand that static std::string is used because of the const & return type. I think it's designed to match the return type of dart's Skeleton::getName function
| return it->second->model->GetChildCount(); | ||
| } | ||
| // return invalid count if model not found | ||
| return -1; |
There was a problem hiding this comment.
this also seems to work:
return this->ReferenceInterface<ModelInfo>(_modelID)->model->GetChildCount();
Signed-off-by: claireyywang <[email protected]>
Thanks for the review, @iche033 ! I have updated functions to use |
There was a problem hiding this comment.
looks good to me!
A side note, I noticed that code check is not running on tpe src code. We can add the check here. After that, running sh tools/code_check.sh should pick up style issues. We can do that and fix the errors in a separate PR
Awesome, thanks for catching the code check! I didn't know it needs to be manually added. I'll follow up with another PR fixing styles as suggested. |
This PR adds
EntityManagementFeaturesandBase.hhto lay the ground work of tpeplugin features. It should be merged after #45 which add functions totpeliband before #46 which adds all the other features totpeplugin.