tewtal / sm_practice_hack

Super Metroid Practice Hack
The Unlicense
27 stars 19 forks source link

Unnecessary update #129

Closed idlechild closed 1 year ago

idlechild commented 1 year ago

Pulled in misc-2.5.1-things branch Now creating two debug symbols files (one sorted by name and one sorted by address) Added labels next to all of our patches so they show up in the debug symbols Named all of the debug symbols (replacing +/++/- labels)

InsaneFirebat commented 1 year ago

I gotta come out and say it. Not everything needs a label. I think this makes a lot of things harder to read, and much more messy. I know the +/- labels can get out of hand, but to remove them entirely just doesn't sit well with me. I don't know if you've ever used the debugger, but a lot of your label names are just unreadable unless the debugger is maximized. The parent labels are at the beginning, so it's the branch name (plus the effective address printed after the label) that gets cut off if there's not enough space. The function search box (Ctrl+comma) can't even read the whole function name a lot of the time. You just have to type the whole name until it's narrowed down to one choice. I hate that you went through all this trouble, but I don't really want this. Long function names suck.

Debug_Symbols_By_Address.sym fails to load in Mesen. Maybe you didn't mean for it to be used that way? If not, it needs a new name so there's no confusion about which one to use. Edit: Could also be a .txt file if it's not meant to be loaded. You'd also want to update .gitignore for it to be included in the repo. I had an exception for Debugging_Symbols.sym specifically, which doesn't exist now.

idlechild commented 1 year ago

lol Okay I renamed my PR, please don't accept it as is. I made a separate PR with the small updates I had made plus your misc-2.5.1-things branch; you can take that one.

I would port in all the new hook labels so that the debug symbols includes all the places we have patched vanilla code, but I need to find a better diff tool so I can sort out those changes from the +/- changes. Also it's just not that important; I may not get back to it for a bit.

I'm hopeful there's a compromise regarding the labels. What's the longest a label can be before it becomes annoying to read?