Tuning: Fixed false positives in addonpart conflict check.

Cause: bad loop code in AddonPartFileFormat.cpp, function `RecordAddonpartConflicts()`, which inserted garbage into `_tweaks` arrays. For example if the addonpart only tweaked prop 6, there should be just the item 6 in the set, but it ended up being garbage items 0-5 and valid 6. Why? This is a backwards compatibility quirk of C++: when you ask a `std::set<>` for non-existent element via `[]` brackets, it inserts a dummy element and gives it to you. And because these loops went "from 0 to number of elements", gradually, as repeated conflict resolutions happened, garbage built up. Fixed by looping using iterators which only retrieve actually existing elements.
RigsOfRods · May 28, 2024 · 47f022e · 47f022e
1 parent 7b25b29
commit 47f022e
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 9 deletions.
diff --git a/source/main/gui/panels/GUI_TopMenubar.cpp b/source/main/gui/panels/GUI_TopMenubar.cpp
@@ -2475,7 +2475,10 @@ void TopMenubar::RefreshTuningMenu()
         {
             for (size_t i2 = i1; i2 < tuning_addonparts.size(); i2++)
             {
-                AddonPartUtility::RecordAddonpartConflicts(tuning_addonparts[i1], tuning_addonparts[i2], tuning_conflicts);
+                if (i1 != i2)
+                {
+                    AddonPartUtility::RecordAddonpartConflicts(tuning_addonparts[i1], tuning_addonparts[i2], tuning_conflicts);
+                }
             }
         }
 

diff --git a/source/main/resources/addonpart_fileformat/AddonPartFileFormat.cpp b/source/main/resources/addonpart_fileformat/AddonPartFileFormat.cpp
@@ -793,9 +793,9 @@ void AddonPartUtility::RecordAddonpartConflicts(CacheEntryPtr addonpart1, CacheE
     }
 
     // NODE TWEAKS:
-    for (size_t i = 0; i < addonpart1->addonpart_data_only->node_tweaks.size(); i++)
+    for (auto& i_pair: addonpart1->addonpart_data_only->node_tweaks)
     {
-        NodeNum_t suspect = addonpart1->addonpart_data_only->node_tweaks[i].tnt_nodenum;
+        NodeNum_t suspect = i_pair.second.tnt_nodenum;
         TuneupNodeTweak* offender = nullptr;
         if (TuneupUtil::isNodeTweaked(addonpart2->addonpart_data_only, suspect, offender))
         {
@@ -805,9 +805,9 @@ void AddonPartUtility::RecordAddonpartConflicts(CacheEntryPtr addonpart1, CacheE
     }
 
     // WHEEL TWEAKS:
-    for (size_t i = 0; i < addonpart1->addonpart_data_only->wheel_tweaks.size(); i++)
+    for (auto& i_pair: addonpart1->addonpart_data_only->wheel_tweaks)
     {
-        WheelID_t suspect = addonpart1->addonpart_data_only->wheel_tweaks[i].twt_wheel_id;
+        WheelID_t suspect = i_pair.second.twt_wheel_id;
         TuneupWheelTweak* offender = nullptr;
         if (TuneupUtil::isWheelTweaked(addonpart2->addonpart_data_only, suspect, offender))
         {
@@ -817,9 +817,9 @@ void AddonPartUtility::RecordAddonpartConflicts(CacheEntryPtr addonpart1, CacheE
     }
 
     // PROP TWEAKS:
-    for (size_t i = 0; i < addonpart1->addonpart_data_only->prop_tweaks.size(); i++)
+    for (auto& i_pair:addonpart1->addonpart_data_only->prop_tweaks)
     {
-        PropID_t suspect = addonpart1->addonpart_data_only->prop_tweaks[i].tpt_prop_id;
+        PropID_t suspect = i_pair.second.tpt_prop_id;
         TuneupPropTweak* offender = nullptr;
         if (TuneupUtil::isPropTweaked(addonpart2->addonpart_data_only, suspect, offender))
         {
@@ -829,9 +829,9 @@ void AddonPartUtility::RecordAddonpartConflicts(CacheEntryPtr addonpart1, CacheE
     }
 
     // FLEXBODY TWEAKS:
-    for (size_t i = 0; i < addonpart1->addonpart_data_only->flexbody_tweaks.size(); i++)
+    for (auto& i_pair: addonpart1->addonpart_data_only->flexbody_tweaks)
     {
-        FlexbodyID_t suspect = addonpart1->addonpart_data_only->flexbody_tweaks[i].tft_flexbody_id;
+        FlexbodyID_t suspect = i_pair.second.tft_flexbody_id;
         TuneupFlexbodyTweak* offender = nullptr;
         if (TuneupUtil::isFlexbodyTweaked(addonpart2->addonpart_data_only, suspect, offender))
         {