Wrye-bash: wxPython 4 Upgrade

[Infernio]

Continuing on from #15, we will of course want to upgrade to wxPython 4 as well. Mostly intended as a tracking and discussion issue, to refer to it in commits, etc.

Roadblock on the way to #460 since wx3 does not have py3 support but wx4 does, hence the TODO label.

190 is a prerequisite for this.

[Infernio]

Literally just opened this so I had a convenient place to post this: I just noticed that wx.NewId() is marked as deprecated in wx4 (also mentioned in the migration guide). So we will have to revisit its usages and decide what to do - for Links using wx.ID_ANY, as recommended in the guide, should be possible, but I can't even pretend to understand what balt.ListCtrl is doing with it 😅

[Infernio]

Made a separate branch for wx4, 488-wx4-upgrade, because I got tired of having to autocomplete three times to get to lojack-py3-wx-dev :P

[Utumno]

Haha

wx.NewId() is marked as deprecated

Yep I am aware - balt.ListCtrl is my next target for wrapping - and I know you want to edit it in the future so let's keep it in its own file, given how central it is. The Links should also probably be turned into a _AComponent (they use this wxNewId too) but these (as UIList) are pretty stable so I would go very carefully (and after pt1 is merged) -> well actually have a look at efb83aba64fb37cfdd974e092cc7c5540bd4b430

Probably wx-4 should be officially squashed to one commit? No use to keep separate.

Next target would be the balt.Picture thing I guess...

[Infernio]

The Links should also probably be turned into a _AComponent (they use this wxNewId too) but these (as UIList) are pretty stable so I would go very carefully (and after pt1 is merged) -> well actually have a look at efb83ab

I agree, we should have a look at all this after pt1 is merged. No need to rush this, it works for now :)

Probably wx-4 should be officially squashed to one commit? No use to keep separate.

Yup, we'll want to do a branch merge for the actual upgrade (with the doc, pipenv and installer edits included), but the code changes need to be a single commit or they break dev.

Edit: one more thing, we'll want to move links to gui eventually, so can we rename basher/links.py to, say, init_links.py, so that we can use links.py for gui?

[Infernio]

Went ahead and squashed the branch down to one commit (4681f9ad74275e528c7bdac9e2eeae79a7d2596f), I don't actually think we can really separate any part of it - otherwise something breaks on dev. I guess we could do the docs as a separate commit, but a branch merge for two commits seems silly :P

[Utumno]

Exactly my thoughts thanks (see below) - I rebased yet again addressing the commit comments (background color and exists()) - nightly too.

I had couple questions on --squash merge -are you using something like:

git merge --squash 190-de-wx-pt1 &&  git commit -eF .git/SQUASH_MSG

Also, as 488-wx4-upgrade was on top 190-de-wx-pt, it required a rebase on squashed 190-de-wx-pt1 for me to do the merge --squash - or am I missing something?

I am really glad it is just a commit now as you can see

[Infernio]

I used git merge --squash <branch>, followed by git commit and manually editing the first line to read Squashed version of <branch> :sweat_smile:

[Utumno]

I'd say we bite the bullet and merge - except if this locks us to x64?

Edit: oh I see we need to decide on pipenv (or not, I mean just edit requirements.txt on dev?)

Dropping the python installer should probably merged before this one - I always wanted to make it easy for people to install the python version but yeah let's revisit with nsis

[Infernio]

It doesn't - and we may want to hold off on 64bit for beta6. One or two people on Discord report that some of the bundled python extensions fail to load for them in standalone builds - but if I build the exact same commit with the 64bit changes reverted, it works fine for them...

[Utumno]

Haha ninj'ed - see my edits. then I'd say dev. Anyway we won't stay with wx3 now that we managed to leave wx2.

Plus much depends on it

[Infernio]

Just tried to merge it, the wx4 commit applies easily enough (plus editing to use requirements.txt instead of Pipfile), but the installer commit won't work without inf-431-loot-parser applied first, since that drops the MSVC part of the installer, which that commit relies on :/

[Utumno]

Well apply it after then no need to get very picky now - the merges are tricky enough on their own

[Infernio]

Whelp, was going to update us to wxPython 4.1.0 because it has lots of neat new things (e.g. would allow dropping our ugly solution for #458), got hit with this upon launching: https://github.com/wxWidgets/Phoenix/issues/1612
Thank god we de-wx'd so much already, otherwise this would have been a nightmare...

[Infernio]

Alright, I got it working. Some appearance changes - most notably wx.ListCtrl, which now uses Windows' native look for it, and buttons which seem to have gotten smaller.
Only real problem I noticed is that the icons in our status bar are trying to escape again, will have to look into moving them up a bit somehow.

Edit: on FANTASTIC news though, the progress bug seems to be gone 👀
Edit 2: or maybe not? Seems to have disappeared for Details, but Mark Mergeable now causes it instead (didn't do that before).

[Infernio]

I think we should stay on 4.0.7 for now - 4.1.0 seems quite buggy (e.g. wx.ProgressDialog is very choppy, now shows the remaining time by default, and if I click the button to hide the remaining time it's gone for good plus takes the Cancel button with it 🤔).

Edit: Pushed as xxx-inf-wx4.1.0, launches and works but again, 4.1.0 seems too buggy to use so far.