DavidoTek / ProtonUp-Qt

Install and manage GE-Proton, Luxtorpeda & more for Steam and Wine-GE & more for Lutris with this graphical user interface.
https://davidotek.github.io/protonup-qt
GNU General Public License v3.0
1.16k stars 39 forks source link

pupgui2: Put Global Tool at top of list #326

Closed sonic2kk closed 7 months ago

sonic2kk commented 7 months ago

Another part for #254, and hopefully a quick win.

This PR puts the global compatibility tool at the top of the tools list. We do this in the same condition here we mark the tool as global. I found a one-liner on StackOverflow that shows how to do this. Since we're looping through the compat_tool_index_map list already, we have a reference to the ct object. So we pop it from the list and since pop returns the element, we can pass this to insert and put it at index 0. Maybe not the most readable one-liner but I think it's fine :-)

image

This doesn't impact interacting with the tool the ctinfo dialog still opens as expected (we can make the change to hide the Games List after #319):

image

(The games list is displaying this way because I edited my config.vdf temporarily, with Steam closed, to make this Proton version the global one. It isn't actually my global compatibility tool. This screenshot is just to illustrate that there should be no impact to moving the ctool to the top of the list :slightly_smiling_face: )

This does mean we can still remove the tool as well, but that change can be made in a separate PR.


Thanks!

DavidoTek commented 7 months ago

Thanks!

So we pop it from the list and since pop returns the element, we can pass this to insert and put it at index 0. Maybe not the most readable one-liner but I think it's fine :-)

I actually wasn't expecting this to work. I think in some cases at least, you cannot modify the array you are iterating over. I tested it with an older Python version (3.7) just in case. Also works fine there. (It reminds me that I should look into upgrading the AppImage Python version before the end of next year)

sonic2kk commented 7 months ago

I also remember a few issues like that but I also figured it would be fine, since by the time we get to the element we're modifying, we'll move past it, so the rest of the loop should be unaffected. We won't hit the element we're modifying. If the list has only one item, well this will do nothing anyway :-)

If we do ever run into trouble we could just clone the list, modify that in the loop, and then overwrite the original list with the updated list after the loop. Much less elegant but one possible fix if we get trouble from this.

Hope that makes sense but I did have similar concerns, I also remembered some weirdness about mutating an array mid-loop but I think that might've been in the Python 2.7 days.