Codecov Report

Attention: Patch coverage is 57.62712% with 25 lines in your changes missing coverage. Please review.

Project coverage is 24.77%. Comparing base (6b81abf) to head (7dc4554).
Report is 1 commits behind head on master.

@@            Coverage Diff             @@
##           master    #1270      +/-   ##
==========================================
- Coverage   24.78%   24.77%   -0.02%     
==========================================
  Files         169      169              
  Lines       19656    19598      -58     
==========================================
- Hits         4872     4855      -17     
+ Misses      14784    14743      -41     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Could you also replace the FGPropertyNode with SGPropertyNode in the Unreal Plugin? Just these two below. The rest of the code should work fine as is. I could submit a PR later but figured it could be included with your PR. Thanks :)

I'm not a great fan of using auto a lot. Sure for fairly complicated template based types like this I have no issue.

map<int,list<string>>::iterator i = m.begin();
auto i = m.begin();

But for these sorts of cases when I'm browsing code I'd prefer to see SGPropertyNode* prop than auto prop. If I want to know whether prop is being used appropriately I need to go and take a look at getNode (name in this case sort of gives it away) to see what it's returning, is it a bool or some node pointer, if so which one FGPropertyNode* or SGPropertyNode* etc. I think it just makes it more difficult and means more work for the reader.

    bool HasNode(const std::string& path) const
    {
      std::string newPath = path;
      if (newPath[0] == '-') newPath.erase(0,1);
      auto prop = root->getNode(newPath);
      return prop;
    }

Also would prefer return prop != nullptr again to make it more explicit to the reader compared to the implicit "type-cast" to bool.

Noticed a couple of other places where the dangerous cast up to a FGPropertyNode* was happening, e.g. FGPropertyNode* operator*(), which you've now fixed. Guessing they weren't code paths that were hit with usban.

Could you also replace the FGPropertyNode with SGPropertyNode in the Unreal Plugin?

Yep! Done. Thanks for the heads up @gallonmate 👍

I'm not a great fan of using auto a lot. [...] I'd prefer to see SGPropertyNode* prop than auto prop.

Fair enough I've replaced all the new occurrences of auto by explicit type names.

Also would prefer return prop != nullptr again to make it more explicit to the reader compared to the implicit "type-cast" to bool.

Done ! Thanks for the comments @seanmcleod70.

All looks good.

All looks good.

Thanks. PR merged.