letsgamedev / Suffragium

A game developed in a democratic cycle.
GNU Affero General Public License v3.0
51 stars 21 forks source link

Tool Python - Run GDScript Toolkit #54

Closed Numenter closed 2 years ago

Numenter commented 2 years ago

Description

Added a python script for simpler use of the GDScript Toolkit. #52

Type of change

Output

image

BjoernAkAManf commented 2 years ago

Just running subprocesses also seems hacky. Still questioning the usefulness, but at least it is cross platform.

Joshix-1 commented 2 years ago

I would do it something like the following: https://github.com/Joshix-1/Suffragium/commit/3d34685d0178afb7b4596dc958cb1eb6d6bab8d3 (That is linted with pylint, formatted with black)

Joshix-1 commented 2 years ago

Found few issues https://github.com/Joshix-1/Suffragium/commit/cb8d46cf01118345f8b8e0a0b4ad834f6cd719ac The game_path didn't work on linux

And I marked it as executable with chmod +x

Numenter commented 2 years ago

game_path was in windows style. I use normpath to fix it, dont know if this works. (I don't use linux)

Joshix-1 commented 2 years ago

That doesn't work, my solution worked, and should also work on windows With your solution I don't get any output from the lint that tells me it didn't worked

Also please mark the file executable

Joshix-1 commented 2 years ago

the mode of the file didn't change Also printing the error code doesn't make any sense if it is a number returned from subprocess.run And it will get printed twice if it is a string. One time with sys.exit and the other time with the print

Numenter commented 2 years ago

I change the chmod with "git update-index --chmod=+x run_gdtoolkit.py" and "git ls-files -s run_gdtoolkit.py" shows that it has 755, but when i commit its back to 644.

Joshix-1 commented 2 years ago

chmod +x: https://github.com/Joshix-1/Suffragium/commit/417a461a03b0f05e878fbd0345bc2a28bd5def5b

Numenter commented 2 years ago

I thing I got it.

chmod +x: Joshix-1@417a461

Thangs for that, but I need to get it myself :P

Joshix-1 commented 2 years ago

I would format it with black (which makes pylint happy too): https://github.com/Joshix-1/Suffragium/commit/865366cf3db5ed68a2fd016b2b4d69c4de87f83b

We could also use this script in the lint workflow

Numenter commented 2 years ago

So, we're gonna comply with PEP 8 (Tabs vs Spaces).

Numenter commented 2 years ago

Just running subprocesses also seems hacky.

How not to use subprocess, enlighten me. @BjoernAkAManf Best I can to is get paths to the scripts and now?

import gdtoolkit
gdtoolkit_path = os.path.dirname(gdtoolkit.__file__)
gdformat_path = os.path.join(gdtoolkit_path, "formatter")
gdlint_path = os.path.join(gdtoolkit_path, "linter")

Import gdformat and import gdlint are not working for me.

BjoernAkAManf commented 2 years ago

Cant look myself right now, but using import for path resolution is better than relying on shell / manual path parsing.

Id have a look at their respective main files though. Not that good with python though.

Joshix-1 commented 2 years ago

gdlint uses from gdtoolkit.linter.__main__ import main and gdformat uses from gdtoolkit.formatter.__main__ import main These both don't look like they are stable API. Both rely on sys.argv and the first even calls sys.exit. It would be really hacky to use those functions I think just calling the commands (like we do now) is the best solution.

Joshix-1 commented 2 years ago

With the current solution there are a few problems:

This commit fixes all of that: https://github.com/Joshix-1/Suffragium/commit/bf755487c797ec6a237ab5393e08de03e1f3ec7f