Rangi42 / polishedcrystal

An upgrade to Pokémon Crystal. Brings features and content up to date, and adds some original content.
https://hax.iimarckus.org/topic/6874/
1.03k stars 197 forks source link

Summary screen #978

Open emilyploszaj opened 1 month ago

emilyploszaj commented 1 month ago

Replaces the old stats screen with the new and improved summary screen. Mostly functional, but slightly incomplete.

https://github.com/user-attachments/assets/62a286aa-0d28-42e3-9dca-b1c4f700e1cd

I would not consider this ready to merge and be played (the way 9bit has an active playerbase) for mostly visual issues. It may be reasonable to have a short lived summary screen branch off of 9bit on this main repo so that multiple people can contribute to knocking out the last couple issues and get this out the door.

Code has been documented to the best of my ability when I was writing it, but it's been written over so many months at this point that multiple authors may as well have had their hands in it. Implementation ideas and reasons have been discussed over several long conversations that at this point it is probably easier to simply respond to questions than try to explain everything.

Outstanding TODOs in the summary screen file, copied for posterity

This is certainly not comprehensive, the summary screen has some artifacting and non-perfect transitions that would be good to polish over as well.

Some things I'd like to further do or seen done for the summary screen code wise that aren't necessarily functional

I'm sure there's more but I've lost it all in my head over the months, if I think of anything I'll make edits

Rangi42 commented 1 month ago

Emi, this already looks great. Thank you for putting all your work into it! ^^

Rangi42 commented 1 month ago

Also, I'd be happy to merge this as just a summary screen replacement. Meaning that the Moves screen and the move-learning page could stay, no need to implement move swapping in this for the initial merge. (Same goes for the "A/Info" functionality. ...Oh, you already did that! :D )

A couple of graphic observations:

The original mockups for comparison:

image

image

image

Rangi42 commented 1 month ago

I definitely agree about using named constants in general. I haven't reviewed the "meat" of this PR yet so can't comment on how many magic numbers there are. But yeah, you can see how other engine files define constants for meaningful tile IDs.