Update Craft Addon #2113
Update Craft Addon #2113
Conversation
What ==== - Updated slpp.py and create_recipes.py to python 3 - Ran create_recipes.py to build the latest recipe set Why? ==== There were some missing recipes and the script for generating new recipes by pulling data from bg-wiki was not working on python3.
|
Unless py2 can run that, I'd probably prefer if we also kept a py2 version so we can pick |
There was a problem hiding this comment.
I don't mind it being Python 3 only. I don't think any of the 0.7 people using this have any trouble installing Python 3, if they don't have it already. Just curious about the import and the default path. I assume this was chosen because that's the default path chosen by the new installer? Though that in itself also bothers me a bit, I'll have a talk with the team about this.
| @@ -118,10 +121,10 @@ def get_recipes_from_soup(soup, spheres=False): | |||
|
|
|||
| def get_items_dictionary(): | |||
| if platform.system() == 'Windows': | |||
| path = 'C:\\Program Files (x86)\\Windower4\\res\\items.lua' | |||
| path = 'C:/Program Files (x86)/Windower/res/items.lua' | |||
There was a problem hiding this comment.
This really bothers me, but it bothered me before your change as well. Why is this hard coded instead of using a relative path, or alternatively a path passed in via the command line (or both, with the relative path being a fallback)?
Also, backwards slashes are correcterer.
There was a problem hiding this comment.
The reason is mostly laziness. The python scripts are things that a developer or user might periodically run to update the recipes and create a pull request, so much less thought was put into these kings of details.
There was a problem hiding this comment.
Considering most people will run this script from their Windower installation I'd just make it relative. This path would break for me, for example, as well as for many people who followed the common advice we gave of where to install Windower to until recently. That falls well within the limits of laziness imo :)
Ideally we'd have another mechanism for specifying the path, but those are obviously more work and would exceed the purpose of the PR, especially since this is nothing the PR changed.
| @@ -204,10 +208,10 @@ def build_recipe_string(name, crystal, ingredients): | |||
|
|
|||
| def save_recipes(recipes): | |||
| with open('recipes.lua', 'w') as fd: | |||
There was a problem hiding this comment.
Would this (and other) open functions benefit from the encoding parameter? You added to to the file you're reading from, but I'm not sure what exactly we're writing here. Can non-ASCII characters appear, like Japanese text?
What
Why?
There were some missing recipes and the script for generating
new recipes by pulling data from bg-wiki was not working on python3.