Skip to content
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

Poketch Memo Pad (ov24) Decompilation #551

Merged
merged 31 commits into from
Sep 28, 2024
Merged

Conversation

Lincoln-LM
Copy link
Contributor

Some names are taken from pokeplatinum but otherwise are just whatever seemed reasonable. Not sure if there are any stylistic/naming standards that need to be taken into account.

Copy link
Member

@red031000 red031000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

firstly, thanks so much for the PR, it's not often that people contribute to diamond, not many people have an interest in it so it's always nice when someone makes a PR

I've labelled some things that need changing, of particular note is the pointer style, the * should be next to the variable name, on the right always, header declarations should also have the variable name there. It'd help massively if you documented as you went along (even best guess) it's fine if there's no info, but giving clues as to what a function does and parameter types helps hugely with further decompilation

thank you again

arm9/overlays/24/include/overlay_24.h Outdated Show resolved Hide resolved
arm9/overlays/24/include/overlay_24.h Outdated Show resolved Hide resolved
arm9/overlays/24/include/overlay_24.h Outdated Show resolved Hide resolved
arm9/overlays/24/src/ov24_02254840.c Outdated Show resolved Hide resolved
}

BOOL ov24_0225489C(MemoPadAppHandler* appHandler, u32 arg1, u32 arg2, u32 arg3) {
static const u8 ov24_022550F8[] = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

genearlly these should be outside the function

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also any idea what these are?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this a general rule just for consistency? these aren't used anywhere else so my intuition would tell me that they're defined and used exclusively within the functions

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah a general rule for consistency

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ov24_022550F8 appears to denote the bounding boxes of the pencil and eraser buttons

arm9/overlays/24/src/ov24_02254CA0.c Outdated Show resolved Hide resolved
int a = (x >> 3) + (y >> 3) * 0x14;
int e = ((x + width - 1) >> 3) - (x >> 3) + 1;
int b = ((y + height - 1) >> 3) - (y >> 3) + 1;
while (b--) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this feels like it should be a for loop, not a while loop

arm9/overlays/24/src/ov24_02254CA0.c Outdated Show resolved Hide resolved

void ov24_02255078(MemoPadDisplayHandler* displayHandler) {
// TODO: types
static const u32 ov24_0225514C[2][4] = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any idea what these are?

arm9/overlays/24/src/ov24_02254CA0.c Outdated Show resolved Hide resolved
@red031000
Copy link
Member

Hi, just wanted to check up here and make sure this PR doesn't fall through the cracks
there's still one or two things to be renamed (e.g. StylusType to TouchType) and then imo it's good to go

if you're a bit busy, that's ok, just lmk when you've done it and I'll re-review

@Lincoln-LM
Copy link
Contributor Author

sorry about the delay on this. should be fine now

@red031000 red031000 merged commit 9a81717 into pret:master Sep 28, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants