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

Move hard coded GEN_2_GRAPHICS to Makefile as a build parameter #26

Closed
wants to merge 3 commits into from

Conversation

etdv-thevoid
Copy link
Contributor

@etdv-thevoid etdv-thevoid commented May 7, 2024

Removes the hard coded constant GEN_2_GRAPHICS out of includes.asm and replaces it with a RGBASM flag _GEN_2_GRAPHICS.

Adds the ability to run make run with GFX=2 to build the ROMs with Gen 2 Graphics. And GFX=1 to build with Gen 1 Graphics.

To maintain current behavior, makefile defaults to Gen 2 Graphics if no option is included when running make.

Allows building both versions from the command line without editing .asm files.

Improves modularity of code base for downstream forks and projects.

@dannye
Copy link
Owner

dannye commented May 9, 2024

This definitely has its advantages, but it also has the downside when you run make GFX=2 immediately followed by make GFX=1 and it misleadingly reports:

make: Nothing to be done for 'all'.

I'll sleep on this and try to think of a way to get the best of both worlds.

The current approach of using includes.asm has the benefit that when you change the constant value, you immediately trigger a full rebuild since includes.asm is a dependency of every other file.

@etdv-thevoid
Copy link
Contributor Author

etdv-thevoid commented May 10, 2024

This definitely has its advantages, but it also has the downside when you run make GFX=2 immediately followed by make GFX=1 and it misleadingly reports:

make: Nothing to be done for 'all'.

I'll sleep on this and try to think of a way to get the best of both worlds.

The current approach of using includes.asm has the benefit that when you change the constant value, you immediate trigger a full rebuild since includes.asm is a dependency of every other file.

Would it be better and more maintainable to instead have makefile targets for each? Maybe something like this?

roms := \
	pokered.gbc \
	pokeblue.gbc \
	pokeblue_debug.gbc \
	pokered_gen1gfx.gbc \
	pokeblue._gen1gfx.gbc \
	pokeblue_gen1gfx_debug.gbc \

Etc. with the rest of the build.

Could potentially make updates from base pokered annoying though.

@dannye
Copy link
Owner

dannye commented May 11, 2024

Would it be better and more maintainable to instead have makefile targets for each? Maybe something like this?

Sounds pretty tedious.

I rather like the simplicity and reliability of the current system (but I recognize the pros and cons).

@etdv-thevoid etdv-thevoid deleted the branch dannye:master May 13, 2024 13:36
@etdv-thevoid etdv-thevoid deleted the master branch May 13, 2024 13:36
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.

None yet

2 participants