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

Species Simplifier™ - Part 3 #3562

Conversation

AsparagusEduardo
Copy link
Collaborator

@AsparagusEduardo AsparagusEduardo commented Nov 14, 2023

Description

Third part of the PR to make mon easier to add.

  • All data has been reordered to match the order set by gSpeciesInfo.
  • All graphical data is now read from gSpeciesInfo.
    • Reducing space used by graphical data by removing the redundant tags and sizes used in CompressedSpriteSheet and CompressedSpritePalette.
  • Learnsets are now read from gSpeciesInfo via GetSpeciesLevelUpLearnset and GetSpeciesTeachableLearnset.
  • Evolutions are now read from gSpeciesInfo.
    • Use compund literals for Evolutions, reducing space used (before Gen 9, it was around 60Kb)
    • Prevent Pokémon offspring if they're disabled.
    • Prevents Pokémon from evolving if the target evolution is disabled.
      • Eviolite is also affected by this.
  • All Pokédex Data is now read from gSpeciesInfo.
    • Pokédex data is now read per form and can be seen using the HGSS dex.
    • When using the HGSS dex, catching a form of a Pokémon shows their dex entry instead of the base form.
    • Pokédex is now stored as compound strings.
  • Form Species ID Tables and Form Change Tables are now read from gSpeciesInfo.
    • Added missing Ursaluna Form ID Table.
  • Simplify Cry Tables by using Cry IDs and asigning those to each species in gSpeciesInfo.
  • Added missing closing block comments in gSpeciesInfo.
  • Converted remaining Species flags in gSpeciesInfo to gcc bitflags.
    • Renamed gigantamax bitflag to isGigantamax.
  • Renamed config/species_families.h to config/species_enabled.h.
    • Moved P_GEN_x_POKEMON settings to it.
    • Added Cosplay and Cap Pikachu toggles.
    • Added Fusion, Primal Reversion and Ultra Burst toggles.
    • Added option to disable cross generational evolutions and pre-evolutions.
    • Added option to include new evolutions in regional dex.
  • Fixed Pokémon Debug Menu not reading animation delay.
  • Fixed button label inconsistency in pokémon debug menu.
  • gFrontierBannedSpecies now checks for IsSpeciesEnabled instead of relying on ifdef blocks.
  • Applied missing uses of PLACEHOLDER_ANIM_SINGLE_FRAME.
  • Renamed PLACEHOLDER_TWO_FRAME_ANIMATION to PLACEHOLDER_ANIM_TWO_FRAMES.
  • Automatic ASSUMPTION_FAIL in tests when used species are disabled.
    • Because of this, instances of ASSUME(P_GEN_x_POKEMON == TRUE) have been removed.
  • Added Minior species label synonyms (eg. SPECIES_MINIOR_ORANGE equates to SPECIES_MINIOR_METEOR_ORANGE)
  • Removed Old Unown level up learnsets.
  • Removed unused Pokémon 2nd animations.
  • Fixed HGSS not checking for null footprints.

Missing stuff to be handled in a separate PR:

  • Dex text for a lot of forms.
  • Optimize gSpeciesInfo as much as it can (it already reduces the amount used as is)
    • Reduce overhead by introducing a struct MonGraphics that groups graphical data for gender differences.
  • EV Yield spacing.
  • Removing .noFlip = FALSE instancess
  • Refactoring Egg Moves' mess. Done in Egg Move Refactor #4534
  • Macro for item definitions.

Issue(s) that this PR closes

Closes #3512
Closes #2738
Closes #3629

Discord contact info

AsparagusEduardo

@AsparagusEduardo AsparagusEduardo changed the title Added SpeciesInfo fields Species Simplifier™ - Part 3 Nov 14, 2023
@AsparagusEduardo
Copy link
Collaborator Author

Accidentally had a different title ^^;

@AsparagusEduardo AsparagusEduardo force-pushed the RHH/pr/upcoming/speciesSimplifier_Part3 branch from d14bdd1 to 0b98485 Compare November 23, 2023 23:31
@AsparagusEduardo AsparagusEduardo force-pushed the RHH/pr/upcoming/speciesSimplifier_Part3 branch from 0b98485 to ede5cc8 Compare November 24, 2023 14:08
@mrgriffin
Copy link
Collaborator

mrgriffin commented Dec 8, 2023

The numbers in FRONT_PIC and BACK_PICK are already incomprehensible.

