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

Pokeradar Documentation #137

Merged
merged 3 commits into from
Feb 4, 2024
Merged

Pokeradar Documentation #137

merged 3 commits into from
Feb 4, 2024

Conversation

froggestspirit
Copy link
Contributor

Started documenting the pokeradar. Still more to do

Copy link
Collaborator

@lhearachel lhearachel left a comment

Choose a reason for hiding this comment

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

Nice work tracking this stuff down.

These aren't full comments, since some of these apply to multiple instances throughout the file.

@@ -1,6 +1,6 @@
#ifndef POKEPLATINUM_STRUCT_0206942C_DECL_H
#define POKEPLATINUM_STRUCT_0206942C_DECL_H

typedef struct UnkStruct_0206942C_t UnkStruct_0206942C;
typedef struct RadarChain_t RadarChain;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Repo style is to omit _t suffixes on struct types wherever possible; it's safe to declare this as typedef struct RadarChain RadarChain;

Additionally, is it possible to merge this and its definition into the pokeradar.h header? We're using transparent structs. Alternatively, merge the typedef and the definition into the same header within include/struct_defs.

src/pokeradar.c Outdated
static BOOL CheckPatchShiny(const int param0);
static void IncWithCap(int * param0);

RadarChain * CreateRadarChain (const int param0) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

  • RadarChain_Init?
  • param0 looks like a heapID to me.
  • function definition should look like:
RadarChain* RadarChain_Init(const int heapID)
{
    // code
}

src/pokeradar.c Outdated
Comment on lines 67 to 69
RadarChain * chain;

chain = Heap_AllocFromHeap(param0, sizeof(RadarChain));
Copy link
Collaborator

Choose a reason for hiding this comment

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

We use C99; prefer declare + define on the same line wherever possible, so long as it matches.

Suggested change
RadarChain * chain;
chain = Heap_AllocFromHeap(param0, sizeof(RadarChain));
RadarChain *chain = Heap_AllocFromHeap(param0, sizeof(RadarChain));

src/pokeradar.c Outdated
return chain;
}

void FreeRadarChain (RadarChain * chain) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

  • RadarChain_Free?
  • RadarChain *chain (in general, align the pointer-star to immediately prefix the variable name)

src/pokeradar.c Outdated
Heap_FreeToHeap(chain);
}

void ClearRadarChain (RadarChain * chain) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

  • RadarChain_Clear? RadarChain_Zero?
  • RadarChain *chain

src/pokeradar.c Outdated
Comment on lines 388 to 402
static BOOL sub_020698AC (const RadarChain * chain, const int param1, const int param2, u8 * param3)
{
u8 patchRing;

for (patchRing = 0; patchRing < NUM_GRASS_PATCHES; patchRing++) {
if (chain->patch[patchRing].active) {
if ((chain->patch[patchRing].unk_00 == param1) && (chain->patch[patchRing].unk_04 == param2)) {
*param3 = patchRing;
return 1;
}
}
}

return 0;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Prefer returning TRUE or FALSE in functions that return a BOOL.

src/pokeradar.c Outdated
}
}

GF_ASSERT(0);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Prefer using a BOOL-evaluating expression inside GF_ASSERT, e.g. GF_ASSERT(FALSE) vs GF_ASSERT(0).

src/pokeradar.c Outdated
Comment on lines 468 to 472
if (battleResult == 0x1) { // If the battle resulted in fainting the mon, use the regular rates
rates = ratesNormal;
} else if (battleResult == 0x4) { // If the battle resulted in a capture, use the boosted rates
rates = ratesBoosted;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you pull these from constants/battle.h?

src/pokeradar.c Outdated
}
break;
case 1:
Sound_PlayBGM(1150);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you create an enum definition for this in constants/sdat.h?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Remind me some time, and I'll fill out the other songs in sdat.h

src/pokeradar.c Outdated
return 0;
}

static BOOL CheckPatchShiny (const int chianCount) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

chainCount? (spelling)

Copy link
Collaborator

@lhearachel lhearachel left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks for doing this! I'll merge this in, pending the PR build action.

@lhearachel lhearachel merged commit 3dfe74e into pret:main Feb 4, 2024
1 check passed
github-actions bot pushed a commit that referenced this pull request Feb 4, 2024
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