Add a guide about adding a SGB border #70
Conversation
try to make the document more grammatically correct attempt to further iterate on how some information is laid out
remove table of contents since thats handled by vue for us another grammar pass, incl. correcting typos revert tabs indent and mixed ul markers add gbdev chat invite where relevant alter some sentences to hopefully be clearer
website/guides/sgb_border.md
Outdated
| 9. [Credits](#credits) | ||
| This document was written to aid developers of DMG compatibile homebrew with adding Super Game Boy borders | ||
|
|
||
| I cannot guarantee skipping certain parts won't leave you confused, so if you wish to add SGB borders to already SGB capable software (that lacks borders), try not to skip anything |
There was a problem hiding this comment.
| I cannot guarantee skipping certain parts won't leave you confused, so if you wish to add SGB borders to already SGB capable software (that lacks borders), try not to skip anything |
I would remove this "disclaimer". It just leaves me confused and tries to convey the (obvious) fact of "please follow the guide without skipping any part if you actually want to succeed"
website/guides/sgb_border.md
Outdated
|
|
||
| Before we can do anything, we must first specify in the header that this game is an SGB game. | ||
|
|
||
| To enable SGB features: <!-- i think this is worded a little awkwardly, but i cant exactly think of how to fix it >< --> |
There was a problem hiding this comment.
| To enable SGB features: <!-- i think this is worded a little awkwardly, but i cant exactly think of how to fix it >< --> | |
| The SGB BIOS checks [the ROM's header](https://gbdev.io/pandocs/The_Cartridge_Header) when starting the Game Boy. | |
| Both of the following criteria must be met for the BIOS to enable enhancements (such as the border): |
There was a problem hiding this comment.
I would say those kind of detail/information is irrelevant at this point. We need to "enable SGB features", not understand why those are enabled/how from the SGB side (I would argue this kind of information is better suided on the pandocs side of docs)
There was a problem hiding this comment.
I wanted to mention that what controls the "enabling of SGB features" is the ROM's header, and not e.g. code. I figured that it could help with troubleshooting?
There was a problem hiding this comment.
Uh, okay, I see your point.. We should then try to simplify it a bit (e.g. to help restrict the cases in which really this "troubleshooting" may happen and the user should be aware of the fact it may involve SGB side, if I understood you correctly)
Co-authored-by: Eldred Habert <eldredhabert0@gmail.com>
Co-authored-by: Eldred Habert <eldredhabert0@gmail.com>
Co-authored-by: Eldred Habert <eldredhabert0@gmail.com>
Co-authored-by: Eldred Habert <eldredhabert0@gmail.com>
Co-authored-by: Eldred Habert <eldredhabert0@gmail.com>
|
|
||
| For an example of such routine, see this [code](https://github.com/zlago/violence-gbc/blob/11cfdb6ee8a35e042fa9712484d814e0961cea7c/src/sub.sm83#L413-L463) and the related Pan Docs entry: [SGB Command Packet on Pandocs](https://gbdev.io/pandocs/SGB_Command_Packet.html). | ||
|
|
||
| This guide glosses over a minor detail, as certain packets can be (albeit unccomon) more than 16 bytes. |
There was a problem hiding this comment.
Minor: should additional references/links be put here (e.g. a use case/example of sending more than 16 bytes?) or is it irrelevant/superficial?
| 3. the tilemap consists of `$00`, `$01`..`$13`, 12 bytes padding (offscreen), `$14`..`$27`, padding, repeat untill `$ff` (inclusive) | ||
| 4. the data you want to send must be loaded at `$8000`-`$8fff` | ||
|
|
||
| You can do 1, 2 and 3 via [this snippet](https://github.com/zlago/snek-gbc/blob/baef0369f57d2b0d58316cb1c28c6cc22475a6c9/code/init.sm83#L208-L230) |
There was a problem hiding this comment.
Should we include the snippet here as we did with the other one?
Co-authored-by: Damian Yerrick <git@pineight.com>
Co-authored-by: Damian Yerrick <git@pineight.com>
Co-authored-by: Damian Yerrick <git@pineight.com>
Co-authored-by: Damian Yerrick <git@pineight.com>
Co-authored-by: Damian Yerrick <git@pineight.com>
Co-authored-by: Damian Yerrick <git@pineight.com>
Co-authored-by: Damian Yerrick <git@pineight.com>
Co-authored-by: Damian Yerrick <git@pineight.com>
website/guides/border.md
Outdated
| ```sm83asm | ||
| ; test for SGB | ||
| xor a /* first it enables STAT interrupt, since snek-gbc happens to just have a `reti` as the handler */ | ||
| ldh [rLYC], a | ||
| ld a, STATF_LYC | ||
| ldh [rSTAT], a | ||
| ld a, IEF_STAT | ||
| ldh [rIE], a | ||
| ei | ||
| ; wait for SGB | ||
| ld b, 12 /* you must wait 12 or more frames before trying to send a packet */ | ||
| :halt | ||
| dec b | ||
| jr nz, :- | ||
| ; enable multi | ||
| ld hl, Packets.mlt /* send a `MLT_REQ` */ | ||
| call Packet | ||
| rept 4 /* then wait 4 more frames, since the SNES wont "read" the packet instantly */ | ||
| halt | ||
| endr | ||
| ; check if SGB responds | ||
| /* and now we actually try to detect the SGB | ||
| setting `P1.5` to low then high advances the "read player" | ||
| setting `P1.4` and `P1.5` high will make the SGB return which player is currently selected in `P1.0` and `P1.1` */ | ||
| lb bc, 5, LOW(rP1) /* b is loaded with 5 which is how many times we try to check if the player changes to anything but P1 */ | ||
| :ld a, P1F_4 /* try to advance player */ | ||
| ldh [c], a | ||
| ld a, P1F_4|P1F_5 /* then set P1 to return the current player */ | ||
| ldh [c], a | ||
| ldh a, [c] | ||
| dec b | ||
| jp z, .init ; give up /* if 5 (4 actually) attempts pass, assume this isnt an SGB */ | ||
| cp $ff /* `P1.6` and `P1.7` always return %1, we set `P1.4` and `P1.5` to %1, and DMG, or SGB player 1, return %1111 in `P1.0`-`P1.3` */ | ||
| jr z, :- ; try again if failed /**/ | ||
| ``` |
There was a problem hiding this comment.
This is perfectly fine code for using in a game, but I don't think it's good tutorial code because it drags in a bunch of implicit assumptions that someone who is learning may not be familiar with. It's using halt to wait for interrupts but this relies on the specific setup listed here:
- Interrupts must be enabled during this section, whereas interrupts are often disabled during setup code.
- LCDC must be on during this code in order to generate the interrupts, whereas LCDC is often disabled during initialization to make VRAM accessible.
- It relies in the specific setup of
STATwhich is given here as "magic values". - It relies on the STAT interrupt being a single
retior similar.
This (imo) makes this code fragile for use by a beginner unless they understand how all the pieces fit together. (By fragile I mean that a change somewhere else can easily cause this code to stop working.) It would be easy for me, who has been doing this for 20 years at this point, to say that this is good code because I can understand, see that it works, or otherwise know exactly what to do if it doesn't work. But I'm not the target audience of this code.
I would much prefer the more "traditional" approach of a busy wait loop for waiting frames, which, while not as fancy, solves all of the issues listed above.
There was a problem hiding this comment.
- doesnt the doc specify that LCD must be enabled during TRNs?
- arent waitloops that a beginner would use ineffective if LCD is off?
- are you not going to comment on the doc assuming you have means of VRAM-safe VRAM access?
There was a problem hiding this comment.
doesnt the doc specify that LCD must be enabled during TRNs?
Sure, but this is something they must keep in mind if they might later rearrange the code to put the detection in a section with LCDC disabled. And that doesn't address any of the other issues. For example, I'm assuming you chose to use STAT interrupt instead of the more obvious VBlank interrupt in order to avoid extra complexity in the VBlank interrupt handler. But now the STAT interrupt is "used up" instead. Using a waitloop makes this code more self contained without forcing assumptions on other parts of the code.
arent waitloops that a beginner would use ineffective if LCD is off?
I'm talking about a waitloop with a counter, not a LY waitloop. Something like Nintendo's suggestion:
ld de,$1b58
: nop
nop
nop
dec de
ld a,d
or e
jr nz,:-
ret
That particular code has some awkward nops in there, seemingly so that the loop takes exactly 10 M cycles and spins 7000 times for a total of 70000 M cycles for easy math. But that's the general idea.
are you not going to comment on the doc assuming you have means of VRAM-safe VRAM access?
Safe VRAM access in itself is one of the most central concepts in GB programming, so someone who is new will be presumed to learn it sooner rather than later.
There was a problem hiding this comment.
actually, i already happened to have STAT used up for doing nothing in snek-gbc (which is where all the snippets are from), in every other project i use vblank since there the handler only does what its told to (snek-gbc was programmed with the assumption that i would never have to build upon it, dont ask me why i felt the need to include a dmg-only startup animation)
There was a problem hiding this comment.
So to summarize, are these the requested changes?
- Do the delay with
diand a busy wait loop instead of vblank/STAT interrupt - Emphasize that the display must be on, or the SGB will never receive the data
There was a problem hiding this comment.
yep, @pinobatch , sounds like it ;)
| ``` | ||
|
|
||
| - `--color-zero` should be the color that your image for transparency, in my case it was blue. | ||
| * If your image has an alpha channel, it can be set can also be set to `00000000` to use the actual transparent color; however, this may cause some issues. |
|
|
||
| 1. You can set the first row of tiles to your transparent color to force superfamiconv to put the transparent tile as the 1st tile, however you must then exclude 64 bytes of the tilemap (`incbin "border.pct"` -> `incbin "border.pct", 64`) | ||
|
|
||
| 2. You can use palettes 0-3 and 7, if you really know what you're doing (animated borders yay). You will probably have to edit the border in YY-CHR, as there aren't really any other tools for that. |
There was a problem hiding this comment.
What does "if you really know what you're doing" imply?
There was a problem hiding this comment.
There may be advanced techniques to draw parts of the border with the game palette (0) instead of the palettes reserved for the border (4-6). Currently no converter supports doing this. Would it be worthwhile to make a test ROM to confirm that it's possible?
There was a problem hiding this comment.
since its not that practical to use the game palette, it should be a low priority
There was a problem hiding this comment.
Then I think this shouldn't be mentioned in the tutorial, beyond something like "palette 0 is used by the displayed GB screen, palettes 1-3 and 7 are used by the SGB BIOS and will be overwritten when opening various menus".
Mentioning that they can be used, but not what the limitations are, is likely to be more confusing.
Co-authored-by: Eldred Habert <eldredhabert0@gmail.com>
Co-authored-by: Eldred Habert <eldredhabert0@gmail.com>
Co-authored-by: Eldred Habert <eldredhabert0@gmail.com>
Co-authored-by: Eldred Habert <eldredhabert0@gmail.com>
Co-authored-by: Eldred Habert <eldredhabert0@gmail.com>
Co-authored-by: Eldred Habert <eldredhabert0@gmail.com>
Co-authored-by: Eldred Habert <eldredhabert0@gmail.com>
Co-authored-by: Eldred Habert <eldredhabert0@gmail.com>
|
It looks like this comment describes the last things that some contributor to this pull request needs to do before a maintainer can merge this pull request. In order to do them, I'd need to start yet another review creating yet more suggestions. However, in the "Files changed" tab, there are still a bunch of suggestions, some mine, that are marked "avivace marked this conversation as resolved" yet there was no action on the suggestion. I'm not sure what happened, nor whether I'm supposed to be making more suggestions on top of my previous suggestions that are "marked [...] as resolved" without either acceptance or any discussion of rejection. |
|
I'll go ahead and merge what we have now as the threads got confusing. Let's continue in another PR. |
PR gbdev#70 ended with different suggested changes stomping on one another. Finish the last few tasks mentioned in that PR's discussion. - fix a comma splice - explain why wait 4 frames - emphasize why the screen must be on - use line comments (;) instead of C block comments (/*...*/) - transform detection code into a subroutine to eliminate `.init` - rewrite detection code to use a busy wait loop instead of vblank interrupt - let the lines settle after each read in case A+Right held on DMG - store the detection result to a variable - Remove mention of use of palettes 1-3 and 7
PR #70 ended with different suggested changes stomping on one another. Finish the last few tasks mentioned in that PR's discussion. - fix a comma splice - explain why wait 4 frames - emphasize why the screen must be on - use line comments (;) instead of C block comments (/*...*/) - transform detection code into a subroutine to eliminate `.init` - rewrite detection code to use a busy wait loop instead of vblank interrupt - let the lines settle after each read in case A+Right held on DMG - store the detection result to a variable - Remove mention of use of palettes 1-3 and 7 --------- Co-authored-by: Antonio Vivace <avivace4@gmail.com>
closes gbdev/awesome-gbdev#246