fix READ_ONLY for real

Author: Gin

Common/Cpp/Options/ConfigOption.h

@@ -27,7 +27,7 @@ enum class LockMode{
 };
 enum class ConfigOptionState{
     ENABLED,
-    DISABLED,
+    DISABLED, // aka locked
     HIDDEN,
 };
 
@@ -93,10 +93,12 @@ class ConfigOption{
     }
 
 public:
-    // Return the lock mode: how locking works on this option. It can be:
-    // - UNLOCK_WHILE_RUNNING,
-    // - LOCK_WHILE_RUNNING,
-    // - READ_ONLY,
+    //  Return the lock mode: how locking works on this option. It can be:
+    //  - UNLOCK_WHILE_RUNNING,
+    //  - LOCK_WHILE_RUNNING,
+    //  - READ_ONLY, (aka always locked)
+    //  This value is const throughout the ConfigOption lifetime. It is set
+    //  when constructing the ConfigOption.
     LockMode lock_mode() const;
 
     //  Returns error message if invalid. Otherwise returns empty string.
@@ -108,7 +110,17 @@ class ConfigOption{
     //  transient state that the option object may have.
     virtual void reset_state(){};
 
+    //  Thread-safe: return the current visibility state. It can be:
+    //  - ENABLED
+    //  - DISABLED
+    //  - HIDDEN
     ConfigOptionState visibility() const;
+    //  Thread-safe: set the option's visibility state. It can be:
+    //  - ENABLED
+    //  - DISABLED
+    //  - HIDDEN
+    //  If visibility changed, all attached listeners' on_config_visibility_changed()
+    //  will be called.
     virtual void set_visibility(ConfigOptionState visibility);
 
 

Common/Qt/Options/ConfigWidget.cpp

@@ -46,6 +46,11 @@ void ConfigWidget::update_visibility(){
 //    cout << "ConfigWidget::update_visibility(): " << (int)m_value.visibility() << endl;
     switch (m_value.visibility()){
     case ConfigOptionState::ENABLED:
+        // setEnable(false) only happens when the lock mode is LOCK_WHILE_RUNNING and the program is running.
+        // Ideally we should handle lock mode READ_ONLY here too, but m_widget as a QWidget does not have such
+        // function. Only some of its derived classes, like QLineEdit, can call setReadOnly().
+        // So here we treat READ_ONLY as enabled and let derived classes of ConfigWidget that have the
+        // functionality to set read-only (e.g. StringOptionWidget) to handle READ_ONLY.
         m_widget->setEnabled(m_value.lock_mode() != LockMode::LOCK_WHILE_RUNNING || !m_program_is_running);
         m_widget->setVisible(true);
         break;

Common/Qt/Options/ConfigWidget.h

@@ -36,11 +36,23 @@ class ConfigWidget : protected ConfigOption::Listener{
 
     //  Needs to be called on the UI thread.
     virtual void update_value(){}
+    //  Needs to be called on the UI thread. Update QWidget's visibility based on
+    //  ConfigOption m_value's visibility setting.
+    //  Note this function is unable to set READ_ONLY visibility state. The derived
+    //  classes of ConfigWidget who have the functionality to set read-only should
+    //  overwrite on_config_visibility_changed() to handle that.
     virtual void update_visibility();
     void update_visibility(bool program_is_running);
     void update_all(bool program_is_running);
 
 protected:
+    //  Overwrite ConfigOption::Listener::on_config_visibility_changed().
+    //  Called when the listened config option's visibility is changed.
+    //  This function invokes this->update_visibility() on the Qt UI thread.
+    //  Note due to update_visibility() is unable to set READ_ONLY visbility state.
+    //  It is up to the derived classes of ConfigWidget who have the functionality
+    //  to set read-only (e.g. StringOptionWidget) to overwrite this function to
+    //  handle read-only.
     virtual void on_config_visibility_changed() override;
     virtual void on_program_state_changed(bool program_is_running) override;
 

Common/Qt/Options/StringWidget.cpp

@@ -9,9 +9,9 @@
 #include <QLineEdit>
 #include "StringWidget.h"
 
-//#include <iostream>
-//using std::cout;
-//using std::endl;
+// #include <iostream>
+// using std::cout;
+// using std::endl;
 
 namespace PokemonAutomation{
 
@@ -59,9 +59,22 @@ void StringCellWidget::on_config_value_changed(void* object){
     }, Qt::QueuedConnection);
 }
 void StringCellWidget::on_config_visibility_changed(){
+    // Overwrite ConfigWidget::on_config_visibility_changed() because ConfigWidget cannot handle
+    // READ_ONLY state.
+    this->setEnabled(true);
     QMetaObject::invokeMethod(this, [this]{
-        update_visibility();
-        setReadOnly(m_value.lock_mode() == LockMode::READ_ONLY || m_value.is_locked());
+        const ConfigOptionState visibility = m_value.visibility();
+        this->setReadOnly(visibility != ConfigOptionState::ENABLED || 
+            m_value.lock_mode() == LockMode::READ_ONLY || m_value.is_locked());
+        switch (visibility){
+        case ConfigOptionState::ENABLED:
+        case ConfigOptionState::DISABLED:
+            this->setVisible(true);
+            break;
+        case ConfigOptionState::HIDDEN:
+            this->setVisible(false);
+            break;
+        }
     }, Qt::QueuedConnection);
 }
 
@@ -109,9 +122,22 @@ void StringOptionWidget::on_config_value_changed(void* object){
     }, Qt::QueuedConnection);
 }
 void StringOptionWidget::on_config_visibility_changed(){
-    QMetaObject::invokeMethod(m_box, [this]{
-        update_visibility();
-        m_box->setReadOnly(m_value.is_locked());
+    // Overwrite ConfigWidget::on_config_visibility_changed() because ConfigWidget cannot handle
+    // READ_ONLY state.
+    this->setEnabled(true);
+    QMetaObject::invokeMethod(this, [this]{
+        const ConfigOptionState visibility = m_value.visibility();
+        m_box->setReadOnly(visibility != ConfigOptionState::ENABLED || 
+            m_value.lock_mode() == LockMode::READ_ONLY || m_value.is_locked());
+        switch (visibility){
+        case ConfigOptionState::ENABLED:
+        case ConfigOptionState::DISABLED:
+            this->setVisible(true);
+            break;
+        case ConfigOptionState::HIDDEN:
+            this->setVisible(false);
+            break;
+        }
     }, Qt::QueuedConnection);
 }
 

SerialPrograms/Source/ML/Programs/ML_LabelImages.cpp

@@ -513,11 +513,10 @@ void LabelImages::on_config_value_changed(void* object){
             FORM_LABEL.set_visibility(ConfigOptionState::HIDDEN);
             CUSTOM_SET_LABEL.set_visibility(ConfigOptionState::ENABLED);
             MANUAL_LABEL.set_visibility(ConfigOptionState::HIDDEN);
-        } else {
+        } else { // value == 2
             FORM_LABEL.set_visibility(ConfigOptionState::HIDDEN);
             CUSTOM_SET_LABEL.set_visibility(ConfigOptionState::HIDDEN);
             MANUAL_LABEL.set_visibility(ConfigOptionState::ENABLED);
-            // cout << "MANUAL_LABEL enabeld" << endl;
         }
     }