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.

Sign up for GitHub

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

Consolidate type properties #4185

Merged
merged 13 commits into from
Mar 12, 2024
Merged

Consolidate type properties #4185

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
898e754
Moved gTypeNames into gTypes
fdeblasio Feb 13, 2024
a701486
Added invalid move text to struct
fdeblasio Feb 13, 2024
4d9c381
Added max move to struct
fdeblasio Feb 13, 2024
8d10ac7
Added icon palette to struct
fdeblasio Feb 13, 2024
545daef
Added macros for invalid and max moves
fdeblasio Feb 13, 2024
c1c6de3
Swapped palette and max move order
fdeblasio Feb 13, 2024
b9db91a
Renamed invalid to generic
fdeblasio Feb 13, 2024
13c0896
Renamed invalid to generic in struct definition
fdeblasio Feb 13, 2024
ac2b0d4
Added zMoves and items to type struct
fdeblasio Feb 14, 2024
259d2f7
Addressed comments
fdeblasio Feb 19, 2024
920887f
Incorporated newer comments
fdeblasio Feb 19, 2024
46c0047
Merge branch 'upcoming' into consolidateTypes
fdeblasio Feb 28, 2024
d8e01a1
Updated comment format
fdeblasio Feb 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion include/battle_main.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ extern struct MultiPartnerMenuPokemon gMultiPartnerParty[MULTI_PARTY_SIZE];
extern const struct SpriteTemplate gUnusedBattleInitSprite;
extern const struct OamData gOamData_BattleSpriteOpponentSide;
extern const struct OamData gOamData_BattleSpritePlayerSide;
extern const u8 gTypeNames[NUMBER_OF_MON_TYPES][TYPE_NAME_LENGTH + 1];
extern const struct Type gTypes[NUMBER_OF_MON_TYPES];

extern const u8 gStatusConditionString_PoisonJpn[8];
extern const u8 gStatusConditionString_SleepJpn[8];
Expand Down
16 changes: 16 additions & 0 deletions include/data.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,22 @@ struct TrainerClass

#define TRAINER_ENCOUNTER_MUSIC(trainer)((gTrainers[trainer].encounterMusic_gender & 0x7F))

struct Type
{
u8 name[TYPE_NAME_LENGTH + 1];
u8 generic[17];
u8 palette;
u16 zMove;
u16 maxMove;
u16 enhanceItem;
u16 berry;
u16 gem;
u16 plate;
u16 memory;
u16 zCrystal;
u16 teraShard;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm, while useful as a reference for items associated to new types, the item fields aren't being used anywere in the code. Maybe adding them as comments as a reminder would be better if you're not planning in adding changes to the battle code.

Copy link
Collaborator

Choose a reason for hiding this comment

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

If this PR doesn't make any changes to the battle code (which I'm fine with) can that be captured in a later issue?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't feel confortable introducing fields and data that aren't used at all :/

Copy link
Collaborator

Choose a reason for hiding this comment

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

I have to agree that if we add this information we may as well be using it in the same PR

Copy link
Collaborator

Choose a reason for hiding this comment

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

Plus, there's also the fact that you'd be moving data away from gItemsInfo, where most item info is stored.

Copy link
Collaborator

@mrgriffin mrgriffin Feb 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I thought the idea was that some of these fields exist mostly to facilitate uses like psf's? Are there any official features which would want to know, e.g. "which Gem is associated with an arbitrary type?"

EDIT: I guess what I'm trying to say is that these could be useful even if they had zero uses throughout base Expansion.

Copy link
Collaborator

Choose a reason for hiding this comment

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

largely agree with @mrgriffin

I don't feel confortable introducing fields and data that aren't used at all :/

This has never been a principle we've had in expansion

  • There are 500 different Pikachu forms that are straight up not being used, we can poll that and I'll bet $100 on that lmao
  • we JUST merged ghoul's new field effect work that doesn't get used in the base game until the developer adds something
  • I just merged in a new function for printing mon's height that doesn't get used anywhere

