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

Competitive-formatted parties #3545

Merged
merged 15 commits into from
Apr 6, 2024

Conversation

mrgriffin
Copy link
Collaborator

@mrgriffin mrgriffin commented Nov 9, 2023

1.9.0 Changelog note: Warn users to not accept the incoming src/data/trainers.h/src/data/trainer_parties.h/src/data/battle_partners.h/src/data/partner_parties.h changes and instead use the migration tool to keep their custom changes.

This format predates Smogon, so "competitive-formatted parties" rather than "Smogon-formatted parties".

Introduces trainerproc, a tool which converts Competitive-formatted parties into Trainer Control-formatted parties.

Also features convert_party.py which can be used as python3 migration_scripts/convert_parties.py src/data/trainers.h src/data/trainer_parties.h src/data/trainers.party to convert Trainer Control-formatted parties into Competitive-formatted parties. This is what I used to produce src/data/trainers.party.
From Discord:

You're just supposed to solve the conflict as keeping your version of the file, and then run the migration script from that state. (git checkout --ours -- src/data/trainers.h maybe?)

So ig the steps are:

# During the merge
git checkout --ours -- src/data/trainers.h src/data/trainer_parties.h
python3 migration_scripts/convert_parties.py src/data/trainers.h src/data/trainer_parties.h src/data/trainers.party

But please test this!

Before:

static const struct TrainerMon sTestParty1[] =
{
    {
        .species = SPECIES_WOBBUFFET,
        .ball = ITEM_MASTER_BALL,
        .ability = ABILITY_TELEPATHY,
        .friendship = 42,
        .gender = TRAINER_MON_FEMALE,
        .heldItem = ITEM_ASSAULT_VEST,
        .isShiny = TRUE,
        .iv = TRAINER_PARTY_IVS(25,26,27,28,29,30),
        .ev = TRAINER_PARTY_EVS(252, 0, 0, 252, 4, 0),
        .lvl = 67,
        .moves = {MOVE_AIR_SLASH, MOVE_BARRIER, MOVE_SOLAR_BEAM, MOVE_EXPLOSION},
        .nature = NATURE_HASTY,
        .nickname = COMPOUND_STRING("Bubbles")
    },
    {
        .species = SPECIES_WOBBUFFET,
        .ability = ABILITY_SHADOW_TAG,
        .lvl = 5,
    },
};

static const struct Trainer sTestTrainer1 =
{
    .trainerName = _("Test1"),
    .party = TRAINER_PARTY(sTestParty1),
};

After:

=== 0 ===
Name: Test1

Bubbles (Wobbuffet) (F) @ Assault Vest
Hasty Nature
Level: 67
Ability: Telepathy
IVs: 25 HP / 26 Atk / 27 Def / 29 SpA / 30 SpD / 28 Spe
EVs: 252 HP / 4 SpA / 252 Spe
Happiness: 42
Shiny: Yes
Ball: Master Ball
- Air Slash
- Barrier
- Solar Beam
- Explosion

Wobbuffet
Level: 5
Ability: Shadow Tag

Backwards compatibility
In order to let people keep using src/data/trainers.h and src/data/trainer_parties.h I have introduced a COMPETITIVE_PARTY_SYNTAX config into include/config.h. If set to TRUE (the default) we use src/data/trainers.party. If set to FALSE we use src/data/trainers.h and src/data/trainer_parties.h.

To make this work, this branch keeps src/data/trainers.h and src/data/trainer_parties.h even though if they're generated they shouldn't be part of the repository.

Change Log:
In addition to the migration script described above:

  • #pragma trainerproc ivs explicit — requires an IVs: line for every Pokémon instead of defaulting to 31s.
  • #pragma trainerproc ivs <IVs> — changes the default IVs.
  • #pragma trainerproc level explicit — requires a Level: line for every Pokémon instead of defaulting to 100.
  • #pragma trainerproc level <level> — changes the default level.
  • During the merge you could choose to keep your original src/data/trainers.h and src/data/trainer_parties.h and then run the conversion script to update the src/data/trainers.party, make, then check in all three files. Alternatively don't run the conversion script and turn off COMPETITIVE_PARTY_SYNTAX in include/config.h.
  • Comments are // or /* ... */ like in C.
  • You're allowed to write SPECIES_PIKACHU instead of Pikachu (etc etc) if you want.

Future work

  • Support Hidden Power: to set IVs appropriate for a particular Hidden Power type.
  • Consider case-insensitivity in the parser.

