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

Add species defines for base forms with names #3248

Merged

Conversation

AsparagusEduardo
Copy link
Collaborator

Description

Adds species defines for base forms with names. Also updates element names that should refer to those names.
Eg. SPECIES_MELOETTA -> SPECIES_MELOETTA_ARIA
and gMonFrontPic_Meloetta -> gMonFrontPic_MeloettaAria

I've been chipping at this for a bit. It's almost done, so I'm opening the PR.
Ported from #2363.

Discord contact info

AsparagusEduardo

@AsparagusEduardo AsparagusEduardo mentioned this pull request Aug 15, 2023
43 tasks
@AsparagusEduardo AsparagusEduardo changed the title Add base form names Add species base form names Aug 22, 2023
@AsparagusEduardo AsparagusEduardo changed the title Add species base form names Add species defines for base forms with names Aug 31, 2023
@AsparagusEduardo AsparagusEduardo marked this pull request as ready for review September 8, 2023 16:13
@AsparagusEduardo
Copy link
Collaborator Author

Ready for review. (please squash)

# Conflicts:
#	src/data/pokemon_graphics/front_pic_anims.h
#	src/pokemon.c
#	src/pokemon_animation.c
# Conflicts:
#	include/constants/species.h
#	src/battle_setup.c
#	src/data/pokemon/form_change_table_pointers.h
#	src/data/pokemon/species_info.h
Copy link
Collaborator

@Bassoonian Bassoonian left a comment

Choose a reason for hiding this comment

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

This could be worth another PR since it's probably also subjective, but with the base forms including the name of the default form, it feels a bit curious to me that the actual data for the other forms is all the way at the end. I understand species IDs can follow this order, but it'd be a lot more easy to work with if forms were gathered together in all other cases (learnsets, base stats, animations etc).

src/battle_util.c Show resolved Hide resolved
src/pokemon_jump.c Outdated Show resolved Hide resolved
src/trade.c Outdated Show resolved Hide resolved
@AsparagusEduardo
Copy link
Collaborator Author

This could be worth another PR since it's probably also subjective, but with the base forms including the name of the default form, it feels a bit curious to me that the actual data for the other forms is all the way at the end. I understand species IDs can follow this order, but it'd be a lot more easy to work with if forms were gathered together in all other cases (learnsets, base stats, animations etc).

Already planning on doing this :D (I did it in #2363)

@AsparagusEduardo
Copy link
Collaborator Author

Changes applied.

@AsparagusEduardo
Copy link
Collaborator Author

Fixed modern compile

Copy link
Collaborator

@mrgriffin mrgriffin left a comment

Choose a reason for hiding this comment

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

Nice!

I think we should add to the modernFatefulEncounter comment in include/pokemon.h to say that this bit does nothing in Expansion, but that it's kept for save-compatibility and if the developer never used seteventmon/setmonmodernfatefulencounter they can repurpose it.
EDIT: Or alternatively, we could keep modernFatefulEncounter in its entirety and let our users remove it if they want to (i.e. know it's safe to).
... Or leave it for now and we could remove it later when we have save migrations.

Other than that, looks good to go! :)

@mrgriffin
Copy link
Collaborator

mrgriffin commented Oct 23, 2023

I tried to resolve the conflicts with #3435 🤞
EDIT: Fuck, I broke it. Sorry 😭

@mrgriffin mrgriffin dismissed Bassoonian’s stale review October 23, 2023 11:34

Will be done in a future PR.

@mrgriffin mrgriffin merged commit 26971fc into rh-hideout:upcoming Oct 23, 2023
1 check passed
@AsparagusEduardo AsparagusEduardo deleted the RHH/pr/upcoming/formConstants branch October 23, 2023 11:59
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.

3 participants