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 overworld sprites #4728

Closed

Conversation

fdeblasio
Copy link

Description

Consolidates data about overworld sprites into fewer files so that it is easier to add new ones

Feature(s) this PR does NOT handle:

Nothing is left out intentionally, but I've only added a new overworld sprite a while ago so I may be missing some files that can be refactored

Discord contact info

Frankfurter0

@mrgriffin
Copy link
Collaborator

Does this impact Porymap? iirc in the past that was the blocker with unifying the graphics... Porymap wouldn't be able to find any of them and so the "Events" tab became very hard to work with. But maybe Porymap's project config features support repointing which files it reads now? Or maybe it doesn't even read any of the affected files? (I can't check, I'm stuck on an older Porymap because I'm stuck on an older repo.)

@fdeblasio
Copy link
Author

Does this impact Porymap? iirc in the past that was the blocker with unifying the graphics... Porymap wouldn't be able to find any of them and so the "Events" tab became very hard to work with. But maybe Porymap's project config features support repointing which files it reads now? Or maybe it doesn't even read any of the affected files? (I can't check, I'm stuck on an older Porymap because I'm stuck on an older repo.)

Ah good catch. I just checked and all of the sprites are just showing up as the generic N square

Also agbcc doesn't seem to like these changes either

@Bassoonian
Copy link
Collaborator

I'm fairly certain that both Porymap compatibility and agbcc support are absolute requirements, unfortunately, so unless you can get a future version of Porymap to support your refactor and have agbcc not complain, this PR won't be making it in.

@fdeblasio
Copy link
Author

I'm fairly certain that both Porymap compatibility and agbcc support are absolute requirements, unfortunately, so unless you can get a future version of Porymap to support your refactor and have agbcc not complain, this PR won't be making it in.

Makes sense! I'm closing this until I can get those working

@fdeblasio fdeblasio closed this Jun 7, 2024
@fdeblasio
Copy link
Author

With agbcc being deprecated soon, that removes one hurdle for this PR, I'll still need to figure out how to get this to work with PoryMap before I reopen this though

@ShinyDragonHunter
Copy link

Leaving a bit of feedback in case this gets reopened again.

If you're changing the object event graphics info pointer data, instead of a compound literal of the info, just change the data type of gObjectEventGraphicsInfoPointers so it's an array of struct ObjectEventGraphicsInfo data instead of an array of pointers.

Not that it matters much but you could gain a bit of space and it would be faster.

You could also make use of ascending_frames for normal object event graphics. One thing I learned is that if images aren't compressed, they can be build together.

const u32 gObjectEventPic_BrendanNormal[] = INCBIN_U32("graphics/object_events/pics/people/brendan/walking.4bpp",
                                                       "graphics/object_events/pics/people/brendan/running.4bpp");

I have this for Brendan's walking frames for example.

static const struct SpriteFrameImage sPicTable_BrendanNormal[] = {
    overworld_ascending_frames(gObjectEventPic_BrendanNormal, 2, 4),
};

And the pic table.

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