Skip to content

Conversation

@just-do-halee
Copy link
Contributor

No description provided.

Copy link
Owner

@CleanCut CleanCut left a comment

Choose a reason for hiding this comment

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

Cool! I think these are great as additional features!

I do not feel that these are easier to use than the existing features, so I have reverted most of the examples, leaving a commented-out example in examples/keyboard_state.rs as a teaser. I feel like this is a nice "discoverable" feature for more advanced/ambitious students.

Question: I noticed that in your own conversion of the examples, you never needed to make use of the &MouseState or &KeyboardState that you passed to the closure. Are you certain that you don't want to remove that parameter? I wouldn't need it for the simple scenarios I lead students through, but perhaps you find it useful? If you would like to make that change, I'll wait for you. If you like it how it is, I'll go ahead and merge this now. Let me know!

@just-do-halee
Copy link
Contributor Author

just-do-halee commented Sep 9, 2022

Cool! I think these are great as additional features!

  • Thank you So much! 😍

I do not feel that these are easier to use than the existing features, so I have reverted most of the examples, leaving a commented-out example in examples/keyboard_state.rs as a teaser. I feel like this is a nice "discoverable" feature for more advanced/ambitious students.

  • I agree that a nice "discoverable" feature for more advanced/ambitious students Yeah, you are always a genius teacher. Thank you so much! And Thank you for commenting on it. It's really awesome! 😆

Question: I noticed that in your own conversion of the examples, you never needed to make use of the &MouseState or &KeyboardState that you passed to the closure. Are you certain that you don't want to remove that parameter? I wouldn't need it for the simple scenarios I lead students through, but perhaps you find it useful? If you would like to make that change, I'll wait for you. If you like it how it is, I'll go ahead and merge this now. Let me know!

  • You're right! So I reverted the last commit once again but... then i realized that i commited it because of the timestamp 1:30 of 34. Tutorial: Mouse Input in the Ultimate Rust Crash Course 2. mouse_state has location() and motion() and wheel(). And i noticed that, How can i make nested conditions if i want to? Then also i was feel that using the borrowed state as an argument is much more idiomatic way to have functionality on Rust. Well, but also as a student, you are right! So I'm not sure what can i do. * The hidden problem is that we can not use &mut Engine inside that closure... But It's not bad noticing pros and cons on Rust functional programming, I was thinking.

Anyway, I love your all lectures on Udemy. Teacher Nathan!
I'd started learning Rust-lang 1 year and half months ago and it was from your first Ultimate Rust Crash Course.
I've been shocked how the teacher can explain and show the illustrations so, so, so easy to understand and then I became a fast rustacean learner thanks to you! Teacher Nathan.
I didn't Ultimate Rust Crash Course 2 in that time, because there wasn't,
but Hands-On Systems programming with Rust.

I recently started Ultimate Rust Crash Course 2. 95% completed. And I can say that it is incredibly amazing lecture too!
Thank you for taking a closer look at my Pull-Request and commenting!

@just-do-halee
Copy link
Contributor Author

just-do-halee commented Sep 9, 2022

The &mut Engine is now available in the closures... And I removed the method name _do on the chainer(_if().).

The argument in the closure, |&KeyboardState| and |&MouseState|, I'll leave the choice to you.... My opinion is

  1. It enables nested conditions.
  2. You may need to use several MouseState dependent methods in the MouseStateChainer closure. (It can also occur later in the KeyboardState, we don't know)

…ressed[_any](..) now you can use &mut Engine inside the closure
@just-do-halee just-do-halee force-pushed the main branch 2 times, most recently from 8be6a1f to 41db2aa Compare September 11, 2022 00:15
@CleanCut
Copy link
Owner

Alright, I left the reference parameters in.

I tweaked the names a little bit. I renamed if_ to chain(), and I renamed *Chainer to *Chain. I think that's a bit more consistent. I also updated the changelog entry.

@CleanCut CleanCut merged commit ff25b47 into CleanCut:main Sep 13, 2022
@CleanCut
Copy link
Owner

Included in version 5.2.0, just released. 😄

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.

2 participants