Like if we want to introduce that principle, I think its a different discussion entirely, but I don't want to hold this PR to a standard that doesn't actually exist yet

Plus, there's also the fact that you'd be moving data away from gItemsInfo, where most item info is stored.

gItemsInfo doesn't appear in the PR at all, nothing is being moved out of there (unless I'm missing something).

I don't think its unreasonable or unusual to

  • introduce a QoL improvement in PR 1
  • iterate and improve on the use of that improvement in future PRs

I am trying not to unnecessarily bloat the scope of this PR.

that being said, I think everybody is happy if

  • the fields in question get commented out for now
  • this PR gets merged
  • a future PR uncomments them and makes changes to the battle code to use them

Copy link
Collaborator

Choose a reason for hiding this comment

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

  • There are 500 different Pikachu forms that are straight up not being used, we can poll that and I'll bet $100 on that lmao

There's a difference between having Species data ready to use for the user for PoryMap and scripts and having data available for edge cases that need to write a custom function to use that data.

  • we JUST merged ghoul's new field effect work that doesn't get used in the base game until the developer adds something

Same with this, the new function is used in scripts to set up pre-battle conditions without the need to write custom code for it.

  • I just merged in a new function for printing mon's height that doesn't get used anywhere

Which function? Are you talking about ReturnHeightStringNoWhitespace? Because that's certainly being used by the Lotad/Seedot house.

gItemsInfo doesn't appear in the PR at all, nothing is being moved out of there (unless I'm missing something).

I meant in context that if you were to use this data, you'd have to remove it from gItemsInfo to avoid reduncancy.

  • the fields in question get commented out for now
  • this PR gets merged
  • a future PR uncomments them and makes changes to the battle code to use them

Sounds good to me at least. Again, this info is useful as a reference, for adding new types, but it also needs a reason to justify adding unused data into the rom.

Copy link
Collaborator

@mrgriffin mrgriffin Feb 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I meant in context that if you were to use this data, you'd have to remove it from gItemsInfo to avoid reduncancy.

I think both have value, depending on whether you're going from type to item, or item to type; so you wouldn't want to move anything. (That's why earlier I suggested writing some quick tests that verify that if a type maps to something, the thing it maps to maps back to that type)

};

extern const u16 gMinigameDigits_Pal[];
extern const u32 gMinigameDigits_Gfx[];

Expand Down
2 changes: 1 addition & 1 deletion src/battle_controller_player.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1755,7 +1755,7 @@ static void MoveSelectionDisplayMoveType(u32 battler)
else
type = gMovesInfo[moveInfo->moves[gMoveSelectionCursor[battler]]].type;

StringCopy(txtPtr, gTypeNames[type]);
StringCopy(txtPtr, gTypes[type].name);
BattlePutTextOnWindow(gDisplayedStringBattle, B_WIN_MOVE_TYPE);
}

Expand Down
2 changes: 1 addition & 1 deletion src/battle_debug.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1480,7 +1480,7 @@ static void PrintSecondaryEntries(struct BattleDebugMenu *data)
{
u8 *types = &gBattleMons[data->battlerId].type1;

PadString(gTypeNames[types[i]], text);
PadString(gTypes[types[i]].name, text);
printer.currentY = printer.y = (i * yMultiplier) + sSecondaryListTemplate.upText_Y;
AddTextPrinter(&printer, 0, NULL);
}
Expand Down
29 changes: 3 additions & 26 deletions src/battle_dynamax.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,29 +24,6 @@

static u8 GetMaxPowerTier(u16 move);

