search

davidfischer / mtg-printable-set-label-generator

Magic: the Gathering Printable Set Label Generator
https://mtg-label-generator.fly.dev
MIT License
42 stars 12 forks source link

Fix problems when running on Windows #28

Closed StepKie closed 4 months ago

StepKie commented 4 months ago

Hi, there was a problem running the generator on a Windows machine, which should be fixed (without affecting other platforms)

In this context, I fixed a couple minor issues:

davidfischer commented 4 months ago

I don't really understand why there's a need to change the scoping on the DEFAULT_* variables. The scoping should be absolutely fine.

The DLL issue is silly but I suppose that's a reasonable temporary workaround.

StepKie commented 4 months ago

I don't really understand why there's a need to change the scoping on the DEFAULT_* variables. The scoping should be absolutely fine.

It is interesting that it runs for you when invoking from the command line without a paper size parameter; when I run it in this fashion (from the state of the current main branch), I get

PS D:\Repos\mtg-printable-set-label-generator> python .\mtglabels\generator.py
Traceback (most recent call last):
  File "D:\Repos\Privat\mtg-printable-set-label-generator\mtglabels\generator.py", line 374, in <module>
    main()
  File "D:\Repos\Privat\mtg-printable-set-label-generator\mtglabels\generator.py", line 369, in main
    generator = LabelGenerator(None, args.output_dir)
  File "D:\Repos\Privat\mtg-printable-set-label-generator\mtglabels\generator.py", line 156, in __init__
    self.paper_size = paper_size or DEFAULT_PAPER_SIZE
NameError: name 'DEFAULT_PAPER_SIZE' is not defined

which my IDE also displays like this:

image

Feel free to do with the PR what you like (close/modify). Of course there is no need to move them out of the class at all, but then they should be scoped as LabelGenerator.DEFAULT_OUTPUT_DIR, I think. I might be wrong, please correct me if so.

Best regards, and thanks again for this tool!

davidfischer commented 4 months ago

Thanks for this. I tested and I'm happy with these changes!