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

Partner Battle refactor #3592

Merged
merged 12 commits into from
Dec 20, 2023

Conversation

AlexOn1ine
Copy link
Collaborator

@AlexOn1ine AlexOn1ine commented Nov 22, 2023

Based on PR #3234 so full credit to ShinyDragonHunter.

The PR decouples partner trainers from gTrainers and adds new files to make it more convenient to create new trainer parties for a partner in a multi battle. Alongside I adjusted the Steven multi battle which can now serve as an example for new partners.

Possible problem: Partner pokemon have different IDs. I'm not sure if this is something we need to care about.

ids_may

Also I've kept in some debug scripts for now. They can be removed once the PR is ready to be merged.

@ShinyDragonHunter
Copy link

ShinyDragonHunter commented Nov 22, 2023

When checking for the partner, you need to do "TRAINER_PARTNER(partner)".

For example: with partner Steven, you'd need check for "TRAINER_PARTNER(PARTNER_STEVEN)".

I had plans to do it so the otId is handled in the partner array itself which would be the reusing of fields for a trainer that wouldn't make sense for a partner. Can partners use items like Potions for example?

@AlexOn1ine
Copy link
Collaborator Author

When checking for the partner, you need to do "TRAINER_PARTNER(partner)".

Oops, right. That solved the issue with the ID number for steven.

Can partners use items like Potions for example?

No, I don't think so.

@ShinyDragonHunter
Copy link

No, I don't think so.

Then that data could be repurposed for the otId of the partner's Pokémon imo.

How I initially set it up was that any partner that wasn't Steven has a random otId and then that would be passed for Mon creation. One thing I couldn't work out was handling shininess.

How trainer control did it was change the otId of the Pokémon so the current personality would be that of a shiny. This doesn't quite work for partners and I'd like to think there's a way to make a mon shiny whilst preserving gender, nature and otId

@AlexOn1ine
Copy link
Collaborator Author

Then that data could be repurposed for the otId of the partner's Pokémon imo.

Yeah, makes sense. I'll see what others think and if it's an issue we should care about but the change can be made.

@Bassoonian
Copy link
Collaborator

I think being able to provide an ID is good - if you team up with the same NPC multiple times throughout the story, you'd expect their otID to at least be consistent. I don't know what the best implementation is - an actual value of the otID in the Trainer struct, or an ID that refers to a table so that multiple trainers can refer to the same ID - but I think it'd be a very good addition anyway.

@ShinyDragonHunter
Copy link

I feel repurposing the fields that don't get used for the partner makes the most sense and it would be easier for a user to be able to edit and tinker with it. From what I've seen, trainer items is 8 bytes (16 bits * MAX_TRAINER_ITEMS). So reusing 32 bits of that is more than enough. You'd also have 4 bytes remaining so there's a bit of room for more unique stuff for the partner. I feel you could add a union to "struct Trainer" for this.

That said though, setting the otId is redundant when adding a shiny to the party roster as the trainer control stuff rewrites the otId to be the shiny value when shiny. This overwrites what you may have set for the otId previously. So shiny Pokémon for trainers or partners have a completely different otId. I feel this needs to be addressed and a alternate method for making the Pokémon shiny whilst preserving mon gender, nature and otId needs to be added for this

@Bassoonian
Copy link
Collaborator

I agree that the shiny generation needs to be revisited, but that shouldn't be a blocking issue for this PR. I'd however argue that being able to specify the otID in all other cases should be incorporated from the get-go.

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.

Need to make sure to remove everything about the hypothetical May partner is removed too when this is ready for a merge besides just the debug scripts

include/constants/opponents.h Outdated Show resolved Hide resolved
src/battle_controller_player_partner.c Outdated Show resolved Hide resolved
src/battle_tower.c Outdated Show resolved Hide resolved
src/battle_tower.c Outdated Show resolved Hide resolved
src/data/battle_partners.h Outdated Show resolved Hide resolved
src/pokemon.c Outdated Show resolved Hide resolved
@AlexOn1ine
Copy link
Collaborator Author

I've applied all reviews but still didn't touch the id part. I tried some things but didn't like what I was doing. Maybe SBird / MGriffin could chime in? 👀

@ShinyDragonHunter
Copy link

I've applied all reviews but still didn't touch the id part. I tried some things but didn't like what I was doing. Maybe SBird / MGriffin could chime in? 👀

What was it you tried?

@ShinyDragonHunter
Copy link

otID ^= GET_SHINY_VALUE(otID, personality) << 16; // make shiny by xoring SID

Merrp came up with this. It xors the SID to make the mon shiny whilst also being able to preserve, nature, gender and the otId a player can see. This is actually setting the SID to the shiny value but because it's hidden from the player, I feel this okay. I've tested it and it works as intended
pokeemerald-2-2
pokeemerald-1-3

#define STEVEN_OTID 61226

