Skip to content

Part II, Lesson 4#37

Merged
ISSOtm merged 22 commits intogbdev:masterfrom
evie-calico:master
Feb 18, 2023
Merged

Part II, Lesson 4#37
ISSOtm merged 22 commits intogbdev:masterfrom
evie-calico:master

Conversation

@evie-calico
Copy link
Copy Markdown
Contributor

Closes #15

@evie-calico
Copy link
Copy Markdown
Contributor Author

At the moment this only implements ball movement, not bouncing.

@avivace avivace requested review from ISSOtm and avivace January 3, 2023 10:37
@ISSOtm
Copy link
Copy Markdown
Member

ISSOtm commented Jan 5, 2023

The current code looks fine to me.

@avivace
Copy link
Copy Markdown
Member

avivace commented Jan 8, 2023

What are we missing here? The text part? @eievui5 do you have any update on this?

@evie-calico
Copy link
Copy Markdown
Contributor Author

We're missing the ball bouncing against the walls and the paddle, and the text.

@evie-calico
Copy link
Copy Markdown
Contributor Author

I'd like some feedback on the bouncing code. The pixel -> tilemap function might be too complex for the tutorial but I'm not sure how else to write it. Otherwise, I think this is pretty understandable :)

@evie-calico
Copy link
Copy Markdown
Contributor Author

The code should be complete now.

Copy link
Copy Markdown
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.

Alright, the logic seems correct, I just hope we won't start overflowing VBlank time... ><

Comment thread unbricked/collision/main.asm Outdated
Comment thread unbricked/collision/main.asm
Comment thread unbricked/collision/main.asm
Comment thread unbricked/collision/main.asm
@evie-calico evie-calico requested review from ISSOtm and removed request for avivace January 12, 2023 03:25
@evie-calico
Copy link
Copy Markdown
Contributor Author

The text and code is now complete.

@evie-calico evie-calico marked this pull request as ready for review January 12, 2023 03:25
Copy link
Copy Markdown
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.

Wow, this actually reads very smoothly! I just left some minor comments and suggestions and will go over it again later.

Comment thread src/part2/collision.md Outdated
Comment thread src/part2/collision.md Outdated
Comment thread src/part2/collision.md Outdated
Comment thread src/part2/collision.md Outdated
evie-calico and others added 2 commits January 12, 2023 07:53
Co-authored-by: Antonio Vivace <avivace4@gmail.com>
@evie-calico evie-calico mentioned this pull request Jan 13, 2023
Copy link
Copy Markdown
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.

Great work! I only have some clean-up suggestions.

Additionally, I would prefer if the Z flag was capitalised, for consistency with the "flags" lesson. (The rationale there was to avoid confusion with the c register.)

And maybe we could link back to some previous lessons, if they are relevant.

Comment thread src/part2/collision.md Outdated
Comment thread src/part2/collision.md Outdated
Comment thread src/part2/collision.md Outdated
Comment thread src/part2/collision.md Outdated
Comment thread src/part2/collision.md Outdated
Comment thread src/part2/collision.md Outdated
Comment thread src/part2/collision.md
Comment thread unbricked/collision/main.asm Outdated
Comment thread unbricked/collision/main.asm Outdated
Comment thread src/part2/collision.md
@zlago
Copy link
Copy Markdown

zlago commented Jan 14, 2023

Additionally, I would prefer if the Z flag was capitalised, for consistency with the "flags" lesson. (The rationale there was to avoid confusion with the c register.)

may i ask why you wont state that theyre called Z/C, but that the tutorial will refer to them as zf/cf (or z/cy) to avoid confusion with registers?

@ISSOtm
Copy link
Copy Markdown
Member

ISSOtm commented Jan 14, 2023

I just went with common practice. Using one of these names would probably help (and cf/cy might help avoid the rrca/rra confusion), but I don't want to have to go back to previous lessons as well 🥲

@evie-calico evie-calico requested a review from ISSOtm January 14, 2023 22:23
Copy link
Copy Markdown
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.

Some minor changes, and then I'll have to make the explanation diagram for the last paragraph.

Comment thread src/part2/collision.md
Comment thread src/part2/collision.md Outdated
Comment thread src/part2/collision.md Outdated
Comment thread src/part2/collision.md Outdated
Comment thread src/part2/collision.md Outdated
Co-authored-by: Eldred Habert <eldredhabert0@gmail.com>
@evie-calico evie-calico requested a review from ISSOtm January 14, 2023 22:49
@evie-calico
Copy link
Copy Markdown
Contributor Author

Go ahead :)

@ISSOtm
Copy link
Copy Markdown
Member

ISSOtm commented Jan 21, 2023

Here are screenshots of the diagram:

image

image

Maybe it should be clarified that we are considering the position of the ball's top-left pixel, though it is highlighted in the above. And, speaking of, going 16 pixels to the right doesn't seem to make much sense given how wide the paddle appears to be. I dunno.

@evie-calico
Copy link
Copy Markdown
Contributor Author

evie-calico commented Jan 21, 2023

going 16 pixels to the right doesn't seem to make much sense given how wide the paddle appears to be

