labbie icon indicating copy to clipboard operation
labbie copied to clipboard

Linux support

Open Xandaros opened this issue 3 years ago • 10 comments

Hello there!

In the README, it currently states:

Labbie was written to use on the Windows operating system, so YMMV if you choose to try with other operating systems

Now, as far as I can tell, labbie will not run on a non-Windows OS as it is, since it uses ctypes.windll and the Windows-only version of mmap.mmap.

I could get it to work for myself fairly easily, though, so I was wondering if you'd be interested in changing it to support Linux at all. I can, of course, submit a PR, since I will be working on this for myself either way, but if you want to support it, I will also try to preserve Windows support ;) If it's just for myself, I'll just hack it together.

If so, I do have some questions, though:

  • ctypes.windll.shell32.SetCurrentProcessExplicitAppUserModelID(myappid) (ui/utils.py) This is the only place where ctypes.windll is used. I just commented it out and it seems to work fine - what does this even do? I suspect just guarding this so it only executes on Windows would be sufficient here.
  • mmap.mmap(-1, 1, 'labbie_instances') (utils.py) The third argument here is Windows-only. Removing this seems to work fine. I think, but have not confirmed, that always using the same tag is the same as no tag. In that case, simply removing this kw argument altogether would work just fine. However, if there is a reason why this needs to stay, the solution for Linux will probably end up a lot more complicated...
  • font.setPixelSize(1.5 * self._thumb_radius) (ui/switch.py) It did not like this being a float, so I just cast it to an integer. I doubt the extra precision is needed, is it?
  • pytesseract.pytesseract.tesseract_cmd = str(utils.bin_dir() / 'tesseract' / 'tesseract.exe') (ocr.py) Bundling tesseract for Linux would be silly, especially since this would most likely still only be unofficial support. Expecting tesseract to be on the PATH is probably the most sensible option here, and it stays bundled for Windows. What do you think?

And finally, it just crashed when I clicked the "All" button, with:

Traceback (most recent call last):
  File "/home/xandaros/workspace/labbie/src/labbie/ui/search/widget/presenter.py", line 180, in on_all
    self.populate_view(bases_result)
  File "/home/xandaros/workspace/labbie/src/labbie/ui/search/widget/presenter.py", line 89, in populate_view
    result_presenter.populate_view(result)
  File "/home/xandaros/workspace/labbie/src/labbie/ui/result/widget/presenter.py", line 169, in populate_view
    league_results = self._build_enchant_search_display_results(result.league_result)
  File "/home/xandaros/workspace/labbie/src/labbie/ui/result/widget/presenter.py", line 456, in _build_enchant_search_display_results
    krangled_base = self._bases.helms[base].base if unique else base
KeyError: 'Sudden Dawn'

I'm... not sure this is actually a Linux problem. For testing, I wrapped it in a try/except and just set krangled_base = base on error. No idea if that is anywhere close to correct, but it appeared to work at first glance.

As I mentioned, I got it mostly working for myself. Hotkeys don't work without root, so that needs fixing, but everything else appears to work. I did test some screen captures, as well, though only from screenshots. If there is interest in making it run on other operating systems, even if there won't be official support, I'll make a proper PR.

Xandaros avatar Feb 12 '22 10:02 Xandaros

This sounds great to me.

I'll answer your questions in more detail tomorrow. I'll also push a fix for the "All" crash, it's caused by new uniques. I actually just encountered it and fixed it today because I'm finally getting to run lab after a krangled league start.

bnorick avatar Feb 12 '22 11:02 bnorick

  • ctypes.windll.shell32.SetCurrentProcessExplicitAppUserModelID(myappid) (ui/utils.py) Guarding for just windows should be fine. I'd probably do it in __main__.py where the call to fix_taskbar_icon is, as this should be Windows specific.
  • mmap.mmap(-1, 1, 'labbie_instances') (utils.py) This is actually being changed to use multiprocessing.shared_memory in 0.7.0. I think that should be Linux compatible, maybe we should hold off until I can get that release finished?
  • font.setPixelSize(1.5 * self._thumb_radius) (ui/switch.py) Seems like int is fine to me!
  • pytesseract.pytesseract.tesseract_cmd = str(utils.bin_dir() / 'tesseract' / 'tesseract.exe') (ocr.py) Yep, I think your suggestion makes the most sense. Let's add a note to the installation instructions for Linux.

bnorick avatar Feb 13 '22 07:02 bnorick

Alright, sounds good. I'll just work on the 0.7.0 branch, then.

Though, I can already report a slight annoyance: The 0.7.0 branch requires Python 3.8, while the current master does work on Python 3.10 (with appropriate modification to pyproject.toml)

Xandaros avatar Feb 13 '22 09:02 Xandaros

I unfortunately haven't even pushed the latest 0.7.0 changes, so if you do a PR now we can just use master and I'll get them merged for 0.7.0 when it's ready.

bnorick avatar Feb 13 '22 09:02 bnorick

Hmm, I would have assumed merging would be easier if I worked off 0.7.0. But the changes are really small anyway. This is all that was needed to get it to run: https://github.com/Xandaros/labbie/commit/72e6d16ffd5332b02498ac06792cb01cd48458fe

(Referring to the problems outlined in the commit message:) The tiling WM thing is already fixed, I just always add the tool hint on Linux. (Though I should probably test that with a floating WM too, shouldn't I...) Hotkeys will be difficult, but hey - at least it works by clicking the button. And the crash... is a segfault. Don't think I can do much about that, probably a problem with Qt itself. Might just make it a requirement to click the set button instead, that works fine.

Either way, I'll fix the rest tomorrow. If you want it to be on master, it shouldn't be too hard for me to move the changes over.

Xandaros avatar Feb 13 '22 09:02 Xandaros

Regarding the "All" crash, until I have a minute to push a bugfix, you can edit resources.py to bump the version numbers to 2 on lines 145-147.

bnorick avatar Feb 14 '22 09:02 bnorick

Hey there I am trying to do the same thing like xandaros but on mac :) Im able to start but the main window does not open :( But I can open settings and about by the TrayIcon. Maybe any idea how to solve that issue ?

DevModeVM avatar May 20 '22 15:05 DevModeVM

Okay I got it working If you need the fix for future mac support: I had to remove QtCore.Qt.SubWindow in -> seach/window/view.py :D

DevModeVM avatar May 20 '22 15:05 DevModeVM

Can you be more specific? Thanks!

On Fri, May 20, 2022, 8:38 AM DevModeVM @.***> wrote:

Okay I got it working If you need the fix for future mac support: I had to remove QtCore.Qt.SubWindow in -> seach/window/view.py :D

— Reply to this email directly, view it on GitHub https://github.com/bnorick/labbie/issues/22#issuecomment-1133047091, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACN5LIVT2P7OMQLF7IQM5LVK6WWLANCNFSM5OGRKWLA . You are receiving this because you commented.Message ID: @.***>

bnorick avatar May 20 '22 16:05 bnorick

Never mind!

On Fri, May 20, 2022, 9:09 AM Brandon Norick @.***> wrote:

Can you be more specific? Thanks!

On Fri, May 20, 2022, 8:38 AM DevModeVM @.***> wrote:

Okay I got it working If you need the fix for future mac support: I had to remove QtCore.Qt.SubWindow in -> seach/window/view.py :D

— Reply to this email directly, view it on GitHub https://github.com/bnorick/labbie/issues/22#issuecomment-1133047091, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACN5LIVT2P7OMQLF7IQM5LVK6WWLANCNFSM5OGRKWLA . You are receiving this because you commented.Message ID: @.***>

bnorick avatar May 20 '22 16:05 bnorick