Bindings: don’t cache the command pointer

Caching it complicates add/remove command logic, so making a simple
fix for now, which can be back-ported to 2020.2; ideally we would
cache the pointer but have an invalidation scheme, but that’s
considerably more work and risk.

Relates to Sentry crash:
https://sentry.io/organizations/flightgear/issues/1858764364
This commit is contained in:
James Turner 2020-09-04 10:51:32 +01:00
parent f6656354b8
commit 52c0f04e6e
2 changed files with 23 additions and 44 deletions

View File

@ -18,24 +18,16 @@
#include <simgear/structure/exception.hxx> #include <simgear/structure/exception.hxx>
SGBinding::SGBinding() SGBinding::SGBinding()
: _command(0), : _arg(new SGPropertyNode)
_arg(new SGPropertyNode),
_setting(0)
{ {
} }
SGBinding::SGBinding(const std::string& commandName) SGBinding::SGBinding(const std::string& commandName)
: _command(0),
_arg(0),
_setting(0)
{ {
_command_name = commandName; _command_name = commandName;
} }
SGBinding::SGBinding(const SGPropertyNode* node, SGPropertyNode* root) SGBinding::SGBinding(const SGPropertyNode* node, SGPropertyNode* root)
: _command(0),
_arg(0),
_setting(0)
{ {
read(node, root); read(node, root);
} }
@ -43,8 +35,8 @@ SGBinding::SGBinding(const SGPropertyNode* node, SGPropertyNode* root)
void void
SGBinding::clear() SGBinding::clear()
{ {
_command = NULL;
_arg.clear(); _arg.clear();
_root.clear();
_setting.clear(); _setting.clear();
} }
@ -57,14 +49,12 @@ SGBinding::read(const SGPropertyNode* node, SGPropertyNode* root)
_command_name = node->getStringValue("command", ""); _command_name = node->getStringValue("command", "");
if (_command_name.empty()) { if (_command_name.empty()) {
SG_LOG(SG_INPUT, SG_WARN, "No command supplied for binding."); SG_LOG(SG_INPUT, SG_DEV_ALERT, "No command supplied for binding.");
_command = 0;
} }
_arg = const_cast<SGPropertyNode*>(node); _arg = const_cast<SGPropertyNode*>(node);
_root = const_cast<SGPropertyNode*>(root); _root = const_cast<SGPropertyNode*>(root);
_setting = 0; _setting.clear();
} }
void void
@ -78,30 +68,29 @@ SGBinding::fire() const
void void
SGBinding::innerFire () const SGBinding::innerFire () const
{ {
if (_command == 0) auto cmd = SGCommandMgr::instance()->getCommand(_command_name);
_command = SGCommandMgr::instance()->getCommand(_command_name); if (!cmd) {
if (_command == 0) { SG_LOG(SG_INPUT, SG_WARN, "No command found for binding:" << _command_name);
SG_LOG(SG_INPUT, SG_WARN, "No command attached to binding:" << _command_name); return;
} else { }
try {
if (!(*_command)(_arg, _root)) { try {
SG_LOG(SG_INPUT, SG_ALERT, "Failed to execute command " if (!(*cmd)(_arg, _root)) {
<< _command_name); SG_LOG(SG_INPUT, SG_ALERT, "Failed to execute command " << _command_name);
} }
} catch (sg_exception& e) { } catch (sg_exception& e) {
SG_LOG(SG_GENERAL, SG_ALERT, "command '" << _command_name << "' failed with exception\n" SG_LOG(SG_GENERAL, SG_ALERT, "command '" << _command_name << "' failed with exception\n"
<< "\tmessage:" << e.getMessage() << " (from " << e.getOrigin() << ")"); << "\tmessage:" << e.getMessage() << " (from " << e.getOrigin() << ")");
} }
}
} }
void void
SGBinding::fire (SGPropertyNode* params) const SGBinding::fire (SGPropertyNode* params) const
{ {
if (test()) { if (test()) {
if (params != NULL) { if (params != nullptr) {
copyProperties(params, _arg); copyProperties(params, _arg);
} }
innerFire(); innerFire();
} }
@ -122,8 +111,9 @@ SGBinding::fire (double setting) const
if (test()) { if (test()) {
// A value is automatically added to // A value is automatically added to
// the args // the args
if (_setting == 0) // save the setting node for efficiency if (!_setting) { // save the setting node for efficiency
_setting = _arg->getChild("setting", 0, true); _setting = _arg->getChild("setting", 0, true);
}
_setting->setDoubleValue(setting); _setting->setDoubleValue(setting);
innerFire(); innerFire();
} }

View File

@ -77,16 +77,6 @@ public:
*/ */
const std::string &getCommandName () const { return _command_name; } const std::string &getCommandName () const { return _command_name; }
/**
* Get the command itself.
*
* @return The command associated with this binding, or 0 if none
* is present.
*/
SGCommandMgr::Command* getCommand () const { return _command; }
/** /**
* Get the argument that will be passed to the command. * Get the argument that will be passed to the command.
* *
@ -140,7 +130,6 @@ private:
SGBinding (const SGBinding &binding); SGBinding (const SGBinding &binding);
std::string _command_name; std::string _command_name;
mutable SGCommandMgr::Command* _command;
mutable SGPropertyNode_ptr _arg; mutable SGPropertyNode_ptr _arg;
mutable SGPropertyNode_ptr _setting; mutable SGPropertyNode_ptr _setting;
mutable SGPropertyNode_ptr _root; mutable SGPropertyNode_ptr _root;