// Constant Data
static const u16 sMaxMoveTable[NUMBER_OF_MON_TYPES] =
{
[TYPE_NORMAL] = MOVE_MAX_STRIKE,
[TYPE_FIGHTING] = MOVE_MAX_KNUCKLE,
[TYPE_FLYING] = MOVE_MAX_AIRSTREAM,
[TYPE_POISON] = MOVE_MAX_OOZE,
[TYPE_GROUND] = MOVE_MAX_QUAKE,
[TYPE_ROCK] = MOVE_MAX_ROCKFALL,
[TYPE_BUG] = MOVE_MAX_FLUTTERBY,
[TYPE_GHOST] = MOVE_MAX_PHANTASM,
[TYPE_STEEL] = MOVE_MAX_STEELSPIKE,
[TYPE_FIRE] = MOVE_MAX_FLARE,
[TYPE_WATER] = MOVE_MAX_GEYSER,
[TYPE_GRASS] = MOVE_MAX_OVERGROWTH,
[TYPE_ELECTRIC] = MOVE_MAX_LIGHTNING,
[TYPE_PSYCHIC] = MOVE_MAX_MINDSTORM,
[TYPE_ICE] = MOVE_MAX_HAILSTORM,
[TYPE_DRAGON] = MOVE_MAX_WYRMWIND,
[TYPE_DARK] = MOVE_MAX_DARKNESS,
[TYPE_FAIRY] = MOVE_MAX_STARFALL,
};

struct GMaxMove
{
u16 species;
Expand Down Expand Up @@ -305,9 +282,9 @@ static u16 GetTypeBasedMaxMove(u16 battlerId, u16 type)
}

// Regular Max Move
if (sMaxMoveTable[type] == MOVE_NONE) // failsafe
return sMaxMoveTable[0];
return sMaxMoveTable[type];
if (gTypes[type].maxMove == MOVE_NONE) // failsafe
return gTypes[0].maxMove;
return gTypes[type].maxMove;
}

// Returns the appropriate Max Move or G-Max Move for a battler to use.
Expand Down
233 changes: 213 additions & 20 deletions src/battle_main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,27 +299,220 @@ const struct OamData gOamData_BattleSpritePlayerSide =

static const s8 sCenterToCornerVecXs[8] ={-32, -16, -16, -32, -32};

const u8 gTypeNames[NUMBER_OF_MON_TYPES][TYPE_NAME_LENGTH + 1] =
#define GENERIC(type) .generic = _(#type" move")
#define MOVES(z, max) \
.zMove = MOVE_##z, \
.maxMove = MOVE_MAX_##max,

#define ITEMS(type, item, berryName, crystal) \
.enhanceItem = ITEM_##item, \
.berry = ITEM_##berryName##_BERRY, \
.gem = ITEM_##type##_GEM, \
.zCrystal = ITEM_##crystal##IUM_Z,

//Not in ITEMS since Normal doesn't have these two items (Blank Plate is not yet implemented)
#define LEGENDARY_ITEMS(plateName, type) \
.plate = ITEM_##plateName##_PLATE, \
.memory = ITEM_##type##_MEMORY, \

//Not in ITEMS since Stellar has Tera Shards but no other item
#define TERA_SHARD(type) .teraShard = ITEM_##type##_TERA_SHARD,
fdeblasio marked this conversation as resolved.
Show resolved Hide resolved

