-
Notifications
You must be signed in to change notification settings - Fork 450
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
Remove THIS macro #1756
base: main
Are you sure you want to change the base?
Remove THIS macro #1756
Conversation
Tbh, my thoughts would be leave it as is for historical purposes. My guess is that we will eventually do what OoT did and just remove it (though keeping the object tutorial is probably still worth it). |
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.
no opinions on tutorial
Ended up reverting the changes to the documentation; let's just leave it as-is for now unless someone feels super strongly about it. |
Could you add it back to 'style.md'? The style guide should continue to be updated as needed imo. |
Oops, I just reverted everything in |
When OoT hit 100%, a PR to remove the
THIS
macro followed shortly thereafter: zeldaret/oot#1047This is just the MM equivalent of that change. Like Fig said in the original PR, this was useful to have around when we were decompiling things, but it's not really necessary now, and it hides exactly what is going on with that upcast.
Some stray thoughts I had:
Should we change the documentation? I did as part of this PR, but I can undo that change if it's desired. I really wasn't sure if we should leave it as-is for historical purposes or modify it to better suit the current repo.THIS
was being used inline (which were probably fake matches), particularly in thez_fbdemo_X
files. I replaced them too, but I wasn't sure if that was the best approach.I wrote a really crappy script to do this automatically for me, so hopefully nothing slipped through or broke.