I think the obviously out of scope best solution would be to somehow generate these numbers from the images themselves (it's just a bounding box rounded up to a multiple of 8?), and equip gbagfx with the ability to produce some special variant of .4bpp which contains that info.

I thought maybe we would have been able to make the macro work like: FRONT_PIC(id, width: W, height: H), but because of MON_COORDS_SIZE I don't think that's workable.

For reference, here's a sketch of the idea, but note that it doesn't work in this case:

struct MonPic {
  const u32 *data;
  u8 width;
  u8 height;
};

#define PIC(id, ...) { .data = gMonFrontPic_##id, __VA_ARGS__ }

// Used as '.frontPic = PIC(Alcremie##sweet, width: 40, height: 56),'
// Where we've defined 'struct MonPic frontPic;'

Of course we don't necessarily have to keep MON_COORDS_SIZE.

Alternatively it's possible to have PIC(Alcremie##sweet, MON_COORDS_SIZE(40, 56)) which is perhaps a little more self-describing. If that was size: MON_COORDS_SIZE(40, 56) and we had a struct, that would lend itself to incorporating more numbers, as CF suggests.

Non-blocking: I would say that I don't like the look of:

  .someMember = ...,
  .someOtherMember = ...,
  FRONT_PIC(...),
  .someFourthMember = ...,

And it would (imo) be nicer if that third line could be .frontPic = FRONT_PIC(...). For pret#1717 we hid multiple members in our defines, but if matching wasn't a concern I would have preferred a struct as above.

@mrgriffin
Copy link
Collaborator

e.g. PALETTE(Bulbasaur) instead of PALETTE(sBulbasaurLevelUpLearnset).

PALETTE covers for both regular and Shiny Palettes. If you'd want to show both full names, you'd need to do:

PALETTE(gMonPalette_Bulbasaur, gMonShinyPalette_Bulbasaur)

...which I feel is overly verbose for labels that whose are consistent across all mon. You'll always have the gMonPalette_ and gMonShinyPalette_ prefixes, so there's no doubt what the full name would be based on this. The only 2 cases where Shiny vs Regular palettes are treated differently is with Minior and Alcremie, and those are already handled by their macros.

An extra line or two is hardly that verbose or worth sacrificing variable integrity. And besides, the point of these structs is to show all the relevant information regarding that mon, no? Obfuscating some of it away in the name of neatness doesn't seem very intuitive. The fact that it's not immediately obvious that the shiny palette data is in the PALETTE macro proves my point.

I think this has been considered and then discarded but I can't remember why, so I'm bringing it up here and somebody can re-explain why it doesn't work 😅

Could we have FRONT_PIC(INCBIN_U32("path/to/bulbasaur.4bpp"), ...), and another macro for SHARED_FRONT_PIC which references a variable? I think almost all Pokémon don't share their front or back pics, so it could be a bit like Pokédex descriptions where we explicitly float those shared ones to the top. That would also mean that we reference far fewer variables, so it would be fine to just write gMonFrontPic_Alcremie, the one time in ALCREMIE_REGULAR_SPECIES_INFO rather than have the SHARED_FRONT_PIC macro token paste?

Although now I write this out, I wonder what FRONT_PIC's macro is even buying because we could just .frontPic = INCBIN_U32("path/to/bulbasaur.4bpp"), .palette = INCBIN_U16("path/to/bulbasaur.gbapal"), .shinyPalette = INCBIN_U16("path/to/bulbasaur_shiny.gbapal"). But I'm certain this was discussed and decided-against 🤔

@AsparagusEduardo
Copy link
Collaborator Author

I think this has been considered and then discarded but I can't remember why, so I'm bringing it up here and somebody can re-explain why it doesn't work 😅

Could we have FRONT_PIC(INCBIN_U32("path/to/bulbasaur.4bpp"), ...), and another macro for SHARED_FRONT_PIC which references a variable? I think almost all Pokémon don't share their front or back pics, so it could be a bit like Pokédex descriptions where we explicitly float those shared ones to the top. That would also mean that we reference far fewer variables, so it would be fine to just write gMonFrontPic_Alcremie, the one time in ALCREMIE_REGULAR_SPECIES_INFO rather than have the SHARED_FRONT_PIC macro token paste?

Although now I write this out, I wonder what FRONT_PIC's macro is even buying because we could just .frontPic = INCBIN_U32("path/to/bulbasaur.4bpp"), .palette = INCBIN_U16("path/to/bulbasaur.gbapal"), .shinyPalette = INCBIN_U16("path/to/bulbasaur_shiny.gbapal"). But I'm certain this was discussed and decided-against 🤔

I think now it's more feasible now than before after the dex text change, so I'd be willing to try :)

Though I wouldn't want to do it for this PR.

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