// .generic is large enough that the text for TYPE_ELECTRIC will exceed TEXT_BUFF_ARRAY_COUNT.
const struct Type gTypes[NUMBER_OF_MON_TYPES] =
fdeblasio marked this conversation as resolved.
Show resolved Hide resolved
{
[TYPE_NORMAL] = _("Normal"),
[TYPE_FIGHTING] = _("Fight"),
[TYPE_FLYING] = _("Flying"),
[TYPE_POISON] = _("Poison"),
[TYPE_GROUND] = _("Ground"),
[TYPE_ROCK] = _("Rock"),
[TYPE_BUG] = _("Bug"),
[TYPE_GHOST] = _("Ghost"),
[TYPE_STEEL] = _("Steel"),
[TYPE_MYSTERY] = _("???"),
[TYPE_FIRE] = _("Fire"),
[TYPE_WATER] = _("Water"),
[TYPE_GRASS] = _("Grass"),
[TYPE_ELECTRIC] = _("Electr"),
[TYPE_PSYCHIC] = _("Psychc"),
[TYPE_ICE] = _("Ice"),
[TYPE_DRAGON] = _("Dragon"),
[TYPE_DARK] = _("Dark"),
[TYPE_FAIRY] = _("Fairy"),
[TYPE_NORMAL] =
{
.name = _("Normal"),
GENERIC(a NORMAL),
fdeblasio marked this conversation as resolved.
Show resolved Hide resolved
.palette = 13,
MOVES(BREAKNECK_BLITZ, STRIKE)
ITEMS(NORMAL, SILK_SCARF, CHILAN, NORMAL)
TERA_SHARD(NORMAL)
},
[TYPE_FIGHTING] =
{
.name = _("Fight"),
GENERIC(a FIGHTING),
.palette = 13,
MOVES(ALL_OUT_PUMMELING, KNUCKLE)
ITEMS(FIGHTING, BLACK_BELT, CHOPLE, FIGHTIN)
LEGENDARY_ITEMS(FIST, FIGHTING)
TERA_SHARD(FIGHTING)
},
[TYPE_FLYING] =
{
.name = _("Flying"),
GENERIC(a FLYING),
.palette = 14,
MOVES(SUPERSONIC_SKYSTRIKE, AIRSTREAM)
ITEMS(FLYING, SHARP_BEAK, COBA, FLYIN)
LEGENDARY_ITEMS(SKY, FLYING)
TERA_SHARD(FLYING)
},
[TYPE_POISON] =
{
.name = _("Poison"),
GENERIC(a POISON),
.palette = 14,
MOVES(ACID_DOWNPOUR, OOZE)
ITEMS(POISON, POISON_BARB, KEBIA, POISON)
LEGENDARY_ITEMS(TOXIC, POISON)
TERA_SHARD(POISON)
},
[TYPE_GROUND] =
{
.name = _("Ground"),
GENERIC(a GROUND),
.palette = 13,
MOVES(TECTONIC_RAGE, QUAKE)
ITEMS(GROUND, SOFT_SAND, SHUCA, GROUND)
LEGENDARY_ITEMS(EARTH, GROUND)
TERA_SHARD(GROUND)
},
[TYPE_ROCK] =
{
.name = _("Rock"),
GENERIC(a ROCK),
.palette = 13,
MOVES(CONTINENTAL_CRUSH, ROCKFALL)
ITEMS(ROCK, HARD_STONE, CHARTI, ROCK)
LEGENDARY_ITEMS(STONE, ROCK)
TERA_SHARD(ROCK)
},
[TYPE_BUG] =
{
.name = _("Bug"),
GENERIC(a BUG),
.palette = 15,
MOVES(SAVAGE_SPIN_OUT, FLUTTERBY)
ITEMS(BUG, SILVER_POWDER, TANGA, BUGIN)
LEGENDARY_ITEMS(INSECT, BUG)
TERA_SHARD(BUG)
},
[TYPE_GHOST] =
{
.name = _("Ghost"),
GENERIC(a GHOST),
.palette = 14,
MOVES(NEVER_ENDING_NIGHTMARE, PHANTASM)
ITEMS(GHOST, SPELL_TAG, KASIB, GHOST)
LEGENDARY_ITEMS(SPOOKY, GHOST)
TERA_SHARD(GHOST)
},
[TYPE_STEEL] =
{
.name = _("Steel"),
GENERIC(a STEEL),
.palette = 13,
MOVES(CORKSCREW_CRASH, STEELSPIKE)
ITEMS(STEEL, METAL_COAT, BABIRI, STEEL)
LEGENDARY_ITEMS(IRON, STEEL)
TERA_SHARD(STEEL)
},
[TYPE_MYSTERY] =
{
.name = _("???"),
GENERIC(a ???),
.palette = 15,
},
[TYPE_FIRE] =
{
.name = _("Fire"),
GENERIC(a FIRE),
.palette = 13,
MOVES(INFERNO_OVERDRIVE, FLARE)
ITEMS(FIRE, CHARCOAL, OCCA, FIR)
LEGENDARY_ITEMS(FLAME, FIRE)
TERA_SHARD(FIRE)
},
[TYPE_WATER] =
{
.name = _("Water"),
GENERIC(a WATER),
.palette = 14,
MOVES(HYDRO_VORTEX, GEYSER)
ITEMS(WATER, MYSTIC_WATER, PASSHO, WATER)
LEGENDARY_ITEMS(SPLASH, WATER)
TERA_SHARD(WATER)
},
[TYPE_GRASS] =
{
.name = _("Grass"),
GENERIC(a GRASS),
.palette = 15,
MOVES(BLOOM_DOOM, OVERGROWTH)
ITEMS(GRASS, MIRACLE_SEED, RINDO, GRASS)
LEGENDARY_ITEMS(MEADOW, GRASS)
TERA_SHARD(GRASS)
},
[TYPE_ELECTRIC] =
{
.name = _("Electr"),
GENERIC(an ELECTRIC),
.palette = 13,
MOVES(GIGAVOLT_HAVOC, LIGHTNING)
ITEMS(ELECTRIC, MAGNET, WACAN, ELECTR)
LEGENDARY_ITEMS(ZAP, ELECTRIC)
TERA_SHARD(ELECTRIC)
},
[TYPE_PSYCHIC] =
{
.name = _("Psychc"),
GENERIC(a PSYCHIC),
.palette = 14,
MOVES(SHATTERED_PSYCHE, MINDSTORM)
ITEMS(PSYCHIC, TWISTED_SPOON, PAYAPA, PSYCH)
LEGENDARY_ITEMS(MIND, PSYCHIC)
TERA_SHARD(PSYCHIC)
},
[TYPE_ICE] =
{
.name = _("Ice"),
GENERIC(a ICE),
fdeblasio marked this conversation as resolved.
Show resolved Hide resolved
.palette = 14,
MOVES(SUBZERO_SLAMMER, HAILSTORM)
ITEMS(ICE, NEVER_MELT_ICE, YACHE, IC)
LEGENDARY_ITEMS(ICICLE, ICE)
TERA_SHARD(ICE)
},
[TYPE_DRAGON] =
{
.name = _("Dragon"),
GENERIC(a DRAGON),
.palette = 15,
MOVES(DEVASTATING_DRAKE, WYRMWIND)
ITEMS(DRAGON, DRAGON_FANG, HABAN, DRAGON)
LEGENDARY_ITEMS(DRACO, DRAGON)
TERA_SHARD(DRAGON)
},
[TYPE_DARK] =
{
.name = _("Dark"),
GENERIC(a DARK),
.palette = 13,
MOVES(BLACK_HOLE_ECLIPSE, DARKNESS)
ITEMS(DARK, BLACK_GLASSES, COLBUR, DARKIN)
LEGENDARY_ITEMS(DREAD, DARK)
TERA_SHARD(DARK)
},
[TYPE_FAIRY] =
{
.name = _("Fairy"),
GENERIC(a FAIRY),
.palette = 14,
MOVES(TWINKLE_TACKLE, STARFALL)
ITEMS(FAIRY, FAIRY_FEATHER, ROSELI, FAIR)
LEGENDARY_ITEMS(PIXIE, FAIRY)
TERA_SHARD(FAIRY)
},
/*
[TYPE_STELLAR] =
{
.name = _("Stellar"),
TERA_SHARD(STELLAR),
},
*/
};

// extra args are money and ball
Expand Down
Loading
Loading