Merge pull request #5577 from ligfx/separateexpressionparsingandbinding

ControlReference/ExpressionParser: separate parsing from binding

Source/Core/Core/HotkeyManager.cpp

@@ -344,7 +344,7 @@ void HotkeyManager::LoadDefaults(const ControllerInterface& ciface)
   auto set_key_expression = [this](int index, const std::string& expression) {
     m_keys[FindGroupByID(index)]
         ->controls[GetIndexForGroup(FindGroupByID(index), index)]
-        ->control_ref->expression = expression;
+        ->control_ref->SetExpression(expression);
   };
 
   // General hotkeys

Source/Core/DolphinQt2/Config/Mapping/IOWindow.cpp

@@ -110,7 +110,7 @@ void IOWindow::CreateMainLayout()
 
 void IOWindow::Update()
 {
-  m_expression_text->setPlainText(QString::fromStdString(m_reference->expression));
+  m_expression_text->setPlainText(QString::fromStdString(m_reference->GetExpression()));
   m_range_spinbox->setValue(m_reference->range * SLIDER_TICK_COUNT);
   m_range_slider->setValue(m_reference->range * SLIDER_TICK_COUNT);
 
@@ -164,7 +164,7 @@ void IOWindow::OnDialogButtonPressed(QAbstractButton* button)
     return;
   }
 
-  m_reference->expression = m_expression_text->toPlainText().toStdString();
+  m_reference->SetExpression(m_expression_text->toPlainText().toStdString());
 
   if (button != m_apply_button)
     accept();

Source/Core/DolphinQt2/Config/Mapping/MappingButton.cpp

@@ -27,7 +27,7 @@ static QString EscapeAmpersand(QString&& string)
 }
 
 MappingButton::MappingButton(MappingWidget* widget, ControlReference* ref)
