Skip to content

Boot-up B register values #75

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

dmitry-shechtman
Copy link
Contributor

Unifies #72 and #74 as requested.

This was referenced Aug 12, 2025
Copy link
Collaborator

@Rangi42 Rangi42 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The BOOTUP_B_CGB and BOOTUP_B_AGB constants have been around since hardware.inc v3.0. They should still be defined in hardware_compat.inc.

@dmitry-shechtman
Copy link
Contributor Author

I'm not sure if that would be a good idea, since they are being removed for a reason.

@Rangi42
Copy link
Collaborator

Rangi42 commented Aug 12, 2025

Everything in hardware_compat.inc was removed for a reason. It's there for backwards compatibility.

@dmitry-shechtman
Copy link
Contributor Author

Actually, it might be a good idea to add a comment in hardware_compat.inc with an explanation for every significant change, such as this one.

@Rangi42 Rangi42 requested a review from avivace August 12, 2025 22:06
@Rangi42 Rangi42 dismissed their stale review August 12, 2025 22:07

Commented

@Rangi42 Rangi42 marked this pull request as draft August 12, 2025 22:07
@Rangi42
Copy link
Collaborator

Rangi42 commented Aug 12, 2025

Actually, it might be a good idea to add a comment in hardware_compat.inc with an explanation for every significant change, such as this one.

Maybe so; some already have such comments, e.g. the ones suggesting RGBASM-feature alternatives.

@dmitry-shechtman
Copy link
Contributor Author

Maybe so; some already have such comments, e.g. the ones suggesting RGBASM-feature alternatives.

So do we agree on this warranting a separate line with a comment?

@dmitry-shechtman dmitry-shechtman marked this pull request as ready for review August 13, 2025 12:21
@Rangi42
Copy link
Collaborator

Rangi42 commented Aug 13, 2025

LGTM, thank you! I'm not actually merging it yet because:

  • We have multiple ongoing PRs, it might be convenient to iron them all out, then merge all at once and call it a new release
  • This removes some constants from hardware.inc and so is a breaking change, needing version number 6.0
  • When we get around to 6.0 I also want to fix Rename _AUD3WAVERAM to AUD3WAVERAM #64

@dmitry-shechtman
Copy link
Contributor Author

Since we're already breaking it, please consider updating hardware.inc to use S_* for consistency.

Copy link
Member

@avivace avivace left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM as well

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants