From f0f46296f24e9dd4f7826c36ec205b44ec026752 Mon Sep 17 00:00:00 2001
From: bicarus <202771338+bicarus-dev@users.noreply.github.com>
Date: Mon, 26 Jan 2026 22:12:04 -0800
Subject: [PATCH] cfg: clean up stale buttons and lights alt bindings on save
 (#534)

## Link to GitHub Issue, if one exists
n/a

## Description of change
For buttons and lights, it's possible to have multiple alternatives.

When you create alternatives and then clear then, the config files
continue to have stale entries. This is because alternatives are stored
in a vector and elements are never deleted when unbound; they get
written out to the XML on modification.

These stale XML entries live forever, and get loaded on next startup of
spice.

This PR fixes that by deleting stale entries - basically, XML nodes will
be kept only until the last valid node (properly bound button or light),
trimming the trailing invalid entries.

## Testing
Manual testing, validating XML.
---
 src/spice2x/cfg/config.cpp | 69 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 68 insertions(+), 1 deletion(-)

diff --git a/src/spice2x/cfg/config.cpp b/src/spice2x/cfg/config.cpp
index 5f4b50f..9f2d4fb 100644
--- a/src/spice2x/cfg/config.cpp
+++ b/src/spice2x/cfg/config.cpp
@@ -1,5 +1,6 @@
 #include "config.h"
 #include "util/logging.h"
+#include "cfg/button.h"
 
 /*
  * This code absolutely sucks.
@@ -510,8 +511,10 @@ bool Config::updateBinding(const Game &game, const Button &button, int alternati
         return false;
     }
 
+    tinyxml2::XMLElement *gameButtonNode = nullptr;
+
     // iterate button nodes
-    tinyxml2::XMLElement *gameButtonNode = gameButtonsNode->FirstChildElement("button");
+    gameButtonNode = gameButtonsNode->FirstChildElement("button");
     int button_count = 0;
     while (gameButtonNode != nullptr) {
         const char *buttonNodeName = gameButtonNode->Attribute("name");
@@ -574,6 +577,39 @@ bool Config::updateBinding(const Game &game, const Button &button, int alternati
         }
     }
 
+    // trim the trailing invalid (unbound) alternatives
+    tinyxml2::XMLElement *to_delete = nullptr;
+    gameButtonNode = gameButtonsNode->LastChildElement("button");
+    while (gameButtonNode != nullptr) {
+        const char *name = gameButtonNode->Attribute("name");
+        const char *devid = gameButtonNode->Attribute("devid");
+        int vkey = INVALID_VKEY;
+        gameButtonNode->QueryIntAttribute("vkey", &vkey);
+
+        // wrong button, ignore and keep iterating backwards
+        if (name == nullptr || std::string(name) != button.getName()) {
+            gameButtonNode = gameButtonNode->PreviousSiblingElement("button");
+            continue;
+        }
+
+        // found a node for this button, which means to_delete is not the first one
+        // delete the node for to_delete
+        if (to_delete != nullptr) {
+            log_misc("cfg", "deleting excessive button binding for {}", name);
+            gameButtonsNode->DeleteChild(to_delete);
+            to_delete = nullptr;
+        }
+
+        if ((devid == nullptr || strlen(devid) == 0) && vkey == INVALID_VKEY) {
+            // unbound button with no device ID, make a note of it so it can be deleted later
+            to_delete = gameButtonNode;
+            gameButtonNode = gameButtonNode->PreviousSiblingElement("button");
+        } else {
+            // valid binding found; stop iterating
+            break;
+        }
+    }
+
     // check if button was not found
     if (button_count == 0) {
         return false;
@@ -760,6 +796,37 @@ bool Config::updateBinding(const Game &game, const Light &light, int alternative
         }
     }
 
+    // trim the trailing invalid (unbound) alternatives
+    tinyxml2::XMLElement *to_delete = nullptr;
+    gameLightNode = gameLightsNode->LastChildElement("light");
+    while (gameLightNode != nullptr) {
+        const char *name = gameLightNode->Attribute("name");
+        const char *devid = gameLightNode->Attribute("devid");
+
+        // wrong light, ignore and keep iterating backwards
+        if (name == nullptr || std::string(name) != light.getName()) {
+            gameLightNode = gameLightNode->PreviousSiblingElement("light");
+            continue;
+        }
+
+        // found a node for this button, which means to_delete is not the first one
+        // delete the node for to_delete
+        if (to_delete != nullptr) {
+            log_misc("cfg", "deleting excessive light binding for {}", name);
+            gameLightsNode->DeleteChild(to_delete);
+            to_delete = nullptr;
+        }
+
+        if (devid == nullptr || strlen(devid) == 0) {
+            // unbound light, make a note of it so it can be deleted later
+            to_delete = gameLightNode;
+            gameLightNode = gameLightNode->PreviousSiblingElement("light");
+        } else {
+            // valid binding found; stop iterating
+            break;
+        }
+    }
+
     // check if light was not found
     if (light_count == 0) {
         return false;