Feature/634/sprite rendering and animation #638

azriel91 · Apr 12, 2018

Attempt at sprite animation: #634. This is branched off #641.

This change is

woo it works 🎉

azriel91 · Apr 13, 2018

woo it works 🎉

WIP: rebasing over Rhuagh's animation implementation.

azriel91 · Apr 14, 2018

WIP: rebasing over Rhuagh's animation implementation.

Updated to work with the new animation types, had to edit the Flat passes, I'm not sure if I should've used the function or just added the AlbedoOffset buffer. Screenshot is on the first comment.

Appveyor build failure is due to something wrong with the window installer download

azriel91 · Apr 15, 2018

Updated to work with the new animation types, had to edit the Flat passes, I'm not sure if I should've used the function or just added the AlbedoOffset buffer. Screenshot is on the first comment.

Appveyor build failure is due to something wrong with the window installer download

So we're to merge #641, followed by #644, followed by this?

Ping @Rhuagh to confirm, I haven't been following gitter as well as I should.

Xaeroxe · Apr 17, 2018

So we're to merge #641, followed by #644, followed by this?

Ping @Rhuagh to confirm, I haven't been following gitter as well as I should.

Yeaps 😄

#641 adds the material animation (texture, and texture coordinates)
#644 adds delayed start over animations
This adds sprites, and takes advantage of the material animation to pick a texture and shift the texture coordinates.

I had this idea, though not sure if I'll do it soon -- I have a self-imposed deadline for something in 1.5 weeks:

Split the bat sprite sheet into two
Make the grey bat evolve into the brown bat by swapping the texture id in the animation

azriel91 · Apr 17, 2018

Yeaps 😄

#641 adds the material animation (texture, and texture coordinates)
#644 adds delayed start over animations
This adds sprites, and takes advantage of the material animation to pick a texture and shift the texture coordinates.

I had this idea, though not sure if I'll do it soon -- I have a self-imposed deadline for something in 1.5 weeks:

Split the bat sprite sheet into two
Make the grey bat evolve into the brown bat by swapping the texture id in the animation

Ready for review

azriel91 · Apr 22, 2018

Ready for review

jojolepro · Apr 23, 2018

jojolepro requested changes Apr 23, 2018

View changes

This looks pretty good, nice job!
A much needed feature. 👍

jojolepro · Apr 23, 2018

Cargo.toml

@@ -59,6 +59,7 @@ thread_profiler = { version = "0.1", optional = true }
 amethyst_gltf = { path = "amethyst_gltf", version = "0.1" }
 genmesh = "0.5"
 amethyst_animation = { path = "amethyst_animation", version = "0.1.0" }
+ron = "0.1"


ron version is 0.2, any reason to use 0.1?

To keep it consistent with amethyst_assets and amethyst_config. Generally within a repo I try to keep the same dependency versions

okay, then we might want to update it in a future PR. For this one you can keep 0.1

jojolepro · Apr 23, 2018

amethyst_renderer/src/sprite.rs

+/// These should be in normalized coordinates:
+///
+/// * X axis: 0.0 is the left side and 1.0 is the right side.
+/// * Y axis: 0.0 is the top and 1.0 is the botoom.


fixed! 😅

jojolepro · Apr 23, 2018

amethyst_renderer/src/sprite.rs

+/// * X axis: 0.0 is the left side and 1.0 is the right side.
+/// * Y axis: 0.0 is the top and 1.0 is the botoom.
+#[derive(Clone, Debug)]
+pub struct Sprite {


An implement of the From trait for ((f32,f32),(f32,f32)) and [f32,4] could be useful I think.

jojolepro · Apr 23, 2018

examples/sprites/main.rs

+use amethyst_animation::{get_animation_set, AnimationBundle, AnimationCommand, EndControl,
+                         MaterialTextureSet};
+
+const BACKGROUND_COLOUR: [f32; 4] = [0.0, 0.0, 0.0, 0.0]; // black


Actually that's transparent ;)

jojolepro · Apr 23, 2018

examples/sprites/main.rs

+            let entity = world
+                .create_entity()
+                // The default `Material`, whose textures will be swapped based on the animation.
+                .with(sprite_sheet_material.clone())


Is this clone necessary?

Yeap, because the loop creates multiple entities, and each of them have their own material component

I didn't realized it was a loop

jojolepro · Apr 23, 2018

examples/sprites/sprite_sheet_loader.rs

+///
+/// * `index`: Index of the sprite sheet's texture in the `MaterialTextureSet`.
+/// * `definition`: Definition of the sprite layout on the sprite sheet.
+pub fn load(index: usize, definition: &sprite::SpriteSheetDefinition) -> SpriteSheet {


Shouldn't that be directly into the engine? It seems pretty inconvenient to code this each time you make a new project.

the SpriteSheetDefinition is user defined, based on #634 (comment), so it has to be outside

okay, I'll make myself a predefined one then ;)

Xaeroxe · Apr 23, 2018

Xaeroxe approved these changes Apr 23, 2018

View changes

I have nothing more to add, @jojolepro already said everything I saw.

jojolepro · Apr 23, 2018

jojolepro approved these changes Apr 23, 2018

View changes

Thanks you!

bors r=@jojolepro, @Xaeroxe

Xaeroxe · Apr 23, 2018

bors r=@jojolepro, @Xaeroxe

Build succeeded

bors · Apr 23, 2018

Build succeeded

Congratz!

jojolepro · Apr 24, 2018

Congratz!

azriel91 changed the title from WIP: Feature/634/sprite rendering and animation to Feature/634/sprite rendering and animation Apr 13, 2018

azriel91 referenced this pull request Apr 13, 2018
Closed
Discussion: Example/sprite render #624

Xaeroxe self-requested a review Apr 13, 2018

azriel91 changed the title from Feature/634/sprite rendering and animation to [WIP] Feature/634/sprite rendering and animation Apr 14, 2018

azriel91 referenced this pull request Apr 15, 2018
Merged
feat: Add texture animation #641

azriel91 changed the title from [WIP] Feature/634/sprite rendering and animation to Feature/634/sprite rendering and animation Apr 15, 2018

bors bot merged commit d8bf0e0 into amethyst:develop Apr 23, 2018
3 checks passed

3 checks passed

bors Build succeeded
Details

continuous-integration/appveyor/pr AppVeyor build succeeded
Details

continuous-integration/travis-ci/pr The Travis CI build passed
Details

azriel91 deleted the azriel91:feature/634/sprite-rendering-and-animation branch Apr 24, 2018

azriel91 referenced this pull request Apr 25, 2018
Closed
[RFC] Support for 2D Sprite Rendering and Animation #634
4 of 4 tasks complete

amethyst/amethyst

Join GitHub today

Feature/634/sprite rendering and animation #638

Conversation

azriel91 commented Apr 12, 2018 • edited Edited 4 times azriel91 edited Apr 22, 2018 (most recent) azriel91 edited Apr 15, 2018 azriel91 edited Apr 13, 2018 torkleyy edited Apr 12, 2018

azriel91 changed the title from WIP: Feature/634/sprite rendering and animation to Feature/634/sprite rendering and animation Apr 13, 2018

This comment has been minimized.

azriel91 commented Apr 13, 2018 • edited Edited 1 time azriel91 edited Apr 13, 2018 (most recent)

azriel91 referenced this pull request Apr 13, 2018

Xaeroxe self-requested a review Apr 13, 2018

azriel91 changed the title from Feature/634/sprite rendering and animation to [WIP] Feature/634/sprite rendering and animation Apr 14, 2018

This comment has been minimized.

azriel91 commented Apr 14, 2018

azriel91 referenced this pull request Apr 15, 2018

azriel91 changed the title from [WIP] Feature/634/sprite rendering and animation to Feature/634/sprite rendering and animation Apr 15, 2018

This comment has been minimized.

azriel91 commented Apr 15, 2018 • edited Edited 1 time azriel91 edited Apr 15, 2018 (most recent)

This comment has been minimized.

Xaeroxe commented Apr 17, 2018

This comment has been minimized.

azriel91 commented Apr 17, 2018 • edited Edited 1 time azriel91 edited Apr 17, 2018 (most recent)

This comment has been minimized.

azriel91 commented Apr 22, 2018

jojolepro requested changes Apr 23, 2018 View changes

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Xaeroxe approved these changes Apr 23, 2018 View changes

azriel91 added some commits Apr 11, 2018

jojolepro approved these changes Apr 23, 2018 View changes

This comment has been minimized.

Xaeroxe commented Apr 23, 2018

bors bot added a commit that referenced this pull request Apr 23, 2018

This comment has been minimized.

bors bot Apr 23, 2018

Build succeeded

bors bot commented Apr 23, 2018

Build succeeded

Hide details View details bors bot merged commit d8bf0e0 into amethyst:develop Apr 23, 2018 3 checks passed

3 checks passed

azriel91 deleted the azriel91:feature/634/sprite-rendering-and-animation branch Apr 24, 2018

This comment has been minimized.

jojolepro commented Apr 24, 2018

azriel91 referenced this pull request Apr 25, 2018

azriel91 commented Apr 12, 2018 •

edited

Edited 4 times

azriel91 edited Apr 22, 2018 (most recent)

azriel91 edited Apr 15, 2018

azriel91 edited Apr 13, 2018

torkleyy edited Apr 12, 2018

azriel91 commented Apr 13, 2018 •

edited

Edited 1 time

azriel91 edited Apr 13, 2018 (most recent)

azriel91 commented Apr 15, 2018 •

edited

Edited 1 time

azriel91 edited Apr 15, 2018 (most recent)

azriel91 commented Apr 17, 2018 •

edited

Edited 1 time

azriel91 edited Apr 17, 2018 (most recent)

jojolepro requested changes Apr 23, 2018

View changes

Xaeroxe approved these changes Apr 23, 2018

View changes

jojolepro approved these changes Apr 23, 2018

View changes

bors bot merged commit `d8bf0e0` into amethyst:develop Apr 23, 2018
3 checks passed