Join GitHub today
GitHub is home to over 28 million developers working together to host and review code, manage projects, and build software together.
Sign upfix: Rest state for animation needs to be controllable. #611
Conversation
| + F: Fn(Entity) -> Option<T>, | ||
| + { | ||
| + for entity in self.nodes.values() { | ||
| + if let None = states.get(*entity) { |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
jojolepro
Mar 28, 2018
Collaborator
just a preference, but when checking for none I prefer using
if states.get(*entity).is_none()
as this shows a little bit better the intent. Just my opinion, feel free to ignore.
jojolepro
Mar 28, 2018
Collaborator
just a preference, but when checking for none I prefer using
if states.get(*entity).is_none()
as this shows a little bit better the intent. Just my opinion, feel free to ignore.
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
| @@ -339,8 +339,8 @@ mod tests { | ||
| match TextureData::from([0.25, 0.50, 0.75]) { | ||
| TextureData::Rgba(color, _) => { | ||
| assert_eq!(color, [0.25, 0.50, 0.75, 1.0]); | ||
| - }, | ||
| - _ => panic!("Expected [f32; 3] to turn into TextureData::Rgba") | ||
| + } |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
jojolepro
Mar 28, 2018
Collaborator
Maybe I should write more helpful messages than "hum" when I review. I was pointing out that the comma was removed, and implicitly asking if that was intended or not. I guess I'll work on communication a bit more :)
jojolepro
Mar 28, 2018
Collaborator
Maybe I should write more helpful messages than "hum" when I review. I was pointing out that the comma was removed, and implicitly asking if that was intended or not. I guess I'll work on communication a bit more :)
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
jojolepro
Mar 29, 2018
Collaborator
if you are using the latest version, then I assume its fine and you can ignore this comment ;)
jojolepro
Mar 29, 2018
Collaborator
if you are using the latest version, then I assume its fine and you can ignore this comment ;)
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
torkleyy
added
type: bug
status: ready
project: animation
labels
Apr 3, 2018
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
torkleyy
Apr 3, 2018
Member
Reviewed 3 of 5 files at r1, 1 of 2 files at r2.
Review status: 4 of 5 files reviewed at latest revision, 2 unresolved discussions.
Comments from Reviewable
|
Reviewed 3 of 5 files at r1, 1 of 2 files at r2. Comments from Reviewable |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
|
bors r+ |
bot
added a commit
that referenced
this pull request
Apr 3, 2018
![@bors[bot]](https://avatars3.githubusercontent.com/in/1847?s=60&v=4){kind=small}
Rhuagh commentedMar 28, 2018
•
edited by torkleyy
Edited 1 time
-
torkleyy
edited Mar 28, 2018 (most recent)
Will now be set to the state of the base component on the first animation, unless it has already been set.
The previous solution caused the end state to become incorrect for staggered animatinos.
This change is