Skip to content

Remove unreferenced yellow files and renamed all H_ constants#166

Closed
vcvtne2ps2bf16 wants to merge 2 commits into
pret:masterfrom
vcvtne2ps2bf16:master
Closed

Remove unreferenced yellow files and renamed all H_ constants#166
vcvtne2ps2bf16 wants to merge 2 commits into
pret:masterfrom
vcvtne2ps2bf16:master

Conversation

@vcvtne2ps2bf16

@vcvtne2ps2bf16 vcvtne2ps2bf16 commented Sep 25, 2017

Copy link
Copy Markdown

Also renamed pic/bmon to pic/mon

@vcvtne2ps2bf16 vcvtne2ps2bf16 changed the title Remove unreferenced yellow files and rename pic/bmon to pic/mon Remove unreferenced yellow files and renamed all H_ constants Sep 25, 2017
@dannye

dannye commented Sep 25, 2017

Copy link
Copy Markdown
Member

As always, I don't speak for everyone, but I don't think these are useful changes.
The pic/bmon folder is named that way to clarify that the pics inside that folder come from English Blue version (in contrast with the pic/rgmon folder where those pics come from the Japanese Red/Green versions, or the pic/ymon folder which comes from Yellow version).
Even though the pics in pic/rgmon and pic/ymon are never used when you build the vanilla game, we keep those files there for documentation's sake.

The same goes for the Yellow-specific song files, although this point is a little weak since the most up-to-date version of those song files can be found in the pokeyellow repo.

Lastly, the reason that WRAM labels are named like wRamAddress is because they are defined in a proper WRAM section. The reason that HRAM labels are named like H_RAM_ADDRESS is because they are hard-coded constants rather than put in a proper HRAM section.
The reason they are hard-coded is because RGBDS's linker can't optimize ld [hRamAddress], a in to the 2-byte version of the instruction for HRAM addresses if a label is provided. It can only perform the optimization if a constant is provided. (this is discussed a little more here)
Using the naming scheme of H_RAM_ADDRESS serves as a useful reminder that those constants aren't really defined in the same way as ordinary address labels.
If RGBDS ever gets the proper improvement to allow the linker to perform this kind of optimization, then I would agree we should change the naming scheme of HRAM labels.

@vcvtne2ps2bf16

vcvtne2ps2bf16 commented Sep 25, 2017

Copy link
Copy Markdown
Author

I understand the reason to keep the unref'd files, but i have some doubts about the H_ renaming argument: what i want to do with that commit is to make hram constants naming consistent, since a few of these constants still use the old naming scheme of H_ADDRESSNAME, and using hAddressName make it clear that these constants are used as addresses

I also think that what define a constant is the EQU keyword, not whether it's capitalized or not

@dannye

dannye commented Sep 25, 2017

Copy link
Copy Markdown
Member

Ah, I see now that hram constants in hram.asm are more inconsistent than I thought. I didn't realize that already more than half of the constants were converted from H_* to h* in 2014/2015.
In the effort of consistency, just changing them all to h* is fine. Besides, it might encourage us to finally make rgblink properly handle 2-byte load instructions with hram labels.

@yenatch

yenatch commented Sep 25, 2017

Copy link
Copy Markdown
Contributor

The alternative is using ldh.

@yenatch

yenatch commented Dec 31, 2017

Copy link
Copy Markdown
Contributor

I think pic/rgmon/ should stay since there's no other repo with these graphics.

@ISSOtm

ISSOtm commented Apr 22, 2018

Copy link
Copy Markdown
Contributor

I know this is a bump, but the PR is still open.
Anyways, there has been some debate about RGBDS allowing ld optimization, which was eventually rejected, and Antonio instead decided to instead add an option to disable the constant optimization (notably, arguing that it was required for disassemblies).

Another problem with using constants is that they're not included in the generated SYM files. When trying to debug the game (which is not dying thanks to ROM hacks and other projects), it's a bit of a bummer.

To sum it up:

  • That optimization will never come
  • It would be more handy to move all constants to labels

Thus, I would like to bring up again the HRAM change -- as standalone, though. Imo the pic/ folder is another debate.

@dannye dannye mentioned this pull request Jul 4, 2018
@PikalaxALT

Copy link
Copy Markdown
Contributor

Is this still necessary?

@dannye dannye closed this May 6, 2020
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.

5 participants