-    : ElidedButton(EscapeAmpersand(QString::fromStdString(ref->expression))), m_parent(widget),
+    : ElidedButton(EscapeAmpersand(QString::fromStdString(ref->GetExpression()))), m_parent(widget),
       m_reference(ref)
 {
   Connect();
@@ -66,7 +66,7 @@ void MappingButton::OnButtonPressed()
 
     if (!expr.isEmpty())
     {
-      m_reference->expression = expr.toStdString();
+      m_reference->SetExpression(expr.toStdString());
       Update();
     }
     else
@@ -78,21 +78,21 @@ void MappingButton::OnButtonPressed()
 
 void MappingButton::OnButtonTimeout()
 {
-  setText(EscapeAmpersand(QString::fromStdString(m_reference->expression)));
+  setText(EscapeAmpersand(QString::fromStdString(m_reference->GetExpression())));
 }
 
 void MappingButton::Clear()
 {
   m_parent->Update();
-  m_reference->expression.clear();
+  m_reference->SetExpression("");
   Update();
 }
 
 void MappingButton::Update()
 {
   const auto lock = ControllerEmu::EmulatedController::GetStateLock();
   m_reference->UpdateReference(g_controller_interface, m_parent->GetParent()->GetDeviceQualifier());
-  setText(EscapeAmpersand(QString::fromStdString(m_reference->expression)));
+  setText(EscapeAmpersand(QString::fromStdString(m_reference->GetExpression())));
   m_parent->SaveSettings();
 }
 

Source/Core/DolphinWX/Input/InputConfigDiag.cpp

@@ -70,7 +70,7 @@
 #include "InputCommon/ControllerInterface/Device.h"
 #include "InputCommon/InputConfig.h"
 
-using namespace ciface::ExpressionParser;
+using ciface::ExpressionParser::ParseStatus;
 
 void InputConfigDialog::ConfigExtension(wxCommandEvent& event)
 {
@@ -243,7 +243,7 @@ ControlButton::ControlButton(wxWindow* const parent, ControlReference* const _re
       m_configured_width(FromDIP(width))
 {
   if (label.empty())
-    SetLabelText(StrToWxStr(_ref->expression));
+    SetLabelText(StrToWxStr(_ref->GetExpression()));
   else
     SetLabel(StrToWxStr(label));
 }
@@ -336,7 +336,7 @@ void ControlDialog::SelectControl(const std::string& name)
 void ControlDialog::UpdateGUI()
 {
   // update textbox
-  textctrl->SetValue(StrToWxStr(control_reference->expression));
+  textctrl->SetValue(StrToWxStr(control_reference->GetExpression()));
 
   // updates the "bound controls:" label
   m_bound_label->SetLabel(
@@ -347,11 +347,12 @@ void ControlDialog::UpdateGUI()
   case ParseStatus::SyntaxError:
     m_error_label->SetLabel(_("Syntax error"));
     break;
-  case ParseStatus::NoDevice:
-    m_error_label->SetLabel(_("Device not found"));
+  case ParseStatus::Successful:
+    m_error_label->SetLabel(control_reference->BoundCount() > 0 ? "" : _("Device not found"));
     break;
-  default:
+  case ParseStatus::EmptyExpression:
     m_error_label->SetLabel("");
+    break;
   }
 };
 
@@ -364,7 +365,7 @@ void InputConfigDialog::UpdateGUI()
   {
     for (ControlButton* button : cgBox->control_buttons)
     {
-      button->SetLabelText(StrToWxStr(button->control_reference->expression));
+      button->SetLabelText(StrToWxStr(button->control_reference->GetExpression()));
     }
 
     for (PadSetting* padSetting : cgBox->options)
@@ -399,7 +400,7 @@ void InputConfigDialog::LoadDefaults(wxCommandEvent&)
 
 bool ControlDialog::Validate()
 {
-  control_reference->expression = WxStrToStr(textctrl->GetValue());
+  control_reference->SetExpression(WxStrToStr(textctrl->GetValue()));
 
   const auto lock = ControllerEmu::EmulatedController::GetStateLock();
   control_reference->UpdateReference(g_controller_interface,
@@ -408,7 +409,7 @@ bool ControlDialog::Validate()
   UpdateGUI();
 
   const auto parse_status = control_reference->GetParseStatus();
-  return parse_status == ParseStatus::Successful || parse_status == ParseStatus::NoDevice;
+  return parse_status == ParseStatus::Successful || parse_status == ParseStatus::EmptyExpression;
 }
 
 void InputConfigDialog::SetDevice(wxCommandEvent&)
@@ -438,7 +439,7 @@ void ControlDialog::SetDevice(wxCommandEvent&)
 
 void ControlDialog::ClearControl(wxCommandEvent&)
 {
-  control_reference->expression.clear();
+  control_reference->SetExpression("");
 
   const auto lock = ControllerEmu::EmulatedController::GetStateLock();
   control_reference->UpdateReference(g_controller_interface,
@@ -497,7 +498,7 @@ void ControlDialog::SetSelectedControl(wxCommandEvent&)
     return;
 
   textctrl->WriteText(expr);
-  control_reference->expression = textctrl->GetValue();
+  control_reference->SetExpression(WxStrToStr(textctrl->GetValue()));
 
   const auto lock = ControllerEmu::EmulatedController::GetStateLock();
   control_reference->UpdateReference(g_controller_interface,
@@ -533,7 +534,7 @@ void ControlDialog::AppendControl(wxCommandEvent& event)
   }
 
   textctrl->WriteText(expr);
-  control_reference->expression = textctrl->GetValue();
+  control_reference->SetExpression(WxStrToStr(textctrl->GetValue()));
 
   const auto lock = ControllerEmu::EmulatedController::GetStateLock();
   control_reference->UpdateReference(g_controller_interface,
@@ -637,7 +638,7 @@ void InputConfigDialog::ConfigControl(wxEvent& event)
 void InputConfigDialog::ClearControl(wxEvent& event)
 {
   ControlButton* const btn = (ControlButton*)event.GetEventObject();
-  btn->control_reference->expression.clear();
+  btn->control_reference->SetExpression("");
   btn->control_reference->range = 1.0;
 
   controller->UpdateReferences(g_controller_interface);
@@ -716,7 +717,7 @@ bool InputConfigDialog::DetectButton(ControlButton* button)
       wxString control_name = ctrl->GetName();
       wxString expr;
       GetExpressionForControl(expr, control_name);
-      button->control_reference->expression = expr;
+      button->control_reference->SetExpression(WxStrToStr(expr));
       const auto lock = ControllerEmu::EmulatedController::GetStateLock();
       button->control_reference->UpdateReference(g_controller_interface,
                                                  controller->default_device);

Source/Core/InputCommon/ControlReference/ControlReference.cpp

@@ -25,21 +25,20 @@ bool ControlReference::InputGateOn()
 // UpdateReference
 //
 // Updates a controlreference's binded devices/controls
-// need to call this to re-parse a control reference's expression after changing it
+// need to call this to re-bind a control reference after changing its expression
 //
 void ControlReference::UpdateReference(const ciface::Core::DeviceContainer& devices,
                                        const ciface::Core::DeviceQualifier& default_device)
 {
-  Expression* expr;
   ControlFinder finder(devices, default_device, IsInput());
-  m_parse_status = ParseExpression(expression, finder, &expr);
-  m_parsed_expression.reset(expr);
+  if (m_parsed_expression)
+    m_parsed_expression->UpdateReferences(finder);
 }
 
 int ControlReference::BoundCount() const
 {
   if (m_parsed_expression)
-    return m_parsed_expression->num_controls;
+    return m_parsed_expression->CountNumControls();
   else
     return 0;
 }
@@ -49,6 +48,17 @@ ParseStatus ControlReference::GetParseStatus() const
   return m_parse_status;
 }
 
+std::string ControlReference::GetExpression() const
+{
+  return m_expression;
+}
+
+void ControlReference::SetExpression(std::string expr)
+{
+  m_expression = std::move(expr);
+  std::tie(m_parse_status, m_parsed_expression) = ParseExpression(m_expression);
+}
+
 ControlReference::ControlReference() : range(1), m_parsed_expression(nullptr)
 {
 }

Source/Core/InputCommon/ControlReference/ControlReference.h

@@ -34,12 +34,14 @@ class ControlReference
   ciface::ExpressionParser::ParseStatus GetParseStatus() const;
   void UpdateReference(const ciface::Core::DeviceContainer& devices,
                        const ciface::Core::DeviceQualifier& default_device);
+  std::string GetExpression() const;
+  void SetExpression(std::string expr);
 
   ControlState range;
-  std::string expression;
 
 protected:
   ControlReference();
+  std::string m_expression;
   std::unique_ptr<ciface::ExpressionParser::Expression> m_parsed_expression;
   ciface::ExpressionParser::ParseStatus m_parse_status;
 };