From 1e8b71558d60711193bb7beae1ce712fc896c52f Mon Sep 17 00:00:00 2001
From: RenechCDDA <84619419+RenechCDDA@users.noreply.github.com>
Date: Tue, 11 Jun 2024 20:50:01 -0400
Subject: [PATCH] Hopefully truly last touches to the follower rules UI
 -Extract the common button setup into a function -Only pop style color when
 it's been pushed -other very minor changes

---
 src/npctalk_rules.cpp | 218 +++++++++++++++---------------------------
 src/npctalk_rules.h   |   4 +
 2 files changed, 82 insertions(+), 140 deletions(-)

diff --git a/src/npctalk_rules.cpp b/src/npctalk_rules.cpp
index c1c368d452a99..e49e55ab93c16 100644
--- a/src/npctalk_rules.cpp
+++ b/src/npctalk_rules.cpp
@@ -87,7 +87,7 @@ template<typename T>
 void follower_rules_ui_impl::multi_rule_header( const std::string &id, T &rule,
         const std::map<T, std::string> &rule_map, bool should_advance )
 {
-    if( ImGui::InvisibleButton( id.c_str(), ImVec2() ) || should_advance ) {
+    if( ImGui::InvisibleButton( id.c_str(), ImVec2( 1.0, 1.0 ) ) || should_advance ) {
         if( rule_map.upper_bound( rule ) == rule_map.end() ) {
             // Then we have the *last* entry the map, so wrap to the first element
             rule = rule_map.begin()->first;
@@ -100,6 +100,30 @@ void follower_rules_ui_impl::multi_rule_header( const std::string &id, T &rule,
     }
 }
 
+void follower_rules_ui_impl::setup_button( int &button_num, std::string &label,
+        bool should_color, bool &was_clicked )
+{
+    ImGui::PushID( button_num );
+    button_num++;
+    if( should_color ) {
+        ImGui::PushStyleColor( ImGuiCol_Button, c_green );
+    }
+    if( ImGui::Button( label.c_str() ) ) {
+        was_clicked = true;
+    }
+    ImGui::PopID();
+    if( should_color ) {
+        ImGui::PopStyleColor();
+    }
+}
+
+void follower_rules_ui_impl::setup_table_button( int &button_num, std::string &label,
+        bool should_color, bool &rules_to_change )
+{
+    ImGui::TableNextColumn();
+    setup_button( button_num, label, should_color, rules_to_change );
+}
+
 void follower_rules_ui_impl::set_npc_pointer_to( npc *new_guy )
 {
     guy = new_guy;
@@ -152,7 +176,6 @@ void follower_rules_ui_impl::rules_transfer_popup( bool &exporting_rules, bool &
                            guy->disp_name() ) );
     if( ImGui::BeginTable( "##SETTINGS_SWAP_TABLE", 8, ImGuiTableFlags_None,
                            ImVec2( window_width, window_height ) ) ) {
-        // Missions selection is purposefully thinner than the description, it has less to convey.
         ImGui::TableSetupColumn( _( "Name" ), ImGuiTableColumnFlags_WidthStretch,
                                  static_cast<float>( window_width / 8.0f ) );
         ImGui::TableSetupColumn( _( "Behaviors" ), ImGuiTableColumnFlags_WidthStretch,
@@ -182,126 +205,63 @@ void follower_rules_ui_impl::rules_transfer_popup( bool &exporting_rules, bool &
                 bool do_cbm_recharge = false;
                 bool do_cbm_reserve = false;
                 bool do_pickup = false;
+                bool do_all = false;
                 ImGui::TableNextRow();
                 ImGui::TableNextColumn();
                 draw_colored_text( role_model.disp_name() );
                 /*Flags and overrides*/
-                ImGui::TableNextColumn();
-                ImGui::PushID( button_number );
-                button_number++;
-                if( role_model.rules.flags == guy->rules.flags ) {
-                    ImGui::PushStyleColor( ImGuiCol_Button, c_green );
-                }
-                if( ImGui::Button( toggle_label.c_str() ) ) {
-                    do_flags_overrides = true;
-                }
-                ImGui::PopID();
-                ImGui::PopStyleColor();
+                setup_table_button( button_number, toggle_label,
+                                    role_model.rules.flags == guy->rules.flags, do_flags_overrides );
                 /*Aim rule*/
-                ImGui::TableNextColumn();
-                ImGui::PushID( button_number );
-                button_number++;
-                if( role_model.rules.aim == guy->rules.aim ) {
-                    ImGui::PushStyleColor( ImGuiCol_Button, c_green );
-                }
-                if( ImGui::Button( toggle_label.c_str() ) ) {
-                    do_aim = true;
-                }
-                ImGui::PopID();
-                ImGui::PopStyleColor();
+                setup_table_button( button_number, toggle_label,
+                                    role_model.rules.aim == guy->rules.aim, do_aim );
                 /*Engagement rule*/
-                ImGui::TableNextColumn();
-                ImGui::PushID( button_number );
-                button_number++;
-                if( role_model.rules.engagement == guy->rules.engagement ) {
-                    ImGui::PushStyleColor( ImGuiCol_Button, c_green );
-                }
-                if( ImGui::Button( toggle_label.c_str() ) ) {
-                    do_engagement = true;
-                }
-                ImGui::PopID();
-                ImGui::PopStyleColor();
+                setup_table_button( button_number, toggle_label,
+                                    role_model.rules.engagement == guy->rules.engagement, do_engagement );
                 /*CBM recharge rule*/
-                ImGui::TableNextColumn();
-                ImGui::PushID( button_number );
-                button_number++;
-                if( role_model.rules.cbm_recharge == guy->rules.cbm_recharge ) {
-                    ImGui::PushStyleColor( ImGuiCol_Button, c_green );
-                }
-                if( ImGui::Button( toggle_label.c_str() ) ) {
-                    do_cbm_recharge = true;
-                }
-                ImGui::PopID();
-                ImGui::PopStyleColor();
+                setup_table_button( button_number, toggle_label,
+                                    role_model.rules.cbm_recharge == guy->rules.cbm_recharge, do_cbm_recharge );
                 /*CBM reserve rule*/
-                ImGui::TableNextColumn();
-                ImGui::PushID( button_number );
-                button_number++;
-                if( role_model.rules.cbm_reserve == guy->rules.cbm_reserve ) {
-                    ImGui::PushStyleColor( ImGuiCol_Button, c_green );
-                }
-                if( ImGui::Button( toggle_label.c_str() ) ) {
-                    do_cbm_reserve = true;
-                }
-                ImGui::PopID();
-                ImGui::PopStyleColor();
+                setup_table_button( button_number, toggle_label,
+                                    role_model.rules.cbm_reserve == guy->rules.cbm_reserve, do_cbm_reserve );
                 /*Pickup whitelist rule*/
-                ImGui::TableNextColumn();
-                ImGui::PushID( button_number );
-                button_number++;
+
                 // This is not correct if both have rules and both have *different* rules, but comparison is expensive
-                if( role_model.rules.pickup_whitelist->empty() == guy->rules.pickup_whitelist->empty() ) {
-                    ImGui::PushStyleColor( ImGuiCol_Button, c_green );
-                }
-                if( ImGui::Button( toggle_label.c_str() ) ) {
-                    do_pickup = true;
-                }
-                ImGui::PopID();
-                ImGui::PopStyleColor();
+                // if( role_model.rules.pickup_whitelist->empty() == guy->rules.pickup_whitelist->empty() )
+                setup_table_button( button_number, toggle_label,
+                                    role_model.rules.pickup_whitelist->empty() == guy->rules.pickup_whitelist->empty(),
+                                    do_pickup );
                 /*The ALL button*/
-                ImGui::TableNextColumn();
-                ImGui::PushID( button_number );
-                button_number++;
-                if( role_model.rules.flags == guy->rules.flags &&
-                    role_model.rules.aim == guy->rules.aim &&
-                    role_model.rules.engagement == guy->rules.engagement &&
-                    role_model.rules.cbm_recharge == guy->rules.cbm_recharge &&
-                    role_model.rules.cbm_reserve == guy->rules.cbm_reserve &&
-                    role_model.rules.pickup_whitelist->empty() == guy->rules.pickup_whitelist->empty() ) {
-                    ImGui::PushStyleColor( ImGuiCol_Button, c_green );
-                }
-                if( ImGui::Button( toggle_label.c_str() ) ) {
-                    do_flags_overrides = true;
-                    do_aim = true;
-                    do_engagement = true;
-                    do_cbm_recharge = true;
-                    do_cbm_reserve = true;
-                    do_pickup = true;
-                }
-                ImGui::PopID();
-                ImGui::PopStyleColor();
+                setup_table_button( button_number, toggle_label,
+                                    role_model.rules.flags == guy->rules.flags &&
+                                    role_model.rules.aim == guy->rules.aim &&
+                                    role_model.rules.engagement == guy->rules.engagement &&
+                                    role_model.rules.cbm_recharge == guy->rules.cbm_recharge &&
+                                    role_model.rules.cbm_reserve == guy->rules.cbm_reserve &&
+                                    role_model.rules.pickup_whitelist->empty() == guy->rules.pickup_whitelist->empty(),
+                                    do_all );
                 /*The great swap*/
                 // To make this slightly easier...
                 npc *guy_with_same = exporting_rules ? guy : &role_model;
                 npc *guy_with_new = exporting_rules ? &role_model : guy;
-                if( do_flags_overrides ) {
+                if( do_flags_overrides || do_all ) {
                     guy_with_new->rules.flags = guy_with_same->rules.flags;
                     guy_with_new->rules.overrides = guy_with_same->rules.overrides;
                     guy_with_new->rules.override_enable = guy_with_same->rules.override_enable;
                 }
-                if( do_aim ) {
+                if( do_aim || do_all ) {
                     guy_with_new->rules.aim = guy_with_same->rules.aim;
                 }
-                if( do_engagement ) {
+                if( do_engagement || do_all ) {
                     guy_with_new->rules.engagement = guy_with_same->rules.engagement;
                 }
-                if( do_cbm_recharge ) {
+                if( do_cbm_recharge || do_all ) {
                     guy_with_new->rules.cbm_recharge = guy_with_same->rules.cbm_recharge;
                 }
-                if( do_cbm_reserve ) {
+                if( do_cbm_reserve || do_all ) {
                     guy_with_new->rules.cbm_reserve = guy_with_same->rules.cbm_reserve;
                 }
-                if( do_pickup ) {
+                if( do_pickup || do_all ) {
                     guy_with_new->rules.pickup_whitelist = guy_with_same->rules.pickup_whitelist;
                 }
             }
@@ -327,7 +287,7 @@ void follower_rules_ui_impl::draw_controls()
         return;
     }
 
-    ImGui::InvisibleButton( "TOP_OF_WINDOW_KB_SCROLL_SELECTABLE", ImVec2() );
+    ImGui::InvisibleButton( "TOP_OF_WINDOW_KB_SCROLL_SELECTABLE", ImVec2( 1.0, 1.0 ) );
 
     const hotkey_queue &hotkeys = hotkey_queue::alphabets();
     input_event assigned_hotkey = input_ptr->first_unassigned_hotkey( hotkeys );
@@ -363,9 +323,7 @@ void follower_rules_ui_impl::draw_controls()
     int rule_number = 0;
     /* Handle all of our regular, boolean rules */
     for( const std::pair<const std::string, ally_rule_data> &rule_data : ally_rule_strs ) {
-        ImGui::PushID( rule_number );
         assigned_hotkey = input_ptr->next_unassigned_hotkey( hotkeys, assigned_hotkey );
-        rule_number++;
         ImGui::NewLine();
         print_hotkey( assigned_hotkey );
         const ally_rule_data &this_rule = rule_data.second;
@@ -377,16 +335,17 @@ void follower_rules_ui_impl::draw_controls()
         } else {
             rules_text = string_format( "%s", get_parsed( this_rule.rule_false_text ) );
         }
-        if( rule_enabled ) {
-            ImGui::PushStyleColor( ImGuiCol_Button, c_green );
-        }
-        if( ImGui::Button( _( "Toggle" ) ) || pressed_key == assigned_hotkey ) {
+        bool was_clicked = false;
+        std::string label = _( "Toggle" );
+        setup_button( rule_number, label, rule_enabled, was_clicked );
+        if( was_clicked || pressed_key == assigned_hotkey ) {
             ImGui::SetKeyboardFocusHere( -1 );
             guy->rules.toggle_flag( this_rule.rule );
             guy->rules.toggle_specific_override_state( this_rule.rule, !rule_enabled );
         }
-        ImGui::PopStyleColor();
         ImGui::SameLine();
+        ImGui::PushID( rule_number );
+        rule_number++;
         if( ImGui::Button( _( "Default" ) ) ) {
             guy->rules.clear_flag( this_rule.rule );
             guy->rules.clear_override( this_rule.rule );
@@ -405,21 +364,16 @@ void follower_rules_ui_impl::draw_controls()
                        pressed_key == assigned_hotkey );
     int engagement_rule_number = 0;
     for( const std::pair<const combat_engagement, std::string> &engagement_rule : engagement_rules ) {
-        ImGui::PushID( rule_number );
-        rule_number++;
         engagement_rule_number++;
         // Could use a better label for these...
         std::string button_label = std::to_string( engagement_rule_number );
         ImGui::SameLine();
-        if( guy->rules.engagement == engagement_rule.first ) {
-            ImGui::PushStyleColor( ImGuiCol_Button, c_green );
-        }
-        if( ImGui::Button( button_label.c_str() ) ) {
+        bool was_clicked = false;
+        setup_button( rule_number, button_label, guy->rules.engagement == engagement_rule.first,
+                      was_clicked );
+        if( was_clicked ) {
             guy->rules.engagement = engagement_rule.first;
-            ImGui::SetKeyboardFocusHere( -1 );
         }
-        ImGui::PopStyleColor();
-        ImGui::PopID();
     }
     ImGui::SameLine();
     draw_colored_text( _( "Engagement rules" ), c_white );
@@ -434,21 +388,15 @@ void follower_rules_ui_impl::draw_controls()
     multi_rule_header( "AIMING_RULES", guy->rules.aim, aim_rule_map, pressed_key == assigned_hotkey );
     int aim_rule_number = 0;
     for( const std::pair<const aim_rule, std::string> &aiming_rule : aim_rule_map ) {
-        ImGui::PushID( rule_number );
-        rule_number++;
         aim_rule_number++;
         // Could use a better label for these...
         std::string button_label = std::to_string( aim_rule_number );
         ImGui::SameLine();
-        if( guy->rules.aim == aiming_rule.first ) {
-            ImGui::PushStyleColor( ImGuiCol_Button, c_green );
-        }
-        if( ImGui::Button( button_label.c_str() ) ) {
+        bool was_clicked = false;
+        setup_button( rule_number, button_label, guy->rules.aim == aiming_rule.first, was_clicked );
+        if( was_clicked ) {
             guy->rules.aim = aiming_rule.first;
-            ImGui::SetKeyboardFocusHere( -1 );
         }
-        ImGui::PopStyleColor();
-        ImGui::PopID();
     }
     ImGui::SameLine();
     draw_colored_text( _( "Aiming rules" ), c_white );
@@ -466,20 +414,15 @@ void follower_rules_ui_impl::draw_controls()
         multi_rule_header( "RECHARGE_RULES", guy->rules.cbm_recharge, recharge_map,
                            pressed_key == assigned_hotkey );
         for( const std::pair<const cbm_recharge_rule, std::string> &recharge_rule : recharge_map ) {
-            ImGui::PushID( rule_number );
-            rule_number++;
             int percent = static_cast<int>( recharge_rule.first );
             std::string button_label = std::to_string( percent ) + "%";
             ImGui::SameLine();
-            if( guy->rules.cbm_recharge == recharge_rule.first ) {
-                ImGui::PushStyleColor( ImGuiCol_Button, c_green );
-            }
-            if( ImGui::Button( button_label.c_str() ) ) {
+            bool was_clicked = false;
+            setup_button( rule_number, button_label, guy->rules.cbm_recharge == recharge_rule.first,
+                          was_clicked );
+            if( was_clicked ) {
                 guy->rules.cbm_recharge = recharge_rule.first;
-                ImGui::SetKeyboardFocusHere( -1 );
             }
-            ImGui::PopStyleColor();
-            ImGui::PopID();
         }
         ImGui::SameLine();
         draw_colored_text( _( "CBM recharging rules" ), c_white );
@@ -493,20 +436,15 @@ void follower_rules_ui_impl::draw_controls()
         multi_rule_header( "RESERVE_RULES", guy->rules.cbm_reserve, reserve_map,
                            pressed_key == assigned_hotkey );
         for( const std::pair<const cbm_reserve_rule, std::string> &reserve_rule : reserve_map ) {
-            ImGui::PushID( rule_number );
-            rule_number++;
             int percent = static_cast<int>( reserve_rule.first );
             std::string button_label = std::to_string( percent ) + "%";
             ImGui::SameLine();
-            if( guy->rules.cbm_reserve == reserve_rule.first ) {
-                ImGui::PushStyleColor( ImGuiCol_Button, c_green );
-            }
-            if( ImGui::Button( button_label.c_str() ) ) {
+            bool was_clicked = false;
+            setup_button( rule_number, button_label, guy->rules.cbm_reserve == reserve_rule.first,
+                          was_clicked );
+            if( was_clicked ) {
                 guy->rules.cbm_reserve = reserve_rule.first;
-                ImGui::SetKeyboardFocusHere( -1 );
             }
-            ImGui::PopStyleColor();
-            ImGui::PopID();
         }
         ImGui::SameLine();
         draw_colored_text( _( "CBM reserve rules" ), c_white );
@@ -514,5 +452,5 @@ void follower_rules_ui_impl::draw_controls()
         draw_colored_text( string_format( "%s", get_parsed( reserve_map[guy->rules.cbm_reserve] ) ) );
     }
 
-    ImGui::InvisibleButton( "BOTTOM_OF_WINDOW_KB_SCROLL_SELECTABLE", ImVec2() );
+    ImGui::InvisibleButton( "BOTTOM_OF_WINDOW_KB_SCROLL_SELECTABLE", ImVec2( 1.0, 1.0 ) );
 }
diff --git a/src/npctalk_rules.h b/src/npctalk_rules.h
index 124dafa252803..fb30771391321 100644
--- a/src/npctalk_rules.h
+++ b/src/npctalk_rules.h
@@ -45,6 +45,10 @@ class follower_rules_ui_impl : public cataimgui::window
         std::string get_parsed( std::string initial_string );
         void print_hotkey( input_event &hotkey );
         void rules_transfer_popup( bool &exporting_rules, bool &still_in_popup );
+        void setup_button( int &button_num, std::string &label,
+                           bool should_color, bool &was_clicked );
+        void setup_table_button( int &button_num, std::string &label,
+                                 bool should_color, bool &rules_to_change );
 
         size_t window_width = str_width_to_pixels( TERMX ) / 2;
         size_t window_height = str_height_to_pixels( TERMY ) / 2;