Open tdadvocate opened 1 year ago
Thanks i'll look this over soon.
@tdadvocate Just comment when you are ready for another review.
So I did this across multiple commits as I had time on my fork. How would I go about requesting a review? This is my first time doing pull requests so I am not sure if it is best for me to close this and request a new pull based on all the commits that I made over the past 2 weeks or if there is some way that I can just merge them all together on this request. Sorry for being a pain and thank you for all of the help so far!
Feel free to ping me or people in the discord if you need quicker reviews. I only noticed your fixes right now, and I won't have time to look at it for another few days at least.
So I did this across multiple commits as I had time on my fork. How would I go about requesting a review? This is my first time doing pull requests so I am not sure if it is best for me to close this and request a new pull based on all the commits that I made over the past 2 weeks or if there is some way that I can just merge them all together on this request. Sorry for being a pain and thank you for all of the help so far!
My bad; I thought you were still making commits and missed your comment here. It's easier to skip past notifications on GitHub. Within reason of course, you are more than welcome to ping us on discord in the future to let us know you are ready for reviews as mid-kid stated. I'll review this tonight when I get home from work. Again, thank you for contributing!!!
I believe that all the changes requested should now match up to what was suggested. I skimmed back through and I don't see any immediately noticeable errors from my new understanding of things. If I did miss something, please let me know and I will be more than happy to correct them! Thank you both for all of your help on so far on supporting this project!
This currently fails to build. Make sure you do a make compare
and verify it is matching, and it builds before pushing :).
error: engine/dumps/bank03.asm(332):
'ItemsThrowAwayText' already defined at engine/dumps/bank03.asm(326)
error: engine/dumps/bank03.asm(3413):
"BallSoCloseText" is not a macro
Comments added to many functions and text strings that were not fully disassembled. Also translated to some of the text within the file related to the functions that have been researched.