pokeathlon part 3 by nathanfallet · Pull Request #418 · pret/pokeheartgold

nathanfallet · 2025-11-13T19:25:37Z

Same idea as #416 but I'm doing the next functions.

Laquinh · 2025-11-14T22:26:35Z

src/pokeathlon/pokeathlon_course.c

+    struct {
+        void *ptr;
+        u8 bytes[4];
+    } *stateInfo = dest;


You have identified the structure stored at the memory address dest, therefore you know the dest argument points to an instance of this struct, instead of an ambiguous void *. The function prototype should make this clear. :)

Try backtracking the function call, see where this dest takes its value from. It's the pointer to a member of PokeathlonCourseData. Can you somehow fit a StateInfo object inside PokeathlonCourseData so that the code still matches? As a hint, this will give you insight into what u8 bytes[4] actually contains.

Laquinh · 2025-11-14T22:48:36Z

src/pokeathlon/pokeathlon_course.c

+        void *dest = PokeathlonCourse_GetPlayerProfile(data->heapAllocPtr1, 0);
+        void *src = Save_PlayerData_GetProfile(data->args->saveData);
+        PlayerProfile_Copy(src, dest);


I feel like you're relying on void pointers too much. Take a look at this for example. You have a call to Save_PlayerData_GetProfile. Looking at the function prototype, you know the parameter is of type SaveData*, and the return value is of type PlayerProfile*. This should be enough for you to

define saveData in PokeathlonCourseArgs as a SaveData* instead of void*,

define the src variable you're initialising here as PlayerProfile* instead of void*,

modify PokeathlonCourse_GetSaveData function declaration so that it returns a SaveData* instead of void*...

By modifying the type of PokeathlonCourseArgs::saveData first, you should get a bunch of compile warnings about implicit conversions, which will help you find all spots in the code you need to update to the new type.

Likewise, PlayerProfile_Copy takes a PlayerProfile* as the second argument, so that should be the type of dest.

Laquinh · 2025-11-14T22:55:52Z

src/pokeathlon/pokeathlon_course.c

+void *PokeathlonCourse_GetPlayerProfile(void *profiles, int index) {
+    return (u8 *)profiles + (PlayerProfile_sizeof() * index);
+}


Once you apply the changes suggested above, you'll see that you've initialised a PlayerProfile* by calling PokeathlonCourse_GetPlayerProfile. This lets you know that PokeathlonCourse_GetPlayerProfile returns a PlayerProfile*, and you should update the function prototype to reflect that. It also suggests that void *profiles is actually an array of PlayerProfile objects.

On a side note, is the cast to (u8 *) really necessary? It matches for me without it.

adrienntindall

I agree with what Laquinh is saying.

Most of the void* calls are actually pointers to structs or function pointers that you haven't defined yet and that need to be defined, even if they're left as unknowns for now.

adrienntindall · 2025-11-13T19:48:01Z

src/pokeathlon/pokeathlon_course.c

 }