I’m not sure how much you’ve played around with the physics but i think this has the best feel to it. Any smaller and the player would probably be frustrated by the small paddle size. It might be worth doubling the width of the paddle with another sprite but i don’t think that’s necessary.

@ISSOtm
Copy link
Copy Markdown
Member

ISSOtm commented Jan 21, 2023

Then maybe we should offset the hitbox a bit more to the left, so it's symmetrical?

@evie-calico
Copy link
Copy Markdown
Contributor Author

I'd rather just go edit the ball graphic to be one pixel over to the right. Could you update your diagram to reflect that?

@ISSOtm
Copy link
Copy Markdown
Member

ISSOtm commented Jan 21, 2023

Sure, it's just a PNG, you can edit it in assets/part2.

@evie-calico
Copy link
Copy Markdown
Contributor Author

Thought it was an svg :) should be good now

@ISSOtm
Copy link
Copy Markdown
Member

ISSOtm commented Jan 21, 2023

Alright, that looks good to me. Should we add a "bonus" heading, suggesting how to make the ball bounce when its bottom pixel touches the paddle's? We should suggest "you only need a single instruction!" (sub a, 6).

@evie-calico
Copy link
Copy Markdown
Contributor Author

Maybe. I like that it kinda sinks in. If we wanna add that in at all then maybe it should just be part of the tutorial itself.

@ISSOtm
Copy link
Copy Markdown
Member

ISSOtm commented Jan 21, 2023

I think it would be beneficial to encourage experimentation, especially if it's just a single instruction.

Copy link
Copy Markdown
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.

Great!

Comment thread src/part2/collision.md
Co-authored-by: Eldred Habert <eldredhabert0@gmail.com>
@evie-calico evie-calico requested a review from ISSOtm January 21, 2023 20:01
@ISSOtm ISSOtm requested a review from avivace January 21, 2023 20:03
Comment thread src/part2/collision.md Outdated
Comment thread src/part2/collision.md Outdated
Comment thread src/part2/collision.md Outdated
Comment thread src/part2/collision.md Outdated
Comment thread src/part2/collision.md

::: tip

Please do not get stuck on the details of this next function, as it uses some techniques and instructions we haven't discussed yet.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Are we sure we shouldn't introduce (at least some) of those technicques and instruction before introducing them as code? It would be simpler and will avoid people having to "ignore" entire blocks of code because they cannot be understand yet.. Besides, what is the advantage or the purpose in doing this? It just adds backlog for the future ..

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

There is so much to explain here that it would take an entire chapter to do so. I don't see any way to explain this without wasting a lot of the reader's time.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

All things considered, I think it would make sense to discuss this function in a chapter of its own, since there is so much to discuss.

Considering the way Part II is written, I think we should explain it after introducing it, so that all of the dry concepts and explanations will at least be in a context and goal already known to the reader.

Thus, I think we should dedicate the lesson following this one to that function. Does that seem reasonable?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If there is so much to discuss, and if it requires an entire chapter, why exactly are we pushing this to the reader without discussing it beforehand? I don't see any advantage in doing so, except adding complexity in how are the concept presented. What I can propose is to actually write "this chapter that will explain this block" and then we merge/add it here. Eventually maybe this chapter could be split in 2 if it's too long, but I believe that the we definitely exaggerated in the "goal" for this chapter by pushing too much "unexplained" or "ignore this , will be explained" later stuff

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's simply because all the math and instructions behind this would completely throw off the pacing of the tutorial. Our primary concern at the moment is teaching the logic of assembly and making a game.

I don't think anyone wants to start off "Hello, world!" by explaining all the intricacies of file descriptors and pipes, for example. I would consider this function to be "#include <stdio.h>". It would be nice to explain more, but now is not the time.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Showing results and explaining them later has been the entire guiding principle behind Part I, since the gb-asm-tutorial-old showed that the "explain everything first, show results at the end" strategy was ineffective.

I think it also helps making the tutorial feeling more cohesive; think of it as a plot thread/cliffhanger?

Maybe we should reword this info box, however. How about:

This function uses a lot of concepts we haven't talked about yet.
We will examine and explain it in the next section; for now, let's assume it does what we want it to.

Copy link
Copy Markdown
Contributor Author

@evie-calico evie-calico Feb 9, 2023

Choose a reason for hiding this comment

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

"next section" being part 3, or lesson 4.5?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I meant "next lesson", don't mind me.

Comment thread src/part2/collision.md Outdated
Comment thread src/part2/collision.md
Comment thread src/part2/collision.md Outdated
@evie-calico evie-calico requested a review from avivace January 26, 2023 15:12
Copy link
Copy Markdown
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.

I don't agree with the suggested order of presenting things (let alone pushing entire functions/sections of code to the reader) but it's merely an order issue, it can be re-examined later when such explanations are added. Let's just keep the open points here https://github.com/ISSOtm/gb-asm-tutorial/pull/37#discussion_r1083534581 on our todos.

Everything else looks very good

@ISSOtm ISSOtm merged commit c354e01 into gbdev:master Feb 18, 2023
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.

Part II, Lesson 4

5 participants