-
Notifications
You must be signed in to change notification settings - Fork 41
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
[draft] fix / rewrite relative addressing #171
Conversation
- fix some bugs, add more checks, docs
Well, the SPC only supports short branches, but otherwise, this code could probably be reused there. That said, I don't think its a worth while adventure unless there is a plan to simply gut and replace the entire mnemonic processing core. I don't think anybody would disagree that the code in that section isn't exactly pretty or extendable... As for the code, and this is sort of a problem in asar already, just keep in mind that this doesn't work well if you place labels in banks which are mirrors of each other. Not necessarily a deal breaking issue, just something to keep in mind. I don't see a need to condense too much, clean up the todo and it should be alright. From a pure visual glance I'd give this a sign off(with a small side note below), but I have NOT actually tested this and that needs to be done a bit more in depth before I'd suggest a full green light. But, this certainly does seem to be an improvement. Side note: pretty sure the lhs needs to be a |
just editing on github, I think this is right, we'll find out :)
haven't forgotten about this! just been working on a release of the tool that depends on this change, should be trying to finish this out soon. |
…essing # Conflicts: # src/asar/arch-65816.cpp
hi folks, was finally revisiting getting this merged in. could I get whitelisted for appveyor, I tried running it on my own fork but there's some FTP upload stuff that's failing. I think I fixed the build error it was complaining about |
i implemented this logic when i rewrote arch-65816.cpp. seems to work fine, thanks |
DRAFT, for review.
This might actually be good to ship, I just want to throw some more tests and it, and maybe it run it on a few larger SNES assembly projects to make sure I didn't break anything. Disclaimer: I'm not an expert in SNES asm, just fixing stuff I have a good repro for.
Fixes #170 (details within)
Changes:
Pre-merge checklist:
Future me / future coders notes: