-
Notifications
You must be signed in to change notification settings - Fork 20
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
Synced function symbols from decomp #187
Merged
UsernameFodder
merged 3 commits into
UsernameFodder:master
from
AnonymousRandomPerson:master
Sep 7, 2023
Merged
Changes from 2 commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
/target | ||
Cargo.lock | ||
/out | ||
__pycache__ | ||
__pycache__ | ||
.vscode |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Do you know if these are (mostly) the only core functions with weird naming? It seems like after these you get into the allocator functions. If these are the only exceptions maybe it might make more sense to implement an exception list for the case checker so we can use the real names.
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.
These are not the only ones. I've seen this naming convention in some of the library code, and the number of functions would make an exception list grow very large.
I don't personally like this naming convention of mixing case with
domain
_action
, but it is a real naming convention used by some C programming circles (e.g., pret/pokeheartgold#213). It may be worth supporting this style outright, rather than making exceptions.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 would need to see more examples. This kind of mixed case convention seems extremely permissive, because
domain
can be anything (MIi
== PascalCase,SVC
== SCREAMING_SNAKE_CASE,_start
== snake_case), so a sufficiently flexible check would have to accept almost all names except maybecamelCase
and constructs likeUPPERlower
. It's not something I'd really be keen on implementing checks for. You might as well just turn off naming checks at that point.Maybe let's just leave it as the converted PascalCase names for now. You're maintaining a list if the mixed case ones, so it should be easy to revisit this if we start finding more of these.
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.
If you search for
jn_spl.o
in this map file, that section of the file has more examples.I think
_start_AutoloadDoneCallback
is a special case because it is related to the_start
function, which is hard-coded to be the entry point of the program regardless of naming convention. Ignoring that, the convention seems to be PascalCase between the underscores, allowing consecutive capitals for abbreviations. This at least allows some restrictions.I'm good with keeping the converted names for now. I'm not very interested in the library code, but if somebody else starts extracting them then we can revisit this.