-
-
Notifications
You must be signed in to change notification settings - Fork 54
Document the jr instruction and use it in hello-world.asm
#151
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
Conversation
|
Notice that this implicitly relies on the fact that instructions are encoded, which IMO is too early a time to mention in the tutorial. I believe we should strive to keep the vague idea that instructions are just an abstract concept that the CPU goes through one by one, so that the reader builds first a mental model of how execution works, and that can later be refined to explain how that is actually implemented (and when that implementation detail surfaces). (This feedback is specific to the wording used here, and thus posted despite the ongoing conversation in #146; wording-agnostic feedback should be kept there IMO.) |
|
...The previous sections have introduced the idea of one-byte registers and two-byte register pairs; "memory chips" with addresses explained by an analogy of street addresses; noting how "if we assume that the tile data ends up being stored starting at Sorry, but I have a hard time even entertaining the idea of fake-teaching the "vague idea that instructions are just an abstract concept", given all the minute detail (some inevitable, some extraneous) that the start of this tutorial has gone into. And if we accept the realistic status quo -- "the tutorial's initial sections teach the basics, but things like |
Then, as I stated in #146 (comment) (after the first quote), I would rather have that extraneous detail trimmed. Reading it again, I agree that this was basically me going into an autistic tangent, and not providing any useful context to the subject matter. Thus, my vote is for removing that. |
avivace
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect for me. Yes the 'encoding' concept may be early but it's not even needed to understand the instruction, just to properly grasp why it's "cheaper". We can make a point and potentially arrive at this more "gracefully" later. For now it's fine.
|
Wow. |
|
I'm feeling very angry right now. There was conversation in #146 that was also starting to shape a common ground (#146 (comment) was thumbed-up by Sylvie, and the two of us also sought a common ground in DMs in order to iterate faster than lengthy GitHub posts, reaching an agreement before I stated I was too sleepy to continue), and yet you merged this PR unilaterally. The last time I've done so was gbdev/hardware.inc#58 (comment), which prompted you to DM me the following:
I apologised for my mistake, and you nonetheless removed my push access to Yet you've done precisely the same here: merging a 14-hour-old PR (and even the issue was just 3 days old, compared to hardware.inc's 8) with unilateral approval, despite significant pushback in the discussion that was even starting to reach some consensus. This, I find unacceptable, especially as it contradicts the guidelines along which you've handed me a sanction. If you believe it was a mistake, then let's revert this, shake hands in apology, and resume where we started like nothing happened—I don't want to create unnecessary tension or drama. I know I haven't been doing much about the tutorial for a while, but like you rejected my (misguided) view that “As far as I could tell, you weren't interested in that project, so I didn't think you wanted to weigh in, and I merged the PR”, I reject any view that I wasn't interested in the tutorial, as evidenced by my recent activity in #146, which I explicitly acknowledged and explained in #146 (comment):
IMO, #146 and #153 should be re-opened for further discussion, and the PRs either reverted or amended depending on the outcome of the issues' discussion. PS: I am posting all of this in public, because I think it's better to discuss the consequences of public actions in public so as to offer better accountability and transparency. I do not wish this to become public shaming. |
|
I agree that merging my PRs was premature, and should be reverted. I had opened them so as not to just be telling people "here's how I think things should be done, now go do them that way". And to have a concrete proposal of just when+how I'd teach those concepts. But there was/is still active discussion going on about them. We did have a similar miscoordination around hardware.inc, and I thought we were now all on the same page about giving maintainers time to review, and getting "2 reviews/endorsements from maintainers" or else "1 and clear inactivity / not interest from the others". |
|
Two maintainers (@ISSOtm and myself) agreed to revert this change. I still stand by it being an improvement, but it was unexpectedly merged in the middle of ongoing disagreement/discussion about the idea. I'm not going to be making further piecemeal changes to gb-asm-tutorial. I've proposed a broader restructure under which my suggestions might be agreeable as a compromise. Any further discussion can go there in #156. |
I am sorry you feel that way. Nothing permanent/irreversible was pushed and I'm very happy to iterate to what we have.
You are (currently) not a maintainer of this project. There's a pinned Issue #113 where I look for maintainers because the vacuum in maintainance and proper reviewing activity provoked a lot of work from contributors to go lost. While the issue was posted in December 2024, the condition depicted in the image had already persisted for approximately a year.
After a long period of inactivity where you personally asked to wind down your responsibility/commitments for personal reasons, the maintainer role is naturally expected to decay. Maintainership and trust is rebuilt through consistent contributions and a demonstrated pattern of following through on commitments over time. I’m open to you regaining that role, but it should come from sustained involvement rather than an expectation of immediate veto power in PR reviews. |
The point of bringing up those DMs was not to say that the merge shouldn't have been done, but to justify the revert that I was advocating for as not merely going back to a stalemate, but to a situation that can lead to a productive solution.
I was in the de-facto maintainer team for this repo until early this morning, and had received no signals to the contrary, so I had good reason to believe that I was still a maintainer when posting each of my previous messages. (There's more discussion of that further below.) Even then, the
I wholeheartedly agree with this goal; however I disagree that this change is an improvement, on the grounds that adding information without thought about its place can actually harm the tutorial's quality (because assuming that quantity matters and pacing doesn't, the tutorial is redundant with the RGBASM language reference). It is based on that observation that... (cont.)
...the discussion turned to “okay, what do we actually want to do? What is the overall plan?” I do not believe this is derailing, but rather drilling down the surface issue to find the real source of the problem. We do this all the time with code, why not with meaning?
I agree, I thought this would better support the point I was trying to make, but since those DMs didn't contain any sensitive, personal, or confidential info or statements, it seemed acceptable to quote them. I should probably have paraphrased them instead; thus, my apologies. Maintainership status
To pick up from where I left off above, until 2026-01-16 01:34 GMT+0100, I was in the
Then why are AntonioND and Sylvie both maintainers of the repo? (They are both part of the aforementioned invisible maintainer team.) The latest activity I've found from the former was feedback on #124 half a year ago; and the latter has only 4 commits, 3 of which are from this week, across 3 PRs. I have visible non-trivial activity in this repo in #112 (Nov 2024), #119 (Feb 2025), #124 (May to August 2025, a substantial PR even though it was rejected), #136 (Dec 2025), #124 (two weeks ago), and #148 (two days ago); and also on Discord, participating in discussions in June and throughout November and December of 2025. I have also followed a good half of the repo's activity, merely saying nothing when I had nothing to bring to the table, but of course there's no proof of that. All in all, I believe this is not significantly less consistent than any of this repo's other current maintainers. Further, I have not reverted the changes to the repo, Sylvie has, on her own; but you have nonetheless force-pushed(!) away her commits: why are you overruling another maintainer? All you have stated is “I'm undoing the revert” on Discord without any rationale.
I have not vetoed this change, I have only pushed back strongly against it, purposefully sticking to the issue thread rather than a PR review, because this PR has something to bring to the tutorial. My arguments in #151, translated into a PR review, would have amounted to “please move this to <other location>”, not “this is bad, closing”: iterative improvement, not vetoing. And, regardless, as I have argued above, I believe I have retained sufficient involvement in this repo to qualify as one of the current maintainers. |
|
The length of all this discussion is already tiring me out, and I've said/asked what I mean to say/ask in #156. But just to note:
I reverted my own commits after discussing it with you, under the impression that it counted as "two maintainers" and would be the simplest/quickest way to avoid another inter-maintainer upset like with hardware.inc. And regardless of whether I have push access to this gb-asm-tutorial repository, I haven't considered myself "a maintainer" of the tutorial, given that I've only even started filing issues/PRs with it in the past week or so. Since @avivace had merged my PRs in to begin with and has now re-pushed them, I'm not going to argue about that. (Especially since, you know, I consider their content good, whether or not I'm the one responsible for the content now being in the tutorial. My concerns here are with whether our process has been followed or is even well-defined at all, as I said in #156.) |
Following the discussion in #146, here's my try at introducing the
jrinstruction to beginners.I think that a section titled "Jumps" is the obvious place to explain how relative jumps work. And they're clearly relevant and idiomatic right away in the hello-world.asm code -- anybody coming onto the gbdev Discord for help, or browsing the source code of other games, would see idioms like
jr c, WaitVBlank, notjp c, WaitVBlank.Furthermore, consider the concepts used and introduced in this paragraph. Section I.7 "Memory" already described the CPU in more than sufficient detail, and noted that instructions take up one or more bytes. And "distance relative to the current point" isn't even a programming concept, it's just... how distance works. The only new-ish concept is the word "cycle" to refer to how long instructions take to execute, but it shouldn't come as a surprise that doing things takes time -- we don't have to discuss at this point why it's faster (because there's one fewer byte to read, so the time is saved by reading less).
Fixes #146 since we'd no longer need to deoptimize/pessimize Unbricked with
jpeverywhere. (I'm not updating the larger projects in parts II and III to usejrconsistently, but with this PR, it's at least be an option for anyone maintaining that code.)