Add sound bits documentation for wOptions

[LinusU]

I'm not sure about how to best express bit ranges but I tried to be consistent here since the previous description of text speed ("bits 0-3") lead me to believe that it took up 4 bits instead of 3. I'm happy to change all of them to whatever format we want...

[Rangi42]

Thanks, but we'd prefer to be removing this sort of bitflag comment from wram.asm. Instead, named constants for the bits should be self-documenting. Also changes like this should apply to pokered not pokeyellow, unless they're for yellow-only things like Pikachu or GBC colors.

[LinusU]

[...] unless they're for yellow-only [...]

@Rangi42 I believe that the Earphone 1, 2, 3 thing is specific to Yellow?

[Rangi42]

Oh, sure. Regardless, gen 1 should have WRAM constants for options and other variables, like https://github.com/pret/pokegold/blob/master/constants/wram_constants.asm#L31-L69.

[dannye]

Thanks, but we'd prefer to be removing this sort of bitflag comment from wram.asm. Instead, named constants for the bits should be self-documenting.

Sure, but until a real such effort is made, why block a legit correction?

[Rangi42]

Sure, but until a real such effort is made, why block a legit correction?

I might be picking up this habit from work, but, why add a comment that you know will shortly be removed? pret/pokered#413 is already in progress.

[LinusU]

Personally, my barrier for accepting a pull request is that it improves things, and doesn't make anything worse. Especially in open source I've found this to make people more happy to contribute, as it lowers the barrier to getting their first few PRs merged.

Anyhow, as I said in the other PR I won't have the time right now to work on further improving the constants. I just wanted to submit some local notes I have made whilst investigating upstream. Feel free to close the PRs if you prefer to fix that with a larger PR at some other time...

[dannye]

I might be picking up this habit from work, but, why add a comment that you know will shortly be removed?

"Shortly" could be too optimistic. Especially since this one is yellow-exclusive.

I think closing this after 2 minutes is overly dismissive, especially since this patch is a strict improvement. Although, I understand the inclination since this past year we have gotten a lot of pull requests that deserved to be closed right away.