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

Decompile overlay_12_0226BEC4, make HeapID into a proper enum #148

Merged
merged 13 commits into from
Jun 6, 2023

Conversation

adrienntindall
Copy link
Collaborator

No description provided.

Also made some _ctor and _new -> _New
This refers to the finger icon in the catching tutorial, I think
Also got *some* documentation for UnkStruct_0200D748 done in the process
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.

trying to think if there's a better name than battle_finger but I can't come up with one

src/overlay_12_0226BEC4.c Show resolved Hide resolved
src/overlay_12_0226BEC4.c Show resolved Hide resolved
src/overlay_12_0226BEC4.c Show resolved Hide resolved
case 0:
GF_ASSERT(unkPtr != NULL);
{
NARC *bgHandleNarc = NARC_ctor(NARC_a_0_0_7, HEAP_ID_BATTLE);
Copy link
Member

Choose a reason for hiding this comment

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

ctor -> New

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

New or Open? Narcs are a lil different

Copy link
Member

Choose a reason for hiding this comment

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

either or, just not ctor

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we'll just let the other PR deal with this for now since they've already changed quite a bit of the files, gonna keep making my way through the battle overlay

Copy link
Member

Choose a reason for hiding this comment

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

do we want to remove all the renamings in this PR then? as since it'll conflict with the other one

Copy link
Contributor

@Laquinh Laquinh Jun 5, 2023

Choose a reason for hiding this comment

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

which renamings will conflict with the other PR? the _ctor to _New change is identical in both PRs so there should be no conflict, right? and the remaining changes here were left untouched in the other PR

Copy link
Member

Choose a reason for hiding this comment

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

thinking about it, I'm actually unsure if it will, there will be two commits making the same changes and I'm not 100% sure how git will handle it, it may just decide that the second commit is redundant and allow it to merge, or it may complain

Copy link
Contributor

@Laquinh Laquinh Jun 5, 2023

Choose a reason for hiding this comment

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

I've just done some tests in a dumb repository and I had no issue merging identical PRs. The conflict appeared when the same line suffered different modifications in each PR (as in #150), but in this case we shouldn't have issues. Anyways, if @adrienntindall rebased the branch before merging the PR (which I believe is the cleanest option), then I think it's guaranteed that rest will be straightforward.

src/scrcmd_c.c Show resolved Hide resolved
src/battle_finger.c Outdated Show resolved Hide resolved
}

if (finger->touchAnimationFlag == FALSE) {
int yOffset;
Copy link
Member

Choose a reason for hiding this comment

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

int -> s32

src/battle_finger.c Show resolved Hide resolved
src/battle_finger.c Show resolved Hide resolved
asm/battle_finger_data.s Outdated Show resolved Hide resolved
Old (left) versus new (right)
ov12_02248660 BattleContext_New
ov12_02250F44 BattleContext_Init
ov12_022486B0 BattleMain
ov12_0223BD14 BattleSystem_GetWinLoseFlags
ov12_0226CA90 sPlayerBattleCommands
ov12_022486FC BattleContext_Delete
ov12_0224BCC4 BattleSystem_CheckMoveHit
ov12_0224C018 BattleSystem_CheckMoveEffect
ov12_02248714 BattleSystem_CheckMoveHitEffect
ov12_0224873C BattleControllerPlayer_GetBattleMon
ov12_0224E4FC BattleSystem_GetBattleMon
ov12_02250370 BattleSystem_SetExperienceEarnFlags
ov12_02250360 BattleSystem_ClearExperienceEarnFlags
ov12_02255844 BattleSystem_GetHeldItemDamageBoost
ov12_0226340C BattleController_SetMoveEffect
void ov12_0226B97C(void *a0, u32 character, u32 pal, u32 cell, u32 animation);
BattleCursor *ov12_0226B9A4(void *a0, void *a1, HeapID heapId, u32 character, u32 pal, u32 cell, u32 animation, u32 a7, u32 a8);
void ov12_0226BA28(BattleCursor *cursor);
void ov12_0226BA4C(BattleCursor *cursor, int x0, int y0, int x1, int y1, int x2, int y2, int x3, int y3, fx32 a9);
Copy link
Member

Choose a reason for hiding this comment

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

these ints should really be s32s for consistency sake

void BattleFinger_FreeResources(void *a0, u32 character, u32 pal, u32 cell, u32 animation);
BattleFinger *BattleFinger_New(void *a0, void *a1, HeapID heapId, u32 character, u32 pal, u32 cell, u32 animation, u32 a7, u32 a8);
void BattleFinger_Delete(BattleFinger *finger);
void ov12_0226BCFC(BattleFinger *finger, int x, int y, fx32 a3);
Copy link
Member

Choose a reason for hiding this comment

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

int -> s32

include/overlay_12_0224E4FC.h Show resolved Hide resolved
include/overlay_12_0226BEC4.h Outdated Show resolved Hide resolved
@@ -97,15 +97,15 @@ BOOL BtlCmd_PokemonSlideIn(BattleSystem *bsys, BATTLECONTEXT *ctx) {
PokedexSetBattlerSeen(bsys, battlerId);
}
}
ov12_02250370(bsys, ctx, 1);
ov12_02250370(bsys, ctx, 3);
BattleSystem_SetExperienceEarnFlags(bsys, ctx, 1);
Copy link
Member

Choose a reason for hiding this comment

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

enum might be appropriate here instead of just numbers?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I tried, it lead to alot of issues elsewhere so I may just resign to using a define

src/battle_cursor.c Outdated Show resolved Hide resolved

//BattleCursor_Delete
void ov12_0226BA28(BattleCursor *cursor) {
int i;
Copy link
Member

Choose a reason for hiding this comment

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

s32 i

}

void ov12_0226BA4C(BattleCursor *cursor, int x0, int y0, int x1, int y1, int x2, int y2, int x3, int y3, fx32 a9) {
int i;
Copy link
Member

Choose a reason for hiding this comment

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

s32 i (does this need to be defined here or can it be defined in the loop?

FreeToHeap(cursor);
}

void ov12_0226BA4C(BattleCursor *cursor, int x0, int y0, int x1, int y1, int x2, int y2, int x3, int y3, fx32 a9) {
Copy link
Member

Choose a reason for hiding this comment

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

these params should probably be s32, as noted in the header

src/battle_cursor.c Outdated Show resolved Hide resolved
aside from int -> s32 stuff since I don't like that very much
This reverts commit eda37c4.
@red031000 red031000 merged commit 3e40bd3 into pret:master Jun 6, 2023
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.

None yet

3 participants