@AsparagusEduardo AsparagusEduardo added this to the 1.8.0 milestone Nov 9, 2023
@mrgriffin mrgriffin marked this pull request as draft November 9, 2023 09:34
@mrgriffin mrgriffin marked this pull request as ready for review November 9, 2023 10:28
@mrgriffin mrgriffin marked this pull request as draft November 9, 2023 10:53
@mrgriffin mrgriffin marked this pull request as ready for review November 9, 2023 11:21
@mrgriffin
Copy link
Collaborator Author

Rebased, conflicts fixed, squashed, ready for review :)

NOTE: This contains the ability to specify Dynamax Level, Gigantamax, and Tera Type. The first two will work once #3438 is merged, and the latter once #3011 is merged.

@mrgriffin mrgriffin mentioned this pull request Nov 10, 2023
27 tasks
@Jaizu
Copy link

Jaizu commented Nov 11, 2023

Can this be done for facility pokémon too?

@AsparagusEduardo
Copy link
Collaborator

Can this be done for facility pokémon too?

There's this open issue for that #3484

@mrgriffin mrgriffin changed the title Smogon-formatted parties Competitive-formatted parties Nov 12, 2023
@mrgriffin
Copy link
Collaborator Author

mrgriffin commented Nov 13, 2023

Added a cpp pre-pass before trainerproc runs, meaning that C-style (// and /* ... */) comments and #defines are now supported.

Please let me rebase this before merging.

EDIT: I have rebased.

@mrgriffin mrgriffin marked this pull request as draft November 17, 2023 10:31
@mrgriffin
Copy link
Collaborator Author

mrgriffin commented Nov 17, 2023

I'm parking this because it doesn't feel like it will be possible to please enough people to get it merged, and I'm not motivated to spend any more time on it.

If somebody picks this back up:

  • trainers.party doesn't match the vanilla trainers. The conversion script needs to work out what battle_main.c would compute for the PID so that it can work out what to explicitly set Gender and Nature to. These need to be set explicitly, because the Competitive format has different defaults—implicit Gender means random, and implicit Nature means a neutral nature. This also affects any users with customized trainers, so even if Vanilla trainers' Pokémon have wrong PIDs/Natures #3641 explicitly specified the natures and genders we'd probably still want to implement the hash function in the conversion script (the user is out of luck if they've also modified the hash function or added any members to their struct [because that would affect the hash]).
  • Related to the above, should all the moves be explicitly vanilla? Probably not, because it makes sense to be dynamic based on the learnset. Empty moves doesn't mean anything in Competitive format (it means no moves, but you can't start a battle with no moves), so it's not like we're changing the semantics there—we're just giving semantics where previously there were none.
  • The idea of #define All(x) x HP / x Atk / x Def / x SpA / x SpD / x Spe is floating around. Personally I think it doesn't belong in Expansion, but that's an option for users to pursue to make IVs: All(0) more terse.
  • It probably makes sense to automatically convert unknown members in convert_parties.py, and automatically preserve unknown attributes in trainerproc/main.c. This would mean that people can migrate things like mugshots without touching convert_parties.py and trainerproc/main.c, at the cost of having slightly uglier .party files than if they'd written a nice syntax. e.g. Transition: Mugshot rather than HasCustomTransition: Yes plus Transition: TRANSITION_MUGSHOT. We should hide this feature behind a #pragma.
  • On the above, note that technically we could support all the possible trainer features in trainerproc even if Expansion doesn't have support for them, it'll just fail when GCC reaches data.c because we'll produce initializers for members that don't exist. We have this for #pragma trainerproc mugshot ShinyDragonHunter (but the conversion script doesn't detect it).
  • Instead of using COMPOUND_STRING and compound literals we should try to detect duplicates, and extract those to static const variables. This would mean reworking how multiple .party files work, probably to push the #includes of other .party files into the main .party file, and adding a scaninc rule to generate dependencies for it.

@mrgriffin mrgriffin force-pushed the rhh-smogon branch 2 times, most recently from 6ad2aec to 5bc3dc5 Compare January 11, 2024 10:24
@AsparagusEduardo AsparagusEduardo removed this from the 1.8.0 milestone Feb 1, 2024
@mrgriffin
Copy link
Collaborator Author

NOTE: In the context of #4169, we should set shouldDynamax and shouldTerastal based on whether Dynamax Level/Gigantamax and Tera Type are present respectively.

@mrgriffin mrgriffin force-pushed the rhh-smogon branch 3 times, most recently from 168ab1f to e8efb70 Compare February 10, 2024 18:07
@mrgriffin mrgriffin marked this pull request as ready for review February 10, 2024 18:07
@mrgriffin
Copy link
Collaborator Author