+
+BOOL ov96_021E5C2C(PokeathlonCourseData *data) {
+    typedef BOOL (*StateFunc)(PokeathlonCourseData *, void **);


Don't typedef in a local scope

adrienntindall · 2025-11-15T06:44:50Z

src/pokeathlon/pokeathlon_course.c

+    typedef BOOL (*StateFunc)(PokeathlonCourseData *, void **);
+    void **stateDataPtr = (void **)&data->stateArgsBase;
+    u8 index = data->stateIndex;
+    void **functionTable = data->stateData;


Why is this a void** instead of a StateFunc* data type?

adrienntindall · 2025-11-15T06:45:18Z

src/pokeathlon/pokeathlon_course.c

+    BOOL result;
+
+    do {
+        statePtr = (void **)data->state;


Should be a StateHandlerFunc* data type

adrienntindall · 2025-11-15T06:46:57Z

src/pokeathlon/pokeathlon_course.c

+}
+
+void *PokeathlonCourse_GetParticipantData1(PokeathlonCourseData *data, int index) {
+    return (u8 *)&data->participantData1Base + (0x7C * index);


Define participantData1Base struct, this is obvious struct access to it.

adrienntindall · 2025-11-15T06:47:25Z

src/pokeathlon/pokeathlon_course.c

+}
+
+void *PokeathlonCourse_GetParticipantData2(PokeathlonCourseData *data, int index) {
+    return &data->participantData2[0x7C * index];


Might be a struct array where participantData2 is a struct of size 0x7C

adrienntindall · 2025-11-15T06:48:08Z

src/pokeathlon/pokeathlon_course.c

+}
+
+void *ov96_021E5D78(PokeathlonCourseData *data, int index) {
+    return &data->field_72C[0x60 * index];


Might be a struct array where field_72C is a struct of size 0x60

adrienntindall

Forgot to request changes on previous review

nathanfallet · 2025-11-16T13:38:57Z

Thanks for all your feedbacks. I'll try to have a look and fix that soon!

nathanfallet · 2025-12-02T03:54:23Z

I tried to make it better based on your reviews, but I cannot get all of it yet. So maybe we keep it close to what we currently have and you can have a look later if you want to improve it. Let me know what you think.

Laquinh · 2025-12-03T21:30:27Z

src/pokeathlon/pokeathlon_course.c

+PokeathlonFieldData *ov96_021E5D6C(PokeathlonCourseData *data) {
+    return (PokeathlonFieldData *)data->field_72C;
+}


This function makes me believe that field_72C is, in fact, an array of PokeathlonFieldData instead of an array of u8. That would allow us to get rid of the casting.
And PokeathlonFieldData takes up 0x60 bytes! Maybe that has something to do with field_72C's 0x60 stride? :)

Laquinh · 2025-12-03T21:51:02Z

src/pokeathlon/pokeathlon_course.c

+PokeathlonParticipantData *PokeathlonCourse_GetParticipantData1(PokeathlonCourseData *data, int index) {
+    return (PokeathlonParticipantData *)((u8 *)&data->participantData1Base + (0x7C * index));
+}
+
+PokeathlonParticipantData *PokeathlonCourse_GetParticipantData2(PokeathlonCourseData *data, int index) {
+    return (PokeathlonParticipantData *)&data->participantData2[0x7C * index];
+}


This is a similar case. It smells a lot like there's gotta be a PokeathlonParticipantData array somewhere inside PokeathlonCourseData, especially given that we're using a 0x7C stride here, and PokeathlonParticipantData takes up 0x7C bytes.

May I ask where you got the idea that the fields are divided in participantData1Base and participantData2? a 4-element array of PokeathlonParticipantData in participantData1Base's place would fit perfectly, and would clean the PokeathlonCourse_GetParticipantData1 function a lot. As for PokeathlonCourse_GetParticipantData2, maybe it's accessing a member of PokeathlonParticipantData? And that would explain the +0x04 offset you were probably seeing in the assembly. From what i know, you don't really have any proof that PokeathlonCourse_GetParticipantData2 returns a PokeathlonParticipantData*, after all.

Afterwards, the functions should be renamed, so the one above is something like PokeathlonCourse_GetParticipantData (not 1), and the one below is something like PokeathlonCourse_GetParticipantUnk04 (this is a suggestion, idk if others may have a different opinion on how to name such a function).

Laquinh · 2025-12-03T21:52:37Z

src/pokeathlon/pokeathlon_course.c

+void PokeathlonCourse_InitStateInfo(const void *src, PokeathlonStateInfo *dest) {
+    dest->ptr = (void *)src;
+    dest->field_04 = 1;
+    dest->stateArgsBase = 0;
+    dest->stateIndex = 0;
+    dest->field_07 = 0;
+}


Not a fan of naming the parameters src and dest. It suggests that you're going to copy the contents of src onto dest or something like that, which you're not doing.

Laquinh · 2025-12-03T21:59:45Z

src/pokeathlon/pokeathlon_course.c

+}
+
+PlayerProfile *PokeathlonCourse_GetPlayerProfile(PlayerProfile *profiles, int index) {
+    return (PlayerProfile *)((u8 *)profiles + (PlayerProfile_sizeof() * index));


I don't have too much spare time right now and I haven't managed to match changing any of this, but I'm pretty confident there should be a cleaner way to write this function.

Laquinh · 2025-12-03T22:07:35Z

include/pokeathlon/pokeathlon.h

+
+// Participant data structure (0x7C = 124 bytes each)
+typedef struct PokeathlonParticipantData {
+    u8 data[0x7C]; // Opaque data for now


As explained in the other comment, there appears to be a function (PokeathlonCourse_GetParticipantData2) that returns the 4th element of this array. May be worth to un-opaque that one element (?), like

Suggested change

u8 data[0x7C]; // Opaque data for now

u8 field_00[0x03];

u8 field_04;

u8 field_05[0x78];

Not sure if this is something we usually do, though.

Laquinh

Definitely an improvement :)

github-actions · 2026-02-02T00:56:42Z

This pull request has had no activity for 60 days and will be marked stale. If there is no further activity, it will be closed in 30 days.

nathanfallet · 2026-02-02T10:04:14Z

Hey @Laquinh @adrienntindall I don't think I'll have more time (in upcoming days) to improve the code here. So either we merge that and you're free to improve it later, or we let the PR close and we loose all of that (would be sad right?) What do you think?

adrienntindall · 2026-02-02T17:18:55Z

Hey @Laquinh @adrienntindall I don't think I'll have more time (in upcoming days) to improve the code here. So either we merge that and you're free to improve it later, or we let the PR close and we loose all of that (would be sad right?) What do you think?

I think that the current state of the PR isn't up to the repository's standards to merge. Asking maintainers to clean up code in your stead when we have already asked for changes isn't fair to anyone actively working on the project.

It is ok if you take a while to get things done, but if the PR remains inactive then it will be closed. If you want to finish it at a later date and reopen it after that then that's fine, but we will still be asking for all the requested changes to me made.

nathanfallet added 16 commits November 13, 2025 19:39

ov96_021E5C2C v0

b697a2c

ov96_021E5C2C v1

352fabf

Merge branch 'master' into pokeathlon-part-3

6bf5cee

ov96_021E5C50 v0

72bcc89

ov96_021E5C80 v0

6ba27d2

ov96_021E5C90 and ov96_021E5D24 v0

35f0c0c

ov96_021E5D34 v0

71d981b

ov96_021E5D24 v1

9e99d51

ov96_021E5D40 v0

5526f40

more functions

a0851cf

more functions

ae15d27

improve readability

9ff0110

more functions again

53a91f4

this is getting repetitive

9082100

giving meaningful names to pokeathlon func

588627a

renaming more fields

d40c759

nathanfallet marked this pull request as ready for review November 14, 2025 00:03

nathanfallet changed the title ~~wip: pokeathlon part 3~~ pokeathlon part 3 Nov 14, 2025

rename like this, cannot do better yet

71ecc17

Laquinh suggested changes Nov 14, 2025

View reviewed changes

adrienntindall reviewed Nov 15, 2025

View reviewed changes

adrienntindall requested changes Nov 15, 2025

View reviewed changes

nathanfallet added 2 commits December 1, 2025 10:37

a few improvements (still need some work)

a849543

better but not sure if i can do more

f4a3f92

Laquinh reviewed Dec 3, 2025

View reviewed changes

Laquinh suggested changes Dec 3, 2025

View reviewed changes

github-actions bot added the stale label Feb 2, 2026

github-actions bot removed the stale label Feb 3, 2026

-    u8 data[0x7C]; // Opaque data for now
+    u8 field_00[0x03];
+    u8 field_04;
+    u8 field_05[0x78];

Conversation

nathanfallet commented Nov 13, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adrienntindall left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adrienntindall left a comment

Choose a reason for hiding this comment

Uh oh!

nathanfallet commented Nov 16, 2025

Uh oh!

nathanfallet commented Dec 2, 2025

Uh oh!

Laquinh Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Laquinh left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Feb 2, 2026

Uh oh!

nathanfallet commented Feb 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adrienntindall commented Feb 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Laquinh Dec 3, 2025 •

edited

Loading

nathanfallet commented Feb 2, 2026 •

edited

Loading