-
Notifications
You must be signed in to change notification settings - Fork 46
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
importDecompme #134
base: main
Are you sure you want to change the base?
importDecompme #134
Conversation
What needs to be done to get this to a mergeable state? I've been finding this really useful and would be happy to help out. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haven't tested this yet, but in the interest of keeping things moving, I perused this and left a couple comments. Hopefully we can work to get it merged :)
@@ -0,0 +1,8 @@ | |||
# Decompme Mappings | |||
|
|||
[decompme_compiler_identifier] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this settings section could/should just go into permuter_settings.toml unless you think there's a good reason to keep it separate
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
permuter_settings.toml only exists in repos, but we don't have a repo at the point we do ./import.py https://decomp.me/...
.
def main() -> None: | ||
|
||
if len(sys.argv) > 1 and sys.argv[1].startswith(DECOMPME_BASE): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm assuming there's a way to do this that's more argparse-y instead of manually checking argv - that might be better here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not really afaik, we need to do different parsing depending on whether something that looks like a URL is passed
I created a branch https://github.com/simonlindholm/decomp-permuter/tree/importDecompme which is an updated version of this PR (though it's untested). It lost some steam once I realized it wasn't clear how to handle macro preservation -- normally the permuter can get information about that from permuter_settings.toml but decomp.me doesn't have that. |
Reopening this as WIP PR,
I know updates still need to be made to support different compiler types / setups. Right now it works for mwcc though.