16
\$\begingroup\$

This script is a tool to help minecraft mod development. When you want to add a new block to the game you have to create three json files. This script generates three JSON files in specific locations. It's intended to be run in a certain directory so that the files go into the right location. Also it's for my own personal use.

The reason I didn't use the json module is simply because I didn't feel like. If it would make this script much better then I am all for switching to that although I did enjoy writing my json methods and found it educational which is another reason I'm making this, to learn a bit of python.

It's quite long and I think the functions could be taken out into a new file but then I'll need to make sure I have both files just to run this which I don't like.

I'm not concerned about efficiency or speed, just readability and maintainability, and general best practice.

import os
# Declare Functions
def jsonStart():
 return "{\n"
def jsonEnd(indent): 
 return "\n" + jsonIndent(indent) + "}"
def jsonIndent(amount):
 amount = amount * 4
 return " " * amount
def jsonKeyValue(key, value, indent = 0):
 return jsonIndent(indent) + jsonKey(key) + jsonValue(value)
def jsonKey(key, indent = 0):
 return jsonIndent(indent) + "\"" + key + "\"" + ": "
def jsonValue(value):
 return "\"" + value + "\""
def deleteCreatedFiles():
 print("\nSomething went wrong")
 for file in createdFiles:
 print("Deleting: " + file)
 os.remove(file)
 print("\n")
def createFile(filename, data):
 try:
 os.makedirs(os.path.dirname(filename), exist_ok=True)
 with open(filename, "w+")as newFile:
 newFile.write(data)
 except:
 deleteCreatedFiles()
 raise
 else:
 createdFiles.append(os.path.relpath(newFile.name))
 print("Created" + os.path.relpath(newFile.name))
def blockStatesFile():
 data = jsonStart()
 data += jsonKey("variants", 1) + jsonStart()
 data += jsonKey("normal", 2) + "{ " + jsonKeyValue("model", modid + ":" + blockName) + "}"
 data += jsonEnd(1)
 data += jsonEnd(0)
 return data
def modelsItemFile():
 data = jsonStart()
 data += jsonKeyValue("parent", modid + ":block/" + blockName, 1) + ",\n"
 data += jsonKey("textures", 1) + jsonStart()
 data += jsonKeyValue("layer0", modid + ":items/" + blockName, 2)
 data += jsonEnd(1)
 data += jsonEnd(0)
 return data
def modelsBlockFile():
 data = jsonStart()
 data += jsonKeyValue("parent", "block/cube_all", 1) + ",\n"
 data += jsonKey("textures", 1) + jsonStart()
 data += jsonKeyValue("all", modid + ":blocks/" + blockName, 2)
 data += jsonEnd(1)
 data += jsonEnd(0)
 return data
# Run Script
createdFiles = []
blockName = input("block name: ")
modid = input("modid: ")
createFile("blockstates/" + blockName + ".json", blockStatesFile())
createFile("models/item/" + blockName + ".json", modelsItemFile())
createFile("models/block/" + blockName + ".json", modelsBlockFile())
200_success
146k22 gold badges190 silver badges479 bronze badges
asked Dec 27, 2018 at 10:40
\$\endgroup\$
0

2 Answers 2

13
\$\begingroup\$

I won't repeat @victor's answer, but Python is a language that comes with batteries included; meaning that a lot of behaviour has already been bundled into modules, and is maintained for correctness and performance. You should really avoid to reinvent the wheel if it is not for learning purposes.

Python also comes with an official style guide: PEP8, which is advised to follow if you want your code to look like Python code to others.

Your code also rely heavily on variables defined globally. This kind of code is error prone and less reusable. Instead, define arguments for your functions and pass information as parameters.

Lastly, you should avoid keeping code at the top-level of the file, protect it with an if __name__ == '__main__' guard:

import os
import json
def delete_files(files):
 for filename in files:
 os.remove(filename)