Ready for re-review.

Promoted SDH's mugshots from being a #pragma to built-in due to #4000.

An important thing that I'm unwilling to budge on is that by default (i.e. unless you use #pragma) everything in a .party file should be interpreted as it would on Showdown/how competitive players interpret it. This does mean that IVs default to 31, but note that there's a #pragma ivs to change the defaults. In the future we may decide that we also want #pragmas for things like a random Nature, or Dynamax Level.

This feature is entirely optional, COMPETITIVE_PARTY_SYNTAX turns it off. It's on by default, again something I'm unwilling to budge on, because I think it's the user-friendly choice. When people with existing projects merge they have two options: either turn off COMPETITIVE_PARTY_SYNTAX or use the provided scripts/1.8/convert_parties.py script to automatically convert their old parties into the Competitive format (or convert manually ig, but...).

trainerproc can output trainers for any array, so it'd support #3484 with only a few tweaks compared to what would be done today.

Note that we run .party files through cpp so that means: //- and /* ... */-style comments are supported, and #define is supported too. If people don't like writing out all the stats having the same value then something like #define All(x) x HP / x Atk / x Def / x SpA / x SpD / x Spe is possible so you can write IVs: All(0). I've not included All by default, and I don't think we should explicitly document it because I think it's best not to encourage people to fill their files with non-standard syntax... but it's available for people who really want it. #include/#if/etc etc are also all supported, but the same "should not promote" applies to those imo.

@mrgriffin
Copy link
Collaborator Author

Addressed some of Alex's comments from Discord—specifically, IVs now default to 31s and levels now default to 100. You can opt-out of both of these defaults with #pragmas as documented in the description of this PR (#pragma trainerproc ivs explicit/#pragma trainerproc level explicit).

$ python3 migration_scripts/convert_parties.py src/data/trainers.h src/data/trainer_parties.h src/data/npc_trainers.party
Is available to convert Trainer Control-formatted trainers/parties into
Competitive-formatted ones.

Multiple '#include's can be placed in the trainer section of src/data.c
to support spreading the trainers across multiple .party files.

trainerproc does not interpret the values, leaving that job to the C
compiler, so we use '#line' to associate those errors with the lines in
the .party file(s). Because the columns don't make sense we use
-fno-show-column and -fno-diagostics-show-caret. We might want to move
gTrainers into its own file so that the rest of src/data.c isn't
affected by those flags.

Extensions (misfeatures, imo):
- .party files are passed through cpp, so '#define's are supported, and so
  are '// ...' and '/* ... */' comments.
- .party files also support writing, e.g. 'SPECIES_PIKACHU' instead of
  'Pikachu'. This allows people to write constants explicitly if they
  like.

Pragmas:
- '#pragma trainerproc ivs explicit' requires an explicit 'IVs:' line
  rather than defaulting to 31s.
- '#pragma trainerproc ivs <IVs>' changes the default IVs.
- '#pragma trainerproc level explicit' requires an explicit 'Level:'
  line rather than defaulting to 100.
- '#pragma trainerproc level <level>' changes the default level.
@mrgriffin
Copy link
Collaborator Author

Added support for Starting Status too! So many new things 😅

include/config.h Outdated Show resolved Hide resolved
@mrgriffin
Copy link
Collaborator Author

btw if/when this gets accepted, please squash the commits and just keep the message from the first one :)

Copy link
Collaborator

@AsparagusEduardo AsparagusEduardo left a comment

Choose a reason for hiding this comment

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

I wonder if it'd be better that we add defines for aliases so that we assist the form name handling 🤔

migration_scripts/convert_parties.py Outdated Show resolved Hide resolved
migration_scripts/convert_parties.py Outdated Show resolved Hide resolved
tools/trainerproc/main.c Show resolved Hide resolved
tools/trainerproc/main.c Outdated Show resolved Hide resolved
tools/trainerproc/main.c Outdated Show resolved Hide resolved
tools/trainerproc/main.c Show resolved Hide resolved
tools/trainerproc/main.c Show resolved Hide resolved
tools/trainerproc/main.c Outdated Show resolved Hide resolved
migration_scripts/convert_parties.py Show resolved Hide resolved
tools/trainerproc/main.c Outdated Show resolved Hide resolved
@AsparagusEduardo AsparagusEduardo merged commit 8bd5ac2 into rh-hideout:upcoming Apr 6, 2024
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.

4 participants