Closed LaroldsJubilantJunkyard closed 10 months ago
I ought to at least leave a comment here, then.
I've basically dreaded reviewing this PR for a few reasons, hence my silence. Despite that I am really thankful that Larold put all of this effort, but I'm afraid e.g. of things being redundant with what's planned for the remainder of Part II. I don't want to leave a review because I don't want the effort to feel wasted.
Thus, I haven't dared look at the contents either.
I ought to at least leave a comment here, then.
I've basically dreaded reviewing this PR for a few reasons, hence my silence. Despite that I am really thankful that Larold put all of this effort, but I'm afraid e.g. of things being redundant with what's planned for the remainder of Part II. I don't want to leave a review because I don't want the effort to feel wasted.
Thus, I haven't dared look at the contents either.
No worries mate!
A) you can just leave it be and not use it. that's fine. B) you can post it, but leave a comment at the start about it being new and needing review
I'm very willing to help, update, and make changes/additions. But don't feel obliged. My mind has moved on to other projects. If you choose not to implement, i won't really notice.
It's up to you guys. If there's anything you need, let me know.
I ought to at least leave a comment here, then.
I've basically dreaded reviewing this PR for a few reasons, hence my silence. Despite that I am really thankful that Larold put all of this effort, but I'm afraid e.g. of things being redundant with what's planned for the remainder of Part II. I don't want to leave a review because I don't want the effort to feel wasted.
Thus, I haven't dared look at the contents either.
Thanks, ISSO.
Claiming redundancy with something that has only been "planned" but it's not being actively worked on it's a factor we should completely ignore right now. Part 2 will be worked on when / if people will decide to try to contribute. Until then, something "planned" there is very far from reality and feasibility (reviewers?) so it shouldn't be considered when reviewing here.
Let me be more precise about what we would need here:
IMO this part is already quite good and provides tons of additional pointers on top of what we already have (already have, not planned to have).
@LaroldsJubilantJunkyard is also being amazing, and he's very open to suggestions and has shown a rare commitment to keep working on this, accepting feedback and implementing it promptly, it would be a huge waste to ignore his work or let this go inactive because "stuff overlaps with some plans nobody has the intention to work on in the next future".
Okay, that all seems fair enough. I'll begin reviewing it now, then, but I ask you give me a little more time, considering the PR's size.
Special thanks to @LaroldsJubilantJunkyard for his patience..!
So as I mentioned via IM, I still haven't finished reviewing everything, but dropping all of the comments and questions I have in a single block would be pretty bad UX so to speak.
So, one thing at a time.
Indeed, this may not be what I had in mind for Part III, or it may happen to be (that would be a happy coincidence!).
But that doesn't really matter, because part of the contract of 1) having other people write the tutorial, and 2) moving the repo under gbdev
was that compromises would have to be made.
So I would rather accept Larold's work, because they put a lot of effort into contributing to a community resource, and that the tutorial would be worse off just incomplete. I think.
I've looked at the Makefile structure, and frankly I'm a little puzzled. It seems to bypass the normal build graph mechanism to instead try and treat Make more as a shell script.
I wouldn't mind that as-is, but it seems to be more complex, and also nullify Make's "only rebuild what needs to be" feature, which I think is not something we should show in a tutorial after which people will pattern their actual projects.
However, maybe I missed a specific reason it's done this way?
re: Makefile
Even if there’s a reason for it to be structured incorrectly, we absolutely need to rewrite it so that it works as expected. Make was chosen specifically because of the features it has which make gbdev easier. I think gb-boilerplate’s makefile (stripped down if needed) should be used and the project needs to be restructured accordingly
re: Makefile
Even if there’s a reason for it to be structured incorrectly, we absolutely need to rewrite it so that it works as expected. Make was chosen specifically because of the features it has which make gbdev easier. I think gb-boilerplate’s makefile (stripped down if needed) should be used and the project needs to be restructured accordingly
Do we have examples or directions on how such a makefile should be structured? Your (and @ISSOtm ) comments on the Makefile seem to say "it's wrong/should standardized" but they don't seem to provide corrections or ideas on how to correct/what's specifically wrong about it. Can we try to move this forward with some suggestion?
Indeed, this may not be what I had in mind for Part III, or it may happen to be (that would be a happy coincidence!).
But that doesn't really matter
@ISSOtm The problem is: it does matter, and we're waiting on you to provide feedback (or even any kind of formalisation) on what is that you have in mind since I don't know how much time that the tutorial got stuck between overlaps of blocking "ideas"/"plans" that are not even formalized yet. This literally blocks the resource.
Any kind of information on this okay, even just "I don't know / I don't have time / I don't care" are acceptable (and perfectly legit) responses (and definitely better than no response only followed by this is not my vision but I'll accept it delivered months after).
We don't want the tutorial to follow your vision/ideas because of a "coincidence" (that you are happy to accept just because someone worked on it?). This really sends the wrong message.
We want the process of formalizing, discussing and exchanging ideas and opinions to be the reason why the tutorial explains A doing B in a C way. None of this can be done if you don't provide practical examples (or even vague formalisations) of your plan.
So please, tell us your ideas/vision and which way this part may or may not be implementing them. That's the only thing we need to go forward. In absence of that (or your leadership) this gets blocked and we're back at square one.
Do we have examples or directions on how such a makefile should be structured? Your (and @ISSOtm ) comments on the Makefile seem to say "it's wrong/should standardized" but they don't seem to provide corrections or ideas on how to correct/what's specifically wrong about it. Can we try to move this forward with some suggestion?
i did make a suggestion: we should use gb-boilerplate’s makefile, removing some features if needed.
gb-boilerplate
Thanks @eievui5 . @LaroldsJubilantJunkyard can you check https://github.com/ISSOtm/gb-boilerplate and see if it could be applicable to the project files here?
I wrote what I thought about the Makefile design, and GitHub's mobile app didn't send it. Great.
The structural problem I have with the Makefile is that the top-level rule is intended to run four steps in parallel (clean, convert gfx files, copy sources to obj
, and build the ROM). This bypasses/abuses Make's dependency graph, which is not a practice I think we should recommend, especially because I believe it would break with parallel builds (-j
).
I did not suggest any alternatives, because I wanted to give @LaroldsJubilantJunkyard a chance to defend his design before suggesting a replacement (see the last line of my previous message). I was afraid that it would be discouraging and/or demeaning.
I do not think that we should use gb-boilerplate, as its Makefile has extra complexity in order to automatically detect dependencies (and it's known to be unstable depending on the version of Make, though Make 4.4 maybe finally explains why). However, the general idea remains the same: merely describe dependency relations & build recipes, and let Make do its thing of figuring out in which order to build what. The difference with gb-boilerplate being that we'd specify everything manually, to keep it relatively simple.
Regarding the Makefile, I have a few primary suggestions right now. Most of them revolve around making the Makefile drastically simpler, following the theme of introducing new concepts incrementally.
SRCDIR
variable. The point of variables like OBJDIR
is to allow building out-of-tree in a flexible location; however, it makes imo no sense to allow pointing the Makefile at a different source tree, since it's inherently part of it.${}
for variables instead of $()
. This helps differentiate them from function calls (which only accept the latter syntax); this is especially useful when both kinds are nested.*DIR
and RGB*
variables. For a "directed" project like this, it should be fine to hardcode all paths; this will notably remove one layer of indirection, and allow the reader to familiarise themselves with what that layer of indirection will "resolve" to. (There may be a case for keeping all of the RGB*
variables, in case someone doesn't have RGBDS in their path, or under different names? Idk.)Of course, the question is then "sure, but when do we introduce all of those? Part 3 is the last one!"; to which the answer I'd give is to make an additional Part 4. (I think not building a full game in that one, but just guiding through the creation of a template, should be sufficient. That said, if this idea sounds good to other people, I should open a separate issue to discuss it and not derail this already lengthy PR.)
Note: I haven't submitted a Makefile applying the above points because I thought it'd be better to discuss them first. But I can also write one if that's preferred.
Adding SHMUP tutorial for the gb-asm-tutorial: Galactic Armada.
Markdown files created for the various parts of the tutorial. Images, markdown files, and code snippets organized into their own folders. The summary.md has been updated with all the part 3 sections.
Repository of the code: https://github.com/LaroldsJubilantJunkyard/galactic-armada