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

Chance to update CGB tile attribute outside proper times #65

Closed
ShenMansellPersonal opened this issue Feb 11, 2024 · 4 comments
Closed

Comments

@ShenMansellPersonal
Copy link

set_bkg_tile_xy(0x1F & (x + scroll_offset_x), 0x1F & (y + scroll_offset_y), *c);

This line should be replaced with
SetTile (id, replacement);

which will check STAT_REG & 2 before writing.

This is similar to the call directly above to write the tile data

@Zal0
Copy link
Owner

Zal0 commented Feb 11, 2024

Yes, you are right
I haven't changed this because to be honest I am not sure why SetTile is sometimes faster than set_bkg_tile_xy which is the function I wanted to use because it is the one that comes with GBDK
You are mentioning "proper times" in the description, what exactly do you mean?

@ShenMansellPersonal
Copy link
Author

by outside proper times I meant while VRAM shouldn't be written.

This can cause failures where the attribute data doesn't get written, and will also throw an exception in bgb or emulicious if they are enabled

SetTile is doing checks to avoid this by looping on STAT_REG & 2 before writing.

@Zal0
Copy link
Owner

Zal0 commented Feb 11, 2024

As far as i know set_bkg_tiles should be doing this too
The only reason why wrote SetTile back on the day is because set_bkg_tile_xy didn't exist. We have set_bkg_tiles which was slower because it expected an rect of tiles as entry param instead of a single tile
When set_bkg_tile_xy was added I did a little profiling and sometimes SetTile was still faster than set_bkg_tile_xy so I didn't change it

Anyways it should be either set_bkg_tile_xy in both calls or SetTile but not a mix so I'll change it

@Zal0
Copy link
Owner

Zal0 commented Feb 11, 2024

that fix from Toxa should fix the issue

@Zal0 Zal0 closed this as completed Feb 11, 2024
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

No branches or pull requests

2 participants