def create_file(filename, data, created_files):
 try:
 os.makedirs(os.path.dirname(filename), exist_ok=True)
 with open(filename, "w+") as new_file:
 json.dump(data, new_file, indent=4)
 except:
 print("\nSomething went wrong")
 print("Deleting:", *created_files)
 print("\n")
 delete_files(created_files)
 raise
 else:
 filepath = os.path.relpath(new_file.name)
 created_files.append(filepath)
 print("Created", filepath)
def block_states(modid, block_name):
 return {
 'variants': {
 'normal': {
 'model': f'{modid}:{block_name}',
 },
 },
 }
def models_item(modid, block_name):
 return {
 'parent': f'{modid}:block/{block_name}',
 'textures': {
 'layer0': f'{modid}:items/{block_name}',
 },
 }
def models_block(modid, block_name):
 return {
 'parent': 'block/cube_all',
 'textures': {
 'all': f'{modid}:blocks/{block_name}',
 },
 }
def main(modid, block_name): 
 created_files = []
 create_file(
 f'blockstates/{block_name}.json',
 block_states(modid, block_name),
 created_files)
 create_file(
 f'models/item/{block_name}.json',
 models_item(modid, block_name),
 created_files)
 create_file(
 f'models/block/{block_name}.json',
 models_block(modid, block_name),
 created_files)
if __name__ == '__main__':
 block_name = input("block name: ")
 modid = input("modid: ")
 main(modid, block_name)
answered Dec 27, 2018 at 12:59
\$\endgroup\$
1
  • \$\begingroup\$ Changed this to the accepted answer as it covers more than just using the json library and has shown me the best practice for using if name == 'main': and the official style guide updates \$\endgroup\$ Commented Dec 27, 2018 at 13:50
10
\$\begingroup\$

If all you need is just readability and maintainability, and general best practice, don't reinvent the wheel. You have the json library and it would be a sin not to use it. Firstly, you are greatly complicated the readability and extensibility. Secondly, JSON is not so simple (at least, you need to escape some special characters).

I rewrote your code and you have the opportunity to compare them:

import os, json
# Declare Functions
def deleteCreatedFiles():
 print("\nSomething went wrong")
 for file in createdFiles:
 print("Deleting: " + file)
 os.remove(file)
 print("\n")
def createFile(filename, data):
 try:
 os.makedirs(os.path.dirname(filename), exist_ok=True)
 with open(filename, "w+") as newFile:
 jstr = json.dumps(data, indent=4)
 newFile.write(jstr)
 except:
 deleteCreatedFiles()
 raise
 else:
 createdFiles.append(os.path.relpath(newFile.name))
 print("Created" + os.path.relpath(newFile.name))
def blockStatesFile():
 return {
 'variants': {
 'normal': {
 'model': modid + ":" + blockName
 }
 }
 }
def modelsItemFile():
 return {
 'parent': modid + ":block/" + blockName,
 'textures': {
 'layer0': modid + ":items/" + blockName
 }
 }
def modelsBlockFile():
 return {
 'parent': 'block/cube_all',
 'textures': {
 'all': modid + ":blocks/" + blockName
 }
 }
# Run Script
createdFiles = []
blockName = input("block name: ")
modid = input("modid: ")
createFile("blockstates/" + blockName + ".json", blockStatesFile())
createFile("models/item/" + blockName + ".json", modelsItemFile())
createFile("models/block/" + blockName + ".json", modelsBlockFile())
answered Dec 27, 2018 at 11:52
\$\endgroup\$
3
  • \$\begingroup\$ This is so much better! I'm surprised to see the functions that return the JSON actually work like that, that is really nice! \$\endgroup\$ Commented Dec 27, 2018 at 12:01
  • 2
    \$\begingroup\$ With Python 3.6 and f-strings: f"{modid}:{blockName}", f"blockstates/{blockName}.json", ... are a bit shorter and more readable IMO. \$\endgroup\$ Commented Dec 27, 2018 at 12:05
  • \$\begingroup\$ @Graipher I wondered if this sort of syntax was available in Python I am now using it in my script! \$\endgroup\$ Commented Dec 27, 2018 at 12:34

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.