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

Minor clean up, Improve tileset palette assignments #19

Merged
merged 5 commits into from
May 11, 2024
Merged

Minor clean up, Improve tileset palette assignments #19

merged 5 commits into from
May 11, 2024

Conversation

JustRegularLuna
Copy link
Contributor

@JustRegularLuna JustRegularLuna commented Jan 30, 2024

  • This stops Polished Map from breaking colors, since Polished Map automatically trims any "unnecessary" tilepal entries from the end of the file when saving your changes.
  • Switched tileset palette sets and tilepal files to both be read from a pointer table, removing the need to INCLUDE them multiple times.
  • Uses a constant for the number of tiles in a tileset, to make things easier if a person decides to expand tilesets.
  • Fixes a hardcoded bank ID in oak_speech2.asm.
  • Adds some missing asserts in easy-to-miss tables from the color code.

- This saves space by not having to include the same file 2 or 3 tiles for duplicate tilesetes.
- This also saves space by not requiring each .asm file to be padded out to the same size for smaller tilesets.
- This stops Polished Map from breaking colors, since it automatically trims any "unnecessary" tilepal entries from the end of the file when saving your changes.
- Uses a constant to make things easier if a person decides to expand tilesets.
@JustRegularLuna
Copy link
Contributor Author

Actually hold off on merging this. My edited code only does $FF tiles and the original did $100. I'll fix that up tomorrow and update the PR.

@JustRegularLuna
Copy link
Contributor Author

Ok, should be good now.

@JustRegularLuna
Copy link
Contributor Author

There were a lot more duplicate Palette Sets than tilepal assignments, so I made those use a pointer table as well.

@JustRegularLuna JustRegularLuna changed the title Clean up tileset palette assignments Minor clean up, Improve tileset palette assignments Feb 13, 2024
Copy link
Owner

@dannye dannye left a comment

Choose a reason for hiding this comment

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

Sorry it took so long. Hopefully I didn't count wrong on any of the tilepal comments.

color/tilesets/club.asm Show resolved Hide resolved
color/tilesets/reds_house.asm Show resolved Hide resolved
color/tilesets/underground.asm Show resolved Hide resolved
; $60 bytes for each tileset. Each byte is the palette number for a tile.
; Remaining $a0 tiles aren't part of the tileset and are set to 7 (text palette).
; One byte for each tile in the tileset
; Remaining tiles are set to 7 (text palette).
Copy link
Owner

Choose a reason for hiding this comment

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

This comment is a little bit ambiguous now. There are still only exactly $a0 tiles that are set to the text palette. And for tilesets which use fewer than $60 tiles, there is also now an uninitialized section between the tileset palettes and the $a0 text palettes, which is OK but not the impression that this comment gives.

color/data/map_palette_assignments.asm Show resolved Hide resolved
@@ -110,47 +103,7 @@ MapPaletteSets:
db INDOOR_LIGHT_BLUE
db ALT_TEXTBOX_PAL ; Uses variant of textbox palette for skeleton pokemon
Copy link
Owner

Choose a reason for hiding this comment

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

This comment now only applies to MuseumPalSet and not GatePalSet.

db INDOOR_YELLOW
db INDOOR_BROWN
db INDOOR_LIGHT_BLUE
db ALT_TEXTBOX_PAL ; Uses variant of textbox palette for Articuno binoculars
Copy link
Owner

Choose a reason for hiding this comment

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

(gate's reason is Articuno)

@@ -3,6 +3,8 @@ INCLUDE "color/data/map_palette_sets.asm"
INCLUDE "color/data/map_palette_assignments.asm"
INCLUDE "color/data/roofpalettes.asm"

DEF TILESET_SIZE EQU $60
Copy link
Owner

Choose a reason for hiding this comment

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

There's MAP_TILESET_SIZE but it's in engine/overworld/movement.asm oddly enough, and not a constants file.
It would be nice in pokered to move it to a constants file and then just use that constant here. But this is good until that happens.

@JustRegularLuna
Copy link
Contributor Author

The "one extra" entries in some tilesets was again because of polished map. Since it works off the gen 2 assumption that palettes are nybbles so it is trimming to an even number of tilepal entries

@dannye
Copy link
Owner

dannye commented May 9, 2024

The "one extra" entries in some tilesets was again because of polished map. Since it works off the gen 2 assumption that palettes are nybbles so it is trimming to an even number of tilepal entries

Hm, okay, that's not a big deal then. Thanks.

@JustRegularLuna
Copy link
Contributor Author

No problem. I'll double check if there's a way around that, maybe loading it with the priority tiles setting in polished map or something (pretty sure that setting is unintentionally supported by pokered-gbc anyway).

@dannye dannye merged commit d5679c5 into dannye:master May 11, 2024
1 check passed
@dannye
Copy link
Owner

dannye commented May 11, 2024

Thanks :-)

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.

2 participants