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

Refactor battle terrain #4891

Open
wants to merge 10 commits into
base: upcoming
Choose a base branch
from

Conversation

fdeblasio
Copy link

Description

Consolidates a lot of BATTLE_TERRAIN properties into a new gBattleTerrainInfo

Feature(s) this PR does NOT handle:

The intro slides in battle_intro.c (I couldn't for the life of me figure out how to get it to work)

Discord contact info

Frankfurter0

@agsmgmaster64
Copy link

Any way to include the FRLG handling for the other battle terrain backgrounds to simplify adding/editing terrains further?

@ShinyDragonHunter
Copy link

If the battle terrain names are only for the sprite visualiser menu, they don't need to be in the battle terrain info struct imo

@ShinyDragonHunter
Copy link

I'd also recommend moving the battle terrain graphics themselves to where the battle terrain info table is for convenience

@fdeblasio
Copy link
Author

If the battle terrain names are only for the sprite visualiser menu, they don't need to be in the battle terrain info struct imo

I'd also recommend moving the battle terrain graphics themselves to where the battle terrain info table is for convenience

Done and done!

@Bassoonian
Copy link
Collaborator

Bassoonian commented Jul 13, 2024

I think terrain needs to be renamed to environment to avoid confusing it with the actual functional battle terrains (Electric Terrain etc). I would personally put this PR on hold, this this is a PR that needs to go to pret first imo. I'll PR it to pret and keep you up to date :)

EDIT: pret#2014

@hedara90 hedara90 added the category: battle-mechanic Pertains to battle mechanics label Jul 26, 2024
@hedara90 hedara90 added the status: on hold delayed until some decision later label Aug 8, 2024
@pkmnsnfrn
Copy link
Collaborator

@fdeblasio I don't think it NEEDS to be part of this PR (but it can be)

Should you also take @agsmgmaster64's suggestions to make it easier to add battle backgrounds using the FRLG method? Josh already ha a tutorial:
https://github.com/pret/pokeemerald/wiki/Improve-the-Loading-of-Battle-Terrain

I was going to do it, but your work conflicts a little and clearly you have a slightly better handle on the system

@fdeblasio
Copy link
Author

fdeblasio commented Aug 27, 2024

@fdeblasio I don't think it NEEDS to be part of this PR (but it can be)

Should you also take @agsmgmaster64's suggestions to make it easier to add battle backgrounds using the FRLG method? Josh already ha a tutorial: https://github.com/pret/pokeemerald/wiki/Improve-the-Loading-of-Battle-Terrain

I was going to do it, but your work conflicts a little and clearly you have a slightly better handle on the system

Done: bba5fe1

@ShinyDragonHunter
Copy link

Not sure if out of scope for this PR but you could potentially separate the palette for the entry anim and add the pointer to it to the battle terrain table? Would make battle terrain data in the table larger but it could allow for some interesting combinations like grassing sliding across the screen in cave instead of rocks.

The entry anim graphics and tilemaps themselves are already separate

@AsparagusEduardo
Copy link
Collaborator

I don't know if you've been checking, but this PR hasn't been compiling since the last commit made to it.

@AlexOn1ine
Copy link
Collaborator

I don't know if you've been checking, but this PR hasn't been compiling since the last commit made to it.

It is being held up by pret#2014

@AsparagusEduardo
Copy link
Collaborator

It is being held up by pret#2014

Ooooh, ok

@fdeblasio
Copy link
Author

I don't know if you've been checking, but this PR hasn't been compiling since the last commit made to it.

Oops, I haven't really been active here in a bit. It's building locally for me so I'm not sure what's breaking the tests here. I can always revert that commit and make it a seperate PR after this one gets merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: battle-mechanic Pertains to battle mechanics status: on hold delayed until some decision later
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants