Skip to content

Conversation

@DexrnZacAttack
Copy link
Collaborator

I made this script a long time ago, it's usable but definitely needs improvement.

@Bagietas
Copy link
Collaborator

This is up to @GRAnimated if that should be merged.

# my least favorite language

# https://www.geeksforgeeks.org/python/command-line-arguments-in-python/#using-argparse-module
parser = argparse.ArgumentParser(description = "Allows you to mangle symbols with our modified toolchain")
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move argument parsing into a main function and call it in an if __name__ == "__main__".

os.makedirs(d, exist_ok=True)


with open(f, 'w') as t:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move each step of the process into its own function

t.write("\n__attribute__((used))\n")
t.write(f"DEFAULT_TYPE {args.gvar} = {dv};")

c = subprocess.run([os.path.join(sd, "../toolchain/clang-4.0.1/bin/clang++"), f"-I{os.path.join(sd, '../src')}", f"-I{os.path.join(sd, '../src/Minecraft.World')}", f"-I{os.path.join(sd, '../src/4JLibraries_Source')}", f"-I{os.path.join(sd, '../src/Minecraft.Client')}", f"-I{os.path.join(sd, '../src/PlatformLibraries_Source')}", f"-I{os.path.join(sd, '../lib/nnheaders/include')}", "-stdlib=libc++", "-O3", "-g2", "-std=c++1z", "-fno-rtti", "-fno-exceptions", "-fno-strict-aliasing", "-c", "-Wno-return-type", f, "-o", o],
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please write some small infrastructure around testing this script that can be used in a --test flag. Ensure to include test cases that pass and some that fail, so we have a tangible goal for improving this script later on.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll look at doing this when available

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants