Skip to content

Add Part 2 Chapter 3#33

Merged
ISSOtm merged 32 commits intogbdev:masterfrom
evie-calico:part3
Dec 8, 2022
Merged

Add Part 2 Chapter 3#33
ISSOtm merged 32 commits intogbdev:masterfrom
evie-calico:part3

Conversation

@evie-calico
Copy link
Copy Markdown
Contributor

Closes #14

Comment thread src/part2/input.md Outdated
Comment thread src/part2/input.md Outdated
Comment thread src/part2/input.md Outdated
avivace and others added 3 commits October 24, 2022 10:09
Co-authored-by: Eldred Habert <eldredhabert0@gmail.com>
Co-authored-by: Eldred Habert <eldredhabert0@gmail.com>
@evie-calico
Copy link
Copy Markdown
Contributor Author

This chapter was split into functions.md and input.md as ISSO suggested

@evie-calico evie-calico requested a review from ISSOtm October 24, 2022 16:48
Comment thread src/part2/functions.md Outdated
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.

Looks already very good to me! Just left a minor observation.

Comment thread src/part2/input.md Outdated
@evie-calico evie-calico requested a review from ISSOtm October 25, 2022 20:37
Comment thread src/part2/functions.md Outdated
Comment thread src/part2/functions.md Outdated
Co-authored-by: Antonio Vivace <avivace4@gmail.com>
@ISSOtm
Copy link
Copy Markdown
Member

ISSOtm commented Nov 8, 2022

I don't know why CI didn't go red, but this doesn't build: https://github.com/ISSOtm/gb-asm-tutorial/actions/runs/3379884599/jobs/5612019079#step:6:93

@evie-calico evie-calico removed the request for review from ISSOtm November 18, 2022 12:59
@evie-calico evie-calico requested a review from avivace November 18, 2022 12:59
@evie-calico evie-calico requested a review from ISSOtm November 18, 2022 13:06
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.

Sorry for the wait.

Comment thread src/part2/functions.md Outdated
Comment thread src/part2/functions.md Outdated
Comment thread src/part2/functions.md Outdated
Comment thread src/part2/functions.md Outdated
Comment thread src/part2/functions.md Outdated
Comment thread src/part2/input.md Outdated
Comment thread src/part2/input.md Outdated
Comment thread src/part2/input.md Outdated
Comment thread unbricked/input/main.asm Outdated
Comment thread unbricked/input/main.asm Outdated
avivace and others added 3 commits November 26, 2022 01:43
Co-authored-by: Eldred Habert <eldredhabert0@gmail.com>
Co-authored-by: Eldred Habert <eldredhabert0@gmail.com>
Co-authored-by: Eldred Habert <eldredhabert0@gmail.com>
Comment thread src/part2/functions.md Outdated
Co-authored-by: Eldred Habert <eldredhabert0@gmail.com>
@ISSOtm
Copy link
Copy Markdown
Member

ISSOtm commented Dec 6, 2022

Alright, so there are only two minor changes that need to be done, and we'll be able to merge this!

@evie-calico evie-calico requested a review from ISSOtm December 6, 2022 13:16
Co-authored-by: Eldred Habert <eldredhabert0@gmail.com>
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.

I re-read the entire thing, and I only have a single minor concern left. Great job @eievui5!

Comment thread src/part2/input.md Outdated
`UpdateKeys` writes the held buttons to a location in memory that we called `wCurKeys`, which we can read from after the function returns.
Because of this, we only need to call `UpdateKeys` once per frame.

This is good, because not only is it faster to just read the inputs last read, it also means that we will always act on the same inputs, even if the player presses or releases a button mid-frame.
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 feel like the "read the inputs just read" formulation may be confusing, but I don't see a better alternative?
I considered "re-read the inputs seen at the beginning of the frame", maybe?

Copy link
Copy Markdown
Contributor Author

@evie-calico evie-calico Dec 6, 2022

Choose a reason for hiding this comment

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

Fixed :)

This is important, because not only is it faster to reload the inputs that we've already processed, but it also means that we will always act on the same inputs, even if the player presses or releases a button mid-frame.

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.

This is important, because not only is it faster to reload the inputs previously read instead of re-polling them, but it also means that we will keep acting on the same inputs for the entire frame, even if the player presses or releases a button in the middle of it.

I think??

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.

Based on external feedback, the current text should be good enough, so let's merge it.

@avivace avivace requested a review from ISSOtm December 6, 2022 18:31
@ISSOtm ISSOtm merged commit c6d7c2b into gbdev:master Dec 8, 2022
@evie-calico evie-calico deleted the part3 branch December 8, 2022 22:16
@ISSOtm
Copy link
Copy Markdown
Member

ISSOtm commented Dec 8, 2022

And a big thank you to @eievui5!!

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 3

3 participants