python-sc2
python-sc2 copied to clipboard
Simplified map and path behavior and added other basic functionality
Simplify map/path usage in run_game Fix misnomer of map_settings -> map_path Refactored calls to run_game and maps.get Added Units.health_highest, Units.health_lowest Added BotAI.enemies_all and deprecated BotAI.all_enemy_units and misc cleanup Ignoring virtual environment
I don't quite understand your automated check failures looking at the code and I don't get them locally:
Run poetry run pylint $(git ls-files '*.py' | grep -E 'sc2/.*')
************* Module sc2.main
sc2/main.py:775:0: C0325: Unnecessary parens after 'if' keyword (superfluous-parens)
************* Module sc2.units
sc2/units.py:217:0: C0325: Unnecessary parens after 'return' keyword (superfluous-parens)
------------------------------------
Your code has been rated at [10](https://github.com/BurnySc2/python-sc2/runs/7002380512?check_suite_focus=true#step:9:11).00/10
vs
bash-3.2$ poetry run pylint $(git ls-files '*.py' | grep -E 'sc2/.*')
--------------------------------------------------------------------
Your code has been rated at 10.00/10 (previous run: 10.00/10, +0.00)
Regarding the FileNotFoundError: Map AcropolisLE not found
This is only happening on Unix systems minus OSX, I'm not sure why this is happening as this should mostly just be a refactor. If the map does exist in that image can you point me to the Dockerfiles so I can repo? Do the Unix systems require an SCPATH to be defined?
I can't easily identify your changes because the changes + autoformatting were done in one commit :frowning:
The primary change that's impacting these checks is the refactoring of map.py and path.py to file_map.py and file_path.py, to disambiguate the map files from the in-game map.
I also refactored the get function into the class and then renamed map_settings to map_path as it was a misnomer and made the run_game map parameter a Union[MapPath, str] to simplify user behavior.
It all seems to work pretty well on my OSX including the addition of the alternate_install_path parameter that I added to run_game.
I can't easily identify your changes because the changes + autoformatting were done in one commit 😦
Would it help if I retracted this, ran black on the structure, then just submitted a delta with Blacks's formatting suggestions? I feel like it does a significantly better job of improving the readability of the code than the current formatter.
I'm going to decline this PR for now because I am unable to review it with most of the changes being formatting changes by Black
formatter, even though I already decided on the yapf
formatter.
Also this PR seems to introduce bot-breaking changes. If that is done for a good reason (e.g. mostly performance or ease of use), I would normally allow it.
It would be better to have the functional changes in a separate reviewable commit, and the styling (formatting) changes in a different commit (or even a discussion about it beforehand would be preferable).