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

Remove confusing and redundant wording #519

Merged
merged 2 commits into from
Jan 2, 2024
Merged

Conversation

tobiasvl
Copy link
Member

  • Remove confusing wording that seemed to imply that timer interrupts are executed immediately (regardless of priority)
  • Remove redundant information about the IF register, which is not present in the descriptions of other interrupts

@avivace avivace requested a review from ISSOtm December 14, 2023 21:43
an interrupt is requested by setting bit 2 in the IF register
($FF0F). As soon as that interrupt is enabled, the CPU will execute it by
calling the timer interrupt vector at $0050.
The timer interrupt is requested every time that the timer overflows
Copy link
Member

Choose a reason for hiding this comment

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

I think making the words "is requested" into a link to either IF's documentation or a more general description of the IRQ process (is there one?) would be appropriate in fulfilling the role that the rightfully removed text was. What do you say?

Copy link
Member

Choose a reason for hiding this comment

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

Agree!

@tobiasvl
Copy link
Member Author

Then maybe the link should be added to every mention of an interrupt being requested? It seems a bit arbitrary to explain what it means for a timer interrupt to be requested, when the VBlank and STAT interrupts have already been discussed above with no explanation of what an IRQ entails.

@ISSOtm
Copy link
Member

ISSOtm commented Dec 19, 2023

Oh, yes, sure! I only focused on the PR's changes, but if you think the change should be made elsewhere as well, then by all means go ahead!

Copy link
Member

@ISSOtm ISSOtm left a comment

Choose a reason for hiding this comment

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

Looks good to me now!

@avivace avivace merged commit 27dc982 into gbdev:master Jan 2, 2024
2 checks passed
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.

3 participants