static const struct TrainerMon sParty_Steven[] = {
    {
    .iv = TRAINER_PARTY_IVS(31, 31, 31, 31, 31, 31),
    .lvl = 77,
    .species = SPECIES_SKARMORY,
    .heldItem = ITEM_NONE,
    .moves = {MOVE_TOXIC, MOVE_AERIAL_ACE, MOVE_SPIKES, MOVE_STEEL_WING},
    .isShiny = TRUE,
    .gender = TRAINER_MON_FEMALE,
    .nature = TRAINER_PARTY_NATURE(NATURE_LONELY),
    },

For testing, I used Steven's Meteor Falls roster

@AlexOn1ine
Copy link
Collaborator Author

Added unique partner Ids
ids_may

Credits to SDH

@ShinyDragonHunter
Copy link

#define TRAINER_TYPE(type, ...) { .type =  { __VA_ARGS__ } }

// Macros used for specific trainer types
#define TRAINER_ITEMS(...) TRAINER_TYPE(trainer, __VA_ARGS__)
#define PARTNER_OTID(...) TRAINER_TYPE(partner, __VA_ARGS__)

Do these macros work for you?
For partners, it'd be:

    [PARTNER_STEVEN] =
    {
        .trainerClass = TRAINER_CLASS_RIVAL,
        .encounterMusic_gender = TRAINER_ENCOUNTER_MUSIC_MALE,
        .trainerPic = TRAINER_BACK_PIC_STEVEN,
        .trainerName = gText_Steven,
        .trainerType = PARTNER_OTID(.otId = STEVEN_OTID),
        .doubleBattle = FALSE,
        .aiFlags = 0, // TODO: allow partners to use their own AI flags
        .party = TRAINER_PARTY(sParty_StevenPartner),
    },

And for trainers, it'd be:

    [TRAINER_SAWYER_1] =
    {
        .trainerClass = TRAINER_CLASS_HIKER,
        .encounterMusic_gender = TRAINER_ENCOUNTER_MUSIC_HIKER,
        .trainerPic = TRAINER_PIC_HIKER,
        .trainerName = gTrainerNameString_Sawyer,
        .trainerType = TRAINER_ITEMS(.items = {}),
        .doubleBattle = FALSE,
        .aiFlags = AI_SCRIPT_CHECK_BAD_MOVE | AI_SCRIPT_TRY_TO_FAINT | AI_SCRIPT_CHECK_VIABILITY,
        .party = TRAINER_PARTY(sParty_Sawyer1),
    },

@AlexOn1ine
Copy link
Collaborator Author

I got the shiny stuff and macro to work. Credits to SDH.

Ready for re-review.

@AlexOn1ine AlexOn1ine force-pushed the multi_battle_refactor branch from a4e229b to fe1bad6 Compare December 19, 2023 22:03
@AlexOn1ine
Copy link
Collaborator Author

Ready for re-review. I force-pushed because I reverted changes.

Maybe there is a better idea for the loop that does the Id thing. Suggestions are welcome.

include/config.h Outdated Show resolved Hide resolved
data/scripts/debug.inc Outdated Show resolved Hide resolved
src/data/battle_partners.h Outdated Show resolved Hide resolved
src/data/partner_parties.h Outdated Show resolved Hide resolved
include/event_scripts.h Outdated Show resolved Hide resolved
@AlexOn1ine
Copy link
Collaborator Author

AlexOn1ine commented Dec 19, 2023

review changes applied.

src/data/battle_partners.h Outdated Show resolved Hide resolved
include/constants/battle_partner.h Outdated Show resolved Hide resolved
include/constants/opponents.h Outdated Show resolved Hide resolved
src/battle_tower.c Outdated Show resolved Hide resolved
@Bassoonian Bassoonian merged commit 1e25b53 into rh-hideout:upcoming Dec 20, 2023
1 check passed
PCG06 pushed a commit to PCG06/pokeemerald that referenced this pull request Dec 22, 2023
* Partner Battle refactor

* fix for steven id

* clean up

* Use trainer partner names for id

* removed testing leftover

* comment change

* more review changes

* fix compiling

* remove partener count

---------

Co-authored-by: Bassoonian <iasperbassoonian@gmail.com>
PCG06 pushed a commit to PCG06/pokeemerald that referenced this pull request Dec 22, 2023
* Partner Battle refactor

* fix for steven id

* clean up

* Use trainer partner names for id

* removed testing leftover

* comment change

* more review changes

* fix compiling

* remove partener count

---------

Co-authored-by: Bassoonian <iasperbassoonian@gmail.com>
@AlexOn1ine AlexOn1ine deleted the multi_battle_refactor branch April 19, 2024 12:31
Pawkkie pushed a commit to Pawkkie/pokeemerald-expansion that referenced this pull request May 16, 2024
* Partner Battle refactor

* fix for steven id

* clean up

* Use trainer partner names for id

* removed testing leftover

* comment change

* more review changes

* fix compiling

* remove partener count

---------

Co-authored-by: Bassoonian <iasperbassoonian@gmail.com>
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