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

clean up encounter.c #208

Merged
merged 1 commit into from
Sep 30, 2023
Merged

clean up encounter.c #208

merged 1 commit into from
Sep 30, 2023

Conversation

red031000
Copy link
Member

WIP and currently extremely messy, will need cleaning up and finishing before review

@red031000 red031000 marked this pull request as ready for review August 16, 2023 22:53
@red031000
Copy link
Member Author

ready for review :)

Copy link
Collaborator

@adrienntindall adrienntindall left a comment

Choose a reason for hiding this comment

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

Only 302/373 files viewed but I have 33 comments and github web is lagging

include/encounter.h Show resolved Hide resolved
include/field_follow_poke.h Outdated Show resolved Hide resolved
include/field_follow_poke.h Outdated Show resolved Hide resolved
include/field_map_object.h Outdated Show resolved Hide resolved
include/field_system.h Outdated Show resolved Hide resolved
src/encounter.c Outdated Show resolved Hide resolved
src/encounter.c Outdated Show resolved Hide resolved
src/field/overlay_01_rock_smash_item.c Outdated Show resolved Hide resolved
src/field/scrcmd_pokemon_misc.c Outdated Show resolved Hide resolved
src/field/scrcmd_pokemon_misc.c Outdated Show resolved Hide resolved
Copy link
Collaborator

@adrienntindall adrienntindall left a comment

Choose a reason for hiding this comment

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

336/373 I hope red realizes that I'm not merging until all these changes are addressed

src/field_map_object.c Outdated Show resolved Hide resolved
src/field_system.c Outdated Show resolved Hide resolved
src/field_system.c Outdated Show resolved Hide resolved
src/field_use_item.c Outdated Show resolved Hide resolved
src/field_use_item.c Outdated Show resolved Hide resolved
src/overlay_36.c Outdated
@@ -40,9 +40,9 @@ static BOOL ov36_App_InitGameState_AfterOakSpeech_AppExit(OVY_MANAGER* man, int*
static BOOL ov36_TitleScreen_NewGame_AppInit(OVY_MANAGER* man, int* state);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we have enough info to name overlay_36?

src/party.c Outdated Show resolved Hide resolved
src/party.c Outdated
BOOL Party_SwapSlots(PARTY *party, int slotA, int slotB) {
PARTY_EXTRA_SUB tmp_PARTY_EXTRA_SUB;
BOOL Party_SwapSlots(Party *party, int slotA, int slotB) {
PartyExtraSub tmp_PartyExtraSub;
Pokemon *tmp_POKEMON;
Copy link
Collaborator

Choose a reason for hiding this comment

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

fix these local variable names please

storage->boxModifiedFlag = 0;
}

u32 PCStorage_GetBoxModifiedFlags(PC_STORAGE* storage) {
u32 PCStorage_GetBoxModifiedFlags(PCStorage* storage) {
return storage->boxModifiedFlag;
}

u32 sub_02074120(void) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

PCBox_sizeof

src/pokemon_storage_system.c Show resolved Hide resolved
src/safari_zone.c Outdated Show resolved Hide resolved
src/sav_system_info.c Outdated Show resolved Hide resolved
src/save_arrays.c Show resolved Hide resolved
src/save_arrays.c Outdated Show resolved Hide resolved
src/scrcmd_17.c Outdated Show resolved Hide resolved
src/scrcmd_17.c Outdated Show resolved Hide resolved
@@ -15,28 +15,28 @@
BOOL ScrCmd_GetTrainerPathToPlayer(ScriptContext *ctx) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we clean up the local variable names in this function please?

src/scrcmd_c.c Outdated
@@ -164,7 +164,7 @@ BOOL ScrCmd_Wait(ScriptContext* ctx) {
}

static BOOL RunPauseTimer(ScriptContext* ctx) {
u16* frames_to_wait = GetVarPointer(ctx->fsys, ctx->data[0]);
u16* frames_to_wait = GetVarPointer(ctx->fieldSystem, ctx->data[0]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

alot of the local variables in this file contradict our style conventions

src/encounter.c Show resolved Hide resolved
adrienntindall added a commit that referenced this pull request Aug 20, 2023
@red031000 red031000 force-pushed the master branch 2 times, most recently from bd3f21c to 51df5f8 Compare August 20, 2023 17:35
@red031000 red031000 force-pushed the master branch 2 times, most recently from 1129ca3 to 75c3424 Compare August 22, 2023 08:18
adrienntindall added a commit that referenced this pull request Aug 22, 2023
#208 split PR 2, rename fsys to fieldSystem
adrienntindall added a commit that referenced this pull request Aug 24, 2023
#208 split PR 3, rename savedata to saveData
include/constants/std_script.h Outdated Show resolved Hide resolved
src/field/scrcmd_pokemon_misc.c Show resolved Hide resolved
include/constants/std_script.h Outdated Show resolved Hide resolved
include/fieldmap.h Outdated Show resolved Hide resolved
include/overlay_02.h Outdated Show resolved Hide resolved
@red031000
Copy link
Member Author

@PikalaxALT review addressed

@PikalaxALT
Copy link
Collaborator

Declining to review due to the habitual squashing of commits deleting progress from previous review.

@red031000
Copy link
Member Author

the comments should still be present, plus, given that you have requested changes before, according to the repo rules, all who have requested changes that have write access must approve for a PR to be merged (unless I bypass branch protections)

red031000 added a commit to red031000/pokeheartgold that referenced this pull request Sep 12, 2023
adrienntindall added a commit that referenced this pull request Sep 17, 2023
#208 split PR 4, rename some misc funcs
@PikalaxALT PikalaxALT dismissed their stale review September 17, 2023 22:44

Review references commits that were rebased out of existence.

Up to Battle Random functions

start cleaning up encounter.c

fix build

fix build again

remove two files from bad rebase

switch fully to battle type constants

rename TRAINER to Trainer

few more funcs

finish cleanup

address some review comments

de-capitalise std_script
@adrienntindall
Copy link
Collaborator

Some state machine reviews were left unaddressed but am approving for the sake of letting red work on other things

@adrienntindall adrienntindall merged commit 1e1d1e7 into pret:master Sep 30, 2023
1 check passed
github-actions bot pushed a commit that referenced this pull request Sep 30, 2023
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.

4 participants