Pong tutorial chapter 03 #507

futurepaul · Dec 10, 2017

Here's the third chapter (we're tracking this work in #506 now). This one feels a little terse to me, so please chime in if you have a more accurate / deeper explanation of the concepts explored here. The main goal of this chapter is to explain systems.

This change is

Xaeroxe · Dec 10, 2017

Xaeroxe requested changes Dec 10, 2017

View changes

Couple nits but this is otherwise really great!

If you're reading this and my two comments have been addressed please dismiss my review and consider this an approval.

Xaeroxe · Dec 10, 2017

book/src/pong_tutorial/pong_tutorial_03.md

+);
+```
+
+Now we're done worrying about liftimes! Painless, right? Also, you might notice 


Xaeroxe · Dec 10, 2017

book/src/pong_tutorial/pong_tutorial_03.md

+
+```rust,ignore
+fn on_start(&mut self, world: &mut World) {
+    world.add_resource(Time::default()); //Add this line


This is redundant. The engine does this automatically.

Good to know! Fixed.

This looks great, thank you!

Reviewed 6 of 8 files at r1, 2 of 2 files at r2.
Review status: all files reviewed at latest revision, 8 unresolved discussions.

book/src/pong_tutorial/pong_tutorial_03.md, line 31 at r2 (raw file):

[Here's the relevant part of the The Rust Book][lifetimes], if you want a 
refresher. But basically, all we're doing is saying this method has a lifetime 
`s`, and all the data this method relies on needs to live as long as `s`. This

Not sure if this is the right place, but we should note that a system needs to implement System<'a> for every possible 'a. This also implies that a system which contains a reference may not have its System implementation bound on that reference (but on a lifetime that has no bounds).

book/src/pong_tutorial/pong_tutorial_03.md, line 40 at r2 (raw file):

First off we need to define the SystemData, which is a tuple of items which

Might be worth to point to the according Specs chapter. In case it's missing something, we should add it (I'll have to check myself, it's been a while since I read that chapter).

book/src/pong_tutorial/pong_tutorial_03.md, line 55 at r2 (raw file):

that this `SystemData` tuple is a nice and succinct description of what we want 
to do with this `System`: we want to change the `LocalTransform` of a `Paddle` 
over `Time` based on keyboard `Input`.

Not sure why you put backticks around Input.

book/src/pong_tutorial/pong_tutorial_03.md, line 91 at r2 (raw file):

            width: PADDLE_WIDTH,
            height: PADDLE_HEIGHT,
            velocity: PADDLE_VELOCITY

nit: missing trailing comma

book/src/pong_tutorial/pong_tutorial_03.md, line 207 at r2 (raw file):

The real heart of this code is the mutation of `transform.translation.y`.
`transform.translation` is a `Vector3`, and we can access its members by name 
(`.x`,`.y`,`.z`) or position (`[0]`,`[1]`,`[2]`). The `.max` and `.min` methods

I think position should be "index"

examples/pong_tutorial_03/pong.rs, line 61 at r2 (raw file):

            width: PADDLE_WIDTH,
            height: PADDLE_HEIGHT,
            velocity: PADDLE_VELOCITY

nit: missing trailing comma

Comments from Reviewable

torkleyy · Dec 10, 2017

This looks great, thank you!

Reviewed 6 of 8 files at r1, 2 of 2 files at r2.
Review status: all files reviewed at latest revision, 8 unresolved discussions.

book/src/pong_tutorial/pong_tutorial_03.md, line 31 at r2 (raw file):

[Here's the relevant part of the The Rust Book][lifetimes], if you want a 
refresher. But basically, all we're doing is saying this method has a lifetime 
`s`, and all the data this method relies on needs to live as long as `s`. This

Not sure if this is the right place, but we should note that a system needs to implement System<'a> for every possible 'a. This also implies that a system which contains a reference may not have its System implementation bound on that reference (but on a lifetime that has no bounds).

book/src/pong_tutorial/pong_tutorial_03.md, line 40 at r2 (raw file):

First off we need to define the SystemData, which is a tuple of items which

Might be worth to point to the according Specs chapter. In case it's missing something, we should add it (I'll have to check myself, it's been a while since I read that chapter).

book/src/pong_tutorial/pong_tutorial_03.md, line 55 at r2 (raw file):

that this `SystemData` tuple is a nice and succinct description of what we want 
to do with this `System`: we want to change the `LocalTransform` of a `Paddle` 
over `Time` based on keyboard `Input`.

Not sure why you put backticks around Input.

book/src/pong_tutorial/pong_tutorial_03.md, line 91 at r2 (raw file):

            width: PADDLE_WIDTH,
            height: PADDLE_HEIGHT,
            velocity: PADDLE_VELOCITY

nit: missing trailing comma

book/src/pong_tutorial/pong_tutorial_03.md, line 207 at r2 (raw file):

The real heart of this code is the mutation of `transform.translation.y`.
`transform.translation` is a `Vector3`, and we can access its members by name 
(`.x`,`.y`,`.z`) or position (`[0]`,`[1]`,`[2]`). The `.max` and `.min` methods

I think position should be "index"

examples/pong_tutorial_03/pong.rs, line 61 at r2 (raw file):

            width: PADDLE_WIDTH,
            height: PADDLE_HEIGHT,
            velocity: PADDLE_VELOCITY

nit: missing trailing comma

Comments from Reviewable

Here's the Specs chapter: https://slide-rs.github.io/specs-website/docs/book/master/06_system_data.html

So it turns out I never finished it :( Some things I'd like to see in there:

Which types implement SystemData (Fetch, ReadStorage, ..)
How to implement it yourself
The LazyUpdate resource

torkleyy · Dec 10, 2017

Here's the Specs chapter: https://slide-rs.github.io/specs-website/docs/book/master/06_system_data.html

So it turns out I never finished it :( Some things I'd like to see in there:

Which types implement SystemData (Fetch, ReadStorage, ..)
How to implement it yourself
The LazyUpdate resource

@torkleyy good thing @futurepaul comes along and give us all a kick in the butt to update things, eh? ;)

Rhuagh · Dec 10, 2017

@torkleyy good thing @futurepaul comes along and give us all a kick in the butt to update things, eh? ;)

Review status: all files reviewed at latest revision, 11 unresolved discussions.

book/src/pong_tutorial/pong_tutorial_03.md, line 10 at r2 (raw file):

First let's make a new file called `paddle.rs` which will hold our 
`PaddleSystem` struct and implementation. Remember how ECS stands for "Entity-
component system"? Well, this is the "system" part. Systems iterate through

Entity-Component-System maybe ?

book/src/pong_tutorial/pong_tutorial_03.md, line 45 at r2 (raw file):

```rust,ignore
type SystemData = (
    WriteStorage<'s, Paddle>,

This can be ReadStorage actually. I realise it ain't in the actual pong example.

book/src/pong_tutorial/pong_tutorial_03.md, line 184 at r2 (raw file):

// Iterate over all planks and move them according to the input the user
// provided.
for (paddle, transform) in (&mut paddles, &mut transforms).join() {

Should we talk about Join?

Comments from Reviewable

Rhuagh · Dec 10, 2017

Review status: all files reviewed at latest revision, 11 unresolved discussions.

book/src/pong_tutorial/pong_tutorial_03.md, line 10 at r2 (raw file):

First let's make a new file called `paddle.rs` which will hold our 
`PaddleSystem` struct and implementation. Remember how ECS stands for "Entity-
component system"? Well, this is the "system" part. Systems iterate through

Entity-Component-System maybe ?

book/src/pong_tutorial/pong_tutorial_03.md, line 45 at r2 (raw file):

```rust,ignore
type SystemData = (
    WriteStorage<'s, Paddle>,

This can be ReadStorage actually. I realise it ain't in the actual pong example.

book/src/pong_tutorial/pong_tutorial_03.md, line 184 at r2 (raw file):

// Iterate over all planks and move them according to the input the user
// provided.
for (paddle, transform) in (&mut paddles, &mut transforms).join() {

Should we talk about Join?

Comments from Reviewable

Reviewed 6 of 8 files at r1, 2 of 2 files at r2.
Review status: all files reviewed at latest revision, 11 unresolved discussions.

Comments from Reviewable

Rhuagh · Dec 10, 2017

Reviewed 6 of 8 files at r1, 2 of 2 files at r2.
Review status: all files reviewed at latest revision, 11 unresolved discussions.

Comments from Reviewable

Review status: all files reviewed at latest revision, 11 unresolved discussions.

book/src/pong_tutorial/pong_tutorial_03.md, line 10 at r2 (raw file):

Previously, Rhuagh (Simon Rönnberg) wrote…

Entity-Component-System maybe ?

The correct term is Entity-Component System AFAIK, dunno why.

Comments from Reviewable

torkleyy · Dec 11, 2017

Review status: all files reviewed at latest revision, 11 unresolved discussions.

book/src/pong_tutorial/pong_tutorial_03.md, line 10 at r2 (raw file):

Previously, Rhuagh (Simon Rönnberg) wrote…

Entity-Component-System maybe ?

The correct term is Entity-Component System AFAIK, dunno why.

Comments from Reviewable

Xaeroxe · Dec 11, 2017

Xaeroxe approved these changes Dec 11, 2017

View changes

I've commented on all concerns so you have a nice summary @futurepaul.

Review status: all files reviewed at latest revision, 9 unresolved discussions.

book/src/pong_tutorial/pong_tutorial_03.md, line 10 at r2 (raw file):

Previously, torkleyy (Thomas Schaller) wrote…

The correct term is Entity-Component System AFAIK, dunno why.

Please respond or acknowledge.

book/src/pong_tutorial/pong_tutorial_03.md, line 31 at r2 (raw file):

Previously, torkleyy (Thomas Schaller) wrote…

Not sure if this is the right place, but we should note that a system needs to implement System<'a> for every possible 'a. This also implies that a system which contains a reference may not have its System implementation bound on that reference (but on a lifetime that has no bounds).

I'm rather neutral on this, any other opinions on this?

book/src/pong_tutorial/pong_tutorial_03.md, line 40 at r2 (raw file):

Previously, torkleyy (Thomas Schaller) wrote…

Might be worth to point to the according Specs chapter. In case it's missing something, we should add it (I'll have to check myself, it's been a while since I read that chapter).

OK, I don't think this is necessary for this PR, I want to improve the Specs book before we link to it everywhere.

book/src/pong_tutorial/pong_tutorial_03.md, line 45 at r2 (raw file):

Previously, Rhuagh (Simon Rönnberg) wrote…

This can be ReadStorage actually. I realise it ain't in the actual pong example.

This has been changed in the example and should be solved here, too.

book/src/pong_tutorial/pong_tutorial_03.md, line 55 at r2 (raw file):

Previously, torkleyy (Thomas Schaller) wrote…

Not sure why you put backticks around Input.

This is still unresolved.

book/src/pong_tutorial/pong_tutorial_03.md, line 91 at r2 (raw file):

Previously, torkleyy (Thomas Schaller) wrote…

nit: missing trailing comma

This is still unresolved.

book/src/pong_tutorial/pong_tutorial_03.md, line 184 at r2 (raw file):

Previously, Rhuagh (Simon Rönnberg) wrote…

Should we talk about Join?

A short reference to it wouldn't be bad IMO.

book/src/pong_tutorial/pong_tutorial_03.md, line 207 at r2 (raw file):

Previously, torkleyy (Thomas Schaller) wrote…

I think position should be "index"

This is still unresolved.

examples/pong_tutorial_03/pong.rs, line 61 at r2 (raw file):

Previously, torkleyy (Thomas Schaller) wrote…

nit: missing trailing comma

This is still unresolved.

Comments from Reviewable

torkleyy · Dec 12, 2017

I've commented on all concerns so you have a nice summary @futurepaul.

Review status: all files reviewed at latest revision, 9 unresolved discussions.

book/src/pong_tutorial/pong_tutorial_03.md, line 10 at r2 (raw file):

Previously, torkleyy (Thomas Schaller) wrote…

The correct term is Entity-Component System AFAIK, dunno why.

Please respond or acknowledge.

book/src/pong_tutorial/pong_tutorial_03.md, line 31 at r2 (raw file):

Previously, torkleyy (Thomas Schaller) wrote…

Not sure if this is the right place, but we should note that a system needs to implement System<'a> for every possible 'a. This also implies that a system which contains a reference may not have its System implementation bound on that reference (but on a lifetime that has no bounds).

I'm rather neutral on this, any other opinions on this?

book/src/pong_tutorial/pong_tutorial_03.md, line 40 at r2 (raw file):

Previously, torkleyy (Thomas Schaller) wrote…

Might be worth to point to the according Specs chapter. In case it's missing something, we should add it (I'll have to check myself, it's been a while since I read that chapter).

OK, I don't think this is necessary for this PR, I want to improve the Specs book before we link to it everywhere.

book/src/pong_tutorial/pong_tutorial_03.md, line 45 at r2 (raw file):

Previously, Rhuagh (Simon Rönnberg) wrote…

This can be ReadStorage actually. I realise it ain't in the actual pong example.

This has been changed in the example and should be solved here, too.

book/src/pong_tutorial/pong_tutorial_03.md, line 55 at r2 (raw file):

Previously, torkleyy (Thomas Schaller) wrote…

Not sure why you put backticks around Input.

This is still unresolved.

book/src/pong_tutorial/pong_tutorial_03.md, line 91 at r2 (raw file):

Previously, torkleyy (Thomas Schaller) wrote…

nit: missing trailing comma

This is still unresolved.

book/src/pong_tutorial/pong_tutorial_03.md, line 184 at r2 (raw file):

Previously, Rhuagh (Simon Rönnberg) wrote…

Should we talk about Join?

A short reference to it wouldn't be bad IMO.

book/src/pong_tutorial/pong_tutorial_03.md, line 207 at r2 (raw file):

Previously, torkleyy (Thomas Schaller) wrote…

I think position should be "index"

This is still unresolved.

examples/pong_tutorial_03/pong.rs, line 61 at r2 (raw file):

Previously, torkleyy (Thomas Schaller) wrote…

nit: missing trailing comma

This is still unresolved.

Comments from Reviewable

Bad robot

Xaeroxe · Jan 6, 2018

Bad robot

Review status: all files reviewed at latest revision, 8 unresolved discussions.

book/src/pong_tutorial/pong_tutorial_03.md, line 31 at r2 (raw file):

Previously, torkleyy (Thomas Schaller) wrote…

I'm rather neutral on this, any other opinions on this?

As you say, it should be described somewhere, but I don't think this tutorial is the correct place for that information. Maybe in the API docs for System?

book/src/pong_tutorial/pong_tutorial_03.md, line 40 at r2 (raw file):

Previously, torkleyy (Thomas Schaller) wrote…

OK, I don't think this is necessary for this PR, I want to improve the Specs book before we link to it everywhere.

Specs book is updated I believe.

book/src/pong_tutorial/pong_tutorial_03.md, line 45 at r2 (raw file):

Previously, torkleyy (Thomas Schaller) wrote…

This has been changed in the example and should be solved here, too.

Yes, please change this to ReadStorage<'s, Paddle>

book/src/pong_tutorial/pong_tutorial_03.md, line 184 at r2 (raw file):

Previously, torkleyy (Thomas Schaller) wrote…

A short reference to it wouldn't be bad IMO.

A reference to the specs book chapter that takes about Join would be good here.

Comments from Reviewable

Rhuagh · Jan 26, 2018

Review status: all files reviewed at latest revision, 8 unresolved discussions.

book/src/pong_tutorial/pong_tutorial_03.md, line 31 at r2 (raw file):

Previously, torkleyy (Thomas Schaller) wrote…

I'm rather neutral on this, any other opinions on this?

As you say, it should be described somewhere, but I don't think this tutorial is the correct place for that information. Maybe in the API docs for System?

book/src/pong_tutorial/pong_tutorial_03.md, line 40 at r2 (raw file):

Previously, torkleyy (Thomas Schaller) wrote…

OK, I don't think this is necessary for this PR, I want to improve the Specs book before we link to it everywhere.

Specs book is updated I believe.

book/src/pong_tutorial/pong_tutorial_03.md, line 45 at r2 (raw file):

Previously, torkleyy (Thomas Schaller) wrote…

This has been changed in the example and should be solved here, too.

Yes, please change this to ReadStorage<'s, Paddle>

book/src/pong_tutorial/pong_tutorial_03.md, line 184 at r2 (raw file):

Previously, torkleyy (Thomas Schaller) wrote…

A short reference to it wouldn't be bad IMO.

A reference to the specs book chapter that takes about Join would be good here.

Comments from Reviewable

@futurepaul, do you intend to follow through with this PR? I was interested in expanding the book with chapter 3, but if this is already done, I would love to keep going, and write some of the other chapters.

akatechis · Jan 30, 2018

@futurepaul, do you intend to follow through with this PR? I was interested in expanding the book with chapter 3, but if this is already done, I would love to keep going, and write some of the other chapters.

Feel free to take over this PR I guess, we've not heard from @futurepaul in quite some time

Review status: all files reviewed at latest revision, 8 unresolved discussions, some commit checks failed.

Comments from Reviewable

Rhuagh · Feb 1, 2018

Feel free to take over this PR I guess, we've not heard from @futurepaul in quite some time

Review status: all files reviewed at latest revision, 8 unresolved discussions, some commit checks failed.

Comments from Reviewable

@Rhuagh Any idea if @akatechis took this over? I can take it over as well, but don't want to step on anyone's toes.

Hittherhod · Feb 14, 2018

@Rhuagh Any idea if @akatechis took this over? I can take it over as well, but don't want to step on anyone's toes.

@Hittherhod, yes, I'm about to submit the PR for this, actually. Took a little longer than I expected, mainly because I was dealing with a couple other projects that came across my way.

akatechis · Feb 14, 2018

@Hittherhod, yes, I'm about to submit the PR for this, actually. Took a little longer than I expected, mainly because I was dealing with a couple other projects that came across my way.

Continued in another PR.

Rhuagh · Feb 14, 2018

Continued in another PR.

futurepaul added some commits Dec 9, 2017

chapter_03 first draft

0bb0a14

Add tutorial_03 code

f2114bc

Add tutorial_03 example listing

c97b6db

Add chapter_03 to summary

Loading status checks…

fb2060c

futurepaul referenced this pull request Dec 10, 2017
Open
Pong tutorial progress! #506
2 of 7 tasks complete

Remove redundant Time and fix typo

Loading status checks…

2fd318e

Aceeri added the project: docs label Dec 10, 2017

torkleyy added status: ready type: improvement labels Dec 10, 2017

bors bot closed this Jan 6, 2018

Xaeroxe reopened this Jan 6, 2018

Rhuagh closed this Feb 14, 2018

amethyst/amethyst

Join GitHub today

Pong tutorial chapter 03 #507

Conversation

futurepaul commented Dec 10, 2017 • edited by torkleyy Edited 1 time torkleyy edited Dec 10, 2017 (most recent)

futurepaul added some commits Dec 9, 2017

futurepaul referenced this pull request Dec 10, 2017

Xaeroxe requested changes Dec 10, 2017 View changes

This comment has been minimized.

This comment has been minimized.

Xaeroxe Dec 10, 2017 • edited Edited 1 time Xaeroxe edited Dec 10, 2017 (most recent)

This comment has been minimized.

Aceeri added the project: docs label Dec 10, 2017

torkleyy added status: ready type: improvement labels Dec 10, 2017

This comment has been minimized.

torkleyy commented Dec 10, 2017

This comment has been minimized.

torkleyy commented Dec 10, 2017

This comment has been minimized.

Rhuagh commented Dec 10, 2017

This comment has been minimized.

Rhuagh commented Dec 10, 2017

This comment has been minimized.

Rhuagh commented Dec 10, 2017

This comment has been minimized.

torkleyy commented Dec 11, 2017

Xaeroxe approved these changes Dec 11, 2017 View changes

This comment has been minimized.

torkleyy commented Dec 12, 2017

bors bot closed this Jan 6, 2018

This comment has been minimized.

Xaeroxe commented Jan 6, 2018

Xaeroxe reopened this Jan 6, 2018

This comment has been minimized.

Rhuagh commented Jan 26, 2018

This comment has been minimized.

akatechis commented Jan 30, 2018

This comment has been minimized.

Rhuagh commented Feb 1, 2018

This comment has been minimized.

Hittherhod commented Feb 14, 2018

This comment has been minimized.

akatechis commented Feb 14, 2018

This comment has been minimized.

Rhuagh commented Feb 14, 2018

Rhuagh closed this Feb 14, 2018

futurepaul commented Dec 10, 2017 •

edited by torkleyy

Edited 1 time

torkleyy edited Dec 10, 2017 (most recent)

Xaeroxe requested changes Dec 10, 2017

View changes

Xaeroxe Dec 10, 2017 •

edited

Edited 1 time

Xaeroxe edited Dec 10, 2017 (most recent)

Xaeroxe approved these changes Dec 11, 2017

View changes