fix: skip the human option on non-human player slots

[akiraravencross]

I made a small optimization but I'd like to see it being considered as an issue even if the pull request doesn't get approved cause it was getting on my nerves. So, when you try to create a custom scenario, evidently you cannot choose the "human" option on any other slot other than yours, yet when you attempt to, instead of the game just omitting entirely the option to put "human" on the slot for some reason it switches the position of your (the main player's) slot with the one you're editing. I don't know if it is a deliberate choice but in any case i found this switcheroo annoying when trying to host a game and I guess other players may have found it annoying too. I made this small change to fix it.

[andy5995]

Thanks @akiraravencross , this works very nicely. Could you make a helper function so the code isn't duplicated in these two places?

[andy5995]

I'm not too good at reviewing code. I asked Claude about it, and his suggestion makes sense to me. The string definitely can't be hard-coded. "Human" is a translated string. You can see here: https://github.com/MegaGlest/megaglest-data/tree/develop/data/lang

Fragile string comparison:
if (item != lang.getString("Human"))
Filtering a control-type list by comparing against a localized display string is fragile — it breaks if the language key is missing,
renamed, or whitespace differs. Better to compare against the ControlType enum value used to build controlItems in the first place, rather
than the translated label.

It suggested this change:

  +        if (i == 0) {
  +            listBoxControls[i].setItems(controlItems);
               } else {
  -                vector<string> controlItemsNoHuman;
  -                for (const string& item : controlItems) {
  -                    if (item != lang.getString("Human")) {
  -                        controlItemsNoHuman.push_back(item);
  -                    }
  -                }
  -                listBoxControls[i].setItems(controlItemsNoHuman);
  +            vector<string> controlItemsNoHuman = controlItems;
  +            controlItemsNoHuman.erase(controlItemsNoHuman.begin() + ctHuman);
  +            listBoxControls[i].setItems(controlItemsNoHuman);
               }

[akiraravencross]

yes, you're right I forgot to account for all other translations lol. And yes, the fix Claude suggested fixes this perfectly. I adjusted it and made it a helper function as requested in the newest commit.

[andy5995]

Let's name the function setControlItemsForSlot instead.

One problem I've found with this patch is that now the person who started or who is hosting can't change to a different slot. Other network players still can change their own slots, which is good.

Here I'm connected in slot 2, I can move to slot 3 by clicking the right arrow, on the right:

If we can add that arrow to the host's menu as well, that should be all that's needed.