[WIP] Texture animation #505

Xaeroxe · Dec 7, 2017

This is a work in progress PR with only data structures at the moment. Processing and generation of these data structures will come later, possibly this weekend. (Written as of 2017-12-07)

My plan for the future is to have the existing passes process these animation values and use them in a vertex shader to modify texture coordinates. This could also be done in a fragment shader. I'm not really picky.

Fixes #510

This change is

Review status: 0 of 3 files reviewed at latest revision, 4 unresolved discussions, some commit checks failed.

amethyst_renderer/src/tex_animation.rs, line 11 at r1 (raw file):

/// Meta data for a sprite sheet texture.  Does not contain a texture, only a description of how
/// a texture can be animated.
#[derive(Clone, Debug)]

I guess you want to implement Serialize and Deserialize here.

amethyst_renderer/src/tex_animation.rs, line 49 at r1 (raw file):

    /// How long the current frame has been played for.
    frame_timer: f32,
}

You also need to specify what to do when animation is finished. Loop, switch to another, stop.

amethyst_renderer/src/tex_animation.rs, line 63 at r1 (raw file):

    pub height: f32,
    /// The duration this frame should be played for in seconds.
    pub duration: f32,

We should prefer to use Duration for durations.
Maybe we want to use our own Duration for efficiency. But f32 leaves you with a choice how to interpretate it.

amethyst_renderer/src/mtl/mod.rs, line 13 at r1 (raw file):

    /// Diffuse map.
    pub albedo: TextureHandle,
    /// Optional animation for albedo

IMO animation isn't part of the material. I'd put it to the separate component.

Comments from Reviewable

omni-viral · Dec 9, 2017

Review status: 0 of 3 files reviewed at latest revision, 4 unresolved discussions, some commit checks failed.

amethyst_renderer/src/tex_animation.rs, line 11 at r1 (raw file):

/// Meta data for a sprite sheet texture.  Does not contain a texture, only a description of how
/// a texture can be animated.
#[derive(Clone, Debug)]

I guess you want to implement Serialize and Deserialize here.

amethyst_renderer/src/tex_animation.rs, line 49 at r1 (raw file):

    /// How long the current frame has been played for.
    frame_timer: f32,
}

You also need to specify what to do when animation is finished. Loop, switch to another, stop.

amethyst_renderer/src/tex_animation.rs, line 63 at r1 (raw file):

    pub height: f32,
    /// The duration this frame should be played for in seconds.
    pub duration: f32,

We should prefer to use Duration for durations.
Maybe we want to use our own Duration for efficiency. But f32 leaves you with a choice how to interpretate it.

amethyst_renderer/src/mtl/mod.rs, line 13 at r1 (raw file):

    /// Diffuse map.
    pub albedo: TextureHandle,
    /// Optional animation for albedo

IMO animation isn't part of the material. I'd put it to the separate component.

Comments from Reviewable

amethyst_renderer/src/tex_animation.rs, line 63 at r1 (raw file):

Previously, omni-viral (Zakarum) wrote…

We should prefer to use Duration for durations.
Maybe we want to use our own Duration for efficiency. But f32 leaves you with a choice how to interpretate it.

Not sure I agree with you here. The documentation states this is in seconds so there is no ambiguity in the correct way to interpret it. Using an f32 allows for more efficient calculations later on. We could create our own Duration type holding just an f32 but I think that would be silly, because variable types don't need to contain semantic information, that's what the variable name is for.

Comments from Reviewable

Xaeroxe · Dec 9, 2017

amethyst_renderer/src/tex_animation.rs, line 63 at r1 (raw file):

Previously, omni-viral (Zakarum) wrote…

We should prefer to use Duration for durations.
Maybe we want to use our own Duration for efficiency. But f32 leaves you with a choice how to interpretate it.

Not sure I agree with you here. The documentation states this is in seconds so there is no ambiguity in the correct way to interpret it. Using an f32 allows for more efficient calculations later on. We could create our own Duration type holding just an f32 but I think that would be silly, because variable types don't need to contain semantic information, that's what the variable name is for.

Comments from Reviewable

Review status: 0 of 3 files reviewed at latest revision, 4 unresolved discussions, some commit checks failed.

amethyst_renderer/src/tex_animation.rs, line 63 at r1 (raw file):

Previously, Xaeroxe (Jacob Kiesel) wrote…

Not sure I agree with you here. The documentation states this is in seconds so there is no ambiguity in the correct way to interpret it. Using an f32 allows for more efficient calculations later on. We could create our own Duration type holding just an f32 but I think that would be silly, because variable types don't need to contain semantic information, that's what the variable name is for.

Creating "newtypes" with single type inside to restrict usage and define semantics is what many people do, is what "newtypes" were created for.
Also I prefer u64 for time measurement

Comments from Reviewable

omni-viral · Dec 10, 2017

Review status: 0 of 3 files reviewed at latest revision, 4 unresolved discussions, some commit checks failed.

amethyst_renderer/src/tex_animation.rs, line 63 at r1 (raw file):

Previously, Xaeroxe (Jacob Kiesel) wrote…

Not sure I agree with you here. The documentation states this is in seconds so there is no ambiguity in the correct way to interpret it. Using an f32 allows for more efficient calculations later on. We could create our own Duration type holding just an f32 but I think that would be silly, because variable types don't need to contain semantic information, that's what the variable name is for.

Creating "newtypes" with single type inside to restrict usage and define semantics is what many people do, is what "newtypes" were created for.
Also I prefer u64 for time measurement

Comments from Reviewable

Review status: 0 of 36 files reviewed at latest revision, 4 unresolved discussions.

amethyst_renderer/src/tex_animation.rs, line 11 at r1 (raw file):

Previously, omni-viral (Zakarum) wrote…

I guess you want to implement Serialize and Deserialize here.

I don't think that's necessary if this is an Asset right?

amethyst_renderer/src/tex_animation.rs, line 49 at r1 (raw file):

Previously, omni-viral (Zakarum) wrote…

You also need to specify what to do when animation is finished. Loop, switch to another, stop.

First implementation is to just loop. I'm also planning on sending out events when an animation finishes via shrev so that the end user can implement their own logic for this.

amethyst_renderer/src/tex_animation.rs, line 63 at r1 (raw file):

Previously, omni-viral (Zakarum) wrote…

Creating "newtypes" with single type inside to restrict usage and define semantics is what many people do, is what "newtypes" were created for.
Also I prefer u64 for time measurement

This now uses a std::time::Duration. After thinking about it I determined the f32 doesn't actually help that much with efficiency.

Comments from Reviewable

Xaeroxe · Dec 17, 2017

Review status: 0 of 36 files reviewed at latest revision, 4 unresolved discussions.

amethyst_renderer/src/tex_animation.rs, line 11 at r1 (raw file):

Previously, omni-viral (Zakarum) wrote…

I guess you want to implement Serialize and Deserialize here.

I don't think that's necessary if this is an Asset right?

amethyst_renderer/src/tex_animation.rs, line 49 at r1 (raw file):

Previously, omni-viral (Zakarum) wrote…

You also need to specify what to do when animation is finished. Loop, switch to another, stop.

First implementation is to just loop. I'm also planning on sending out events when an animation finishes via shrev so that the end user can implement their own logic for this.

amethyst_renderer/src/tex_animation.rs, line 63 at r1 (raw file):

Previously, omni-viral (Zakarum) wrote…

Creating "newtypes" with single type inside to restrict usage and define semantics is what many people do, is what "newtypes" were created for.
Also I prefer u64 for time measurement

This now uses a std::time::Duration. After thinking about it I determined the f32 doesn't actually help that much with efficiency.

Comments from Reviewable

amethyst_renderer/src/mtl/mod.rs, line 13 at r1 (raw file):

Previously, omni-viral (Zakarum) wrote…

IMO animation isn't part of the material. I'd put it to the separate component.

Done.

Comments from Reviewable

Xaeroxe · Dec 17, 2017

amethyst_renderer/src/mtl/mod.rs, line 13 at r1 (raw file):

Previously, omni-viral (Zakarum) wrote…

IMO animation isn't part of the material. I'd put it to the separate component.

Done.

Comments from Reviewable

Your work looks great, but one problem I see is that you created a lot of formatting changes. Given that we have @omni-viral writing a renderer rewrite, I don't think pushing formatting changes is a good idea currently.

Reviewed 1 of 47 files at r2.
Review status: 1 of 47 files reviewed at latest revision, 5 unresolved discussions, some commit checks failed.

rustfmt.toml, line 6 at r2 (raw file):

reorder_imports_in_group = true
attributes_on_same_line_as_variant = false
attributes_on_same_line_as_field = false

Why did you remove these?

Comments from Reviewable

torkleyy · Dec 22, 2017

Your work looks great, but one problem I see is that you created a lot of formatting changes. Given that we have @omni-viral writing a renderer rewrite, I don't think pushing formatting changes is a good idea currently.

Reviewed 1 of 47 files at r2.
Review status: 1 of 47 files reviewed at latest revision, 5 unresolved discussions, some commit checks failed.

rustfmt.toml, line 6 at r2 (raw file):

reorder_imports_in_group = true
attributes_on_same_line_as_variant = false
attributes_on_same_line_as_field = false

Why did you remove these?

Comments from Reviewable

Alright, I'll go ahead and remove the formatting changes.

Review status: 1 of 47 files reviewed at latest revision, 5 unresolved discussions, some commit checks failed.

rustfmt.toml, line 6 at r2 (raw file):

Previously, torkleyy (Thomas Schaller) wrote…

Why did you remove these?

Latest version of rustc didn't recognize them as valid options. Some of them were renamed, merged into other options, or just removed because they never worked in the first place.

Comments from Reviewable

Xaeroxe · Dec 22, 2017

Alright, I'll go ahead and remove the formatting changes.

Review status: 1 of 47 files reviewed at latest revision, 5 unresolved discussions, some commit checks failed.

rustfmt.toml, line 6 at r2 (raw file):

Previously, torkleyy (Thomas Schaller) wrote…

Why did you remove these?

Latest version of rustc didn't recognize them as valid options. Some of them were renamed, merged into other options, or just removed because they never worked in the first place.

Comments from Reviewable

Phew, this was a lot to review. I really like it, thank you @Xaeroxe!

Reviewed 46 of 47 files at r2.
Review status: all files reviewed at latest revision, 10 unresolved discussions, some commit checks failed.

amethyst_renderer/src/tex_animation.rs, line 18 at r2 (raw file):

    /// A collection of animations, the first layer contains a list of "animations" and the second
    /// layer contains a list of frame indices within the animation.
    pub animations: Vec<Vec<usize>>,

Please don't nest Vecs like this. I'd suggest you change this to Vec<usize> and add another Vec which just stores the number of frames for a given animation index.

amethyst_renderer/src/tex_animation.rs, line 19 at r2 (raw file):

    /// layer contains a list of frame indices within the animation.
    pub animations: Vec<Vec<usize>>,
    /// A mapping between string names and indexes into the first layer of the animations member.

"indexes"

amethyst_renderer/src/tex_animation.rs, line 66 at r2 (raw file):

    /// ## Returns
    /// If the `starting_animation` wasn't found then this will return `None`.  This could be due
    /// to one of a few things:

Then it should better use a Result.

amethyst_renderer/src/tex_animation.rs, line 100 at r2 (raw file):

        sheet_storage: &AssetStorage<SpriteSheetData>,
        animation: &str,
    ) -> Result<(), ()> {

Just a remainder, this needs an error type (I assume you're aware of this since this is a WIP).

amethyst_renderer/src/tex_animation.rs, line 104 at r2 (raw file):

            .get(&self.sprite_sheet_data)
            .and_then(|data| data.animation_mapping.get(animation).map(|i| *i));
        match animation {

You can match against <above expression>.ok_or(())?

amethyst_renderer/src/mtl/mod.rs, line 34 at r2 (raw file):

/// create a `Material` that has the spritesheets attached in the textures, then provide how
/// those spritesheets are to be animated from this structure.
pub struct MaterialAnimation {

Please derive Default for this.

Comments from Reviewable

torkleyy · Dec 25, 2017

Phew, this was a lot to review. I really like it, thank you @Xaeroxe!

Reviewed 46 of 47 files at r2.
Review status: all files reviewed at latest revision, 10 unresolved discussions, some commit checks failed.

amethyst_renderer/src/tex_animation.rs, line 18 at r2 (raw file):

    /// A collection of animations, the first layer contains a list of "animations" and the second
    /// layer contains a list of frame indices within the animation.
    pub animations: Vec<Vec<usize>>,

Please don't nest Vecs like this. I'd suggest you change this to Vec<usize> and add another Vec which just stores the number of frames for a given animation index.

amethyst_renderer/src/tex_animation.rs, line 19 at r2 (raw file):

    /// layer contains a list of frame indices within the animation.
    pub animations: Vec<Vec<usize>>,
    /// A mapping between string names and indexes into the first layer of the animations member.

"indexes"

amethyst_renderer/src/tex_animation.rs, line 66 at r2 (raw file):

    /// ## Returns
    /// If the `starting_animation` wasn't found then this will return `None`.  This could be due
    /// to one of a few things:

Then it should better use a Result.

amethyst_renderer/src/tex_animation.rs, line 100 at r2 (raw file):

        sheet_storage: &AssetStorage<SpriteSheetData>,
        animation: &str,
    ) -> Result<(), ()> {

Just a remainder, this needs an error type (I assume you're aware of this since this is a WIP).

amethyst_renderer/src/tex_animation.rs, line 104 at r2 (raw file):

            .get(&self.sprite_sheet_data)
            .and_then(|data| data.animation_mapping.get(animation).map(|i| *i));
        match animation {

You can match against <above expression>.ok_or(())?

amethyst_renderer/src/mtl/mod.rs, line 34 at r2 (raw file):

/// create a `Material` that has the spritesheets attached in the textures, then provide how
/// those spritesheets are to be animated from this structure.
pub struct MaterialAnimation {

Please derive Default for this.

Comments from Reviewable

Reviewed 43 of 47 files at r2.
Review status: all files reviewed at latest revision, 10 unresolved discussions, some commit checks failed.

Comments from Reviewable

Rhuagh · Dec 28, 2017

Reviewed 43 of 47 files at r2.
Review status: all files reviewed at latest revision, 10 unresolved discussions, some commit checks failed.

Comments from Reviewable

Review status: 41 of 46 files reviewed at latest revision, 10 unresolved discussions.

amethyst_renderer/src/tex_animation.rs, line 18 at r2 (raw file):

Previously, torkleyy (Thomas Schaller) wrote…

Please don't nest Vecs like this. I'd suggest you change this to Vec<usize> and add another Vec which just stores the number of frames for a given animation index.

That makes the assumption the frames are always sequentially laid out, which while a lot of the time that's true, is not guaranteed, particularly if animations are sharing frames.

amethyst_renderer/src/tex_animation.rs, line 100 at r2 (raw file):

Previously, torkleyy (Thomas Schaller) wrote…

Just a remainder, this needs an error type (I assume you're aware of this since this is a WIP).

I'll make zero sized type for this then

amethyst_renderer/src/tex_animation.rs, line 104 at r2 (raw file):

Previously, torkleyy (Thomas Schaller) wrote…

You can match against <above expression>.ok_or(())?

Ah yes, thank you!

Comments from Reviewable

Xaeroxe · Jan 4, 2018

Review status: 41 of 46 files reviewed at latest revision, 10 unresolved discussions.

amethyst_renderer/src/tex_animation.rs, line 18 at r2 (raw file):

Previously, torkleyy (Thomas Schaller) wrote…

Please don't nest Vecs like this. I'd suggest you change this to Vec<usize> and add another Vec which just stores the number of frames for a given animation index.

That makes the assumption the frames are always sequentially laid out, which while a lot of the time that's true, is not guaranteed, particularly if animations are sharing frames.

amethyst_renderer/src/tex_animation.rs, line 100 at r2 (raw file):

Previously, torkleyy (Thomas Schaller) wrote…

Just a remainder, this needs an error type (I assume you're aware of this since this is a WIP).

I'll make zero sized type for this then

amethyst_renderer/src/tex_animation.rs, line 104 at r2 (raw file):

Previously, torkleyy (Thomas Schaller) wrote…

You can match against <above expression>.ok_or(())?

Ah yes, thank you!

Comments from Reviewable

Reviewed 5 of 6 files at r3.
Review status: all files reviewed at latest revision, 10 unresolved discussions.

Comments from Reviewable

Rhuagh · Jan 4, 2018

Reviewed 5 of 6 files at r3.
Review status: all files reviewed at latest revision, 10 unresolved discussions.

Comments from Reviewable

Bad robot

Xaeroxe · Jan 6, 2018

Bad robot

omni-viral · Jan 9, 2018

omni-viral requested changes Jan 9, 2018

View changes

Looks great!
And can be used with new renderer 😄

omni-viral · Jan 9, 2018

amethyst_core/src/timing.rs

+
+/// Multiplies a given Duration by the float provided.
+pub fn duration_f32_mul(duration: Duration, float: f32) -> Duration {
+    Duration::new((duration.as_secs() as f32 * float) as u64, (duration.subsec_nanos() as f32 * float) as u32)


That's why I don't like Duration.
Can we consider to replace it with our own type?

You can always use secs f64 if you don't want to use Duration.

I'm leaving this function since I've already written it, but I'm not using it in this PR anymore.

@jojolepro how many seconds I can store in f64 until error become more than epsilon?
I'm strongly against using floats where error may become observable.

Relax, an f32 has much higher precision than we need in this circumstance. You only really need an f64 when storing very large values, which we definitely are not. Most of these values are going to be below 1.0.

omni-viral · Jan 9, 2018

amethyst_renderer/src/tex_animation.rs

+    /// If the name provided can't be found in the `SpriteSheetData` for this component then this
+    /// will return `Err` otherwise it will return `Ok`.
+    pub fn set_animation(&mut self, sheet_storage: &AssetStorage<SpriteSheetData>, animation: &str) -> Result<(), ()> {
+        let animation = sheet_storage.get(&self.sprite_sheet_data).and_then(|data| data.animation_mapping.get(animation).map(|i| *i));


Returned error should be either "Invalid handle" and "Animation not found".

omni-viral · Jan 9, 2018

amethyst_renderer/src/tex_animation.rs

+
+    /// Advance the current animation by the time given.  Normally this will be called by the
+    /// engine by default using the regularly running game clock.  This function is made public so
+    /// that you can manually advance the animation if you require.


How user can disable automatic animation?
The system is added by bundle.
What if user wants only some entities to be manually animated?

Not built into the current implementation, but I'll give it some thought.

omni-viral · Jan 9, 2018

omni-viral reviewed Jan 9, 2018

View changes

omni-viral · Jan 9, 2018

amethyst_renderer/src/tex_animation.rs

+    /// that you can manually advance the animation if you require.
+    pub fn advance(&mut self, time: Duration, sheet_storage: &AssetStorage<SpriteSheetData>) -> Result<(), ()> {
+        use amethyst_core::timing::duration_f32_mul;
+        self.frame_timer += duration_f32_mul(time, self.playback_speed);


Shouldn't you wait with this before success one line below?

omni-viral · Jan 9, 2018

omni-viral reviewed Jan 9, 2018

View changes

omni-viral · Jan 9, 2018

amethyst_renderer/src/tex_animation.rs

+        let mut frame_time = self.current_frame(sheet_storage).ok_or(())?.duration;
+        while self.frame_timer >= frame_time {
+            self.current_frame += 1;
+            let max_frame = sheet_storage.get(&self.sprite_sheet_data).map(|data| data.animations[self.current_animation].len() - 1).ok_or(())?;


Don't see any chance for failure here. Should be unwrap.
Or get &SpriteSheetData at the first line and then reuse it.

omni-viral · Jan 9, 2018

omni-viral reviewed Jan 9, 2018

View changes

omni-viral · Jan 9, 2018

amethyst_renderer/src/tex_animation.rs

+            self.current_frame += 1;
+            let max_frame = sheet_storage.get(&self.sprite_sheet_data).map(|data| data.animations[self.current_animation].len() - 1).ok_or(())?;
+            if self.current_frame > max_frame {
+                self.current_frame = 0;


So currently animation looping is the only option.
Just writing it so we won't forget to add other options 😄

Objectives (quoting @Xaeroxe on gitter):

Create a data structure that can store animation data
Make passes display the appropriate section of the SpriteSheet based on this data structure
Create a system that can automatically advance through the frames once their stored duration has elapsed.
Add the system to the renderer bundle (trivial, one line)
Build passes that can render entities with Transform transparently. The UI pass can render transparently, and all of the other passes render entities with mesh and transform, so we mostly just need to combine these two concepts and make a few tweaks.
@torkleyy has also requested I see what I can do about reducing duplication of pass code. Not quite sure what I want to do about that yet, but since the PR is high enough priority I'm somewhat comfortable putting that off for now.
A data importer for aseprite JSON export was on my priority list. I'm OK omitting this for a first implementation.

mtsr · Jan 10, 2018

Objectives (quoting @Xaeroxe on gitter):

Create a data structure that can store animation data
Make passes display the appropriate section of the SpriteSheet based on this data structure
Create a system that can automatically advance through the frames once their stored duration has elapsed.
Add the system to the renderer bundle (trivial, one line)
Build passes that can render entities with Transform transparently. The UI pass can render transparently, and all of the other passes render entities with mesh and transform, so we mostly just need to combine these two concepts and make a few tweaks.
@torkleyy has also requested I see what I can do about reducing duplication of pass code. Not quite sure what I want to do about that yet, but since the PR is high enough priority I'm somewhat comfortable putting that off for now.
A data importer for aseprite JSON export was on my priority list. I'm OK omitting this for a first implementation.

torkleyy · Jan 12, 2018

torkleyy reviewed Jan 12, 2018

View changes

torkleyy · Jan 12, 2018

amethyst_renderer/src/tex_animation.rs

@@ -36,6 +37,43 @@ impl Into<Result<SpriteSheetData, ()>> for SpriteSheetData {
     }
 }
 
+/// Error returned by the `set_animation` function on the `SpriteSheetAnimation`
+#[derive(Copy, Clone, Debug)]
+pub enum SetAnimationError {


You can use error-chain for this, that would give us a back trace in case of an error.

torkleyy · Jan 12, 2018

amethyst_renderer/src/tex_animation.rs

+impl Display for SetAnimationError {
+    fn fmt(&self, f: &mut Formatter) -> Result<(), FmtError> {
+        f.write_str(
+            match self {


According to clippy, you should dereference self.

omni-viral · Jan 12, 2018

omni-viral requested changes Jan 12, 2018

View changes

It's good to see progress here!

omni-viral · Jan 12, 2018

amethyst_renderer/src/tex_animation.rs

+    type HandleStorage = VecStorage<Handle<Self>>;
+}
+
+impl Into<Result<SpriteSheetData, ()>> for SpriteSheetData {


When ! going to be stable?

And why do you need this. Can't it be Option? Into<Option> is already implemented.

So I realize now I did this wrong, but it was an attempt at making the type compatible with this

omni-viral · Jan 12, 2018

amethyst_renderer/src/tex_animation.rs

+        time: FloatDuration,
+        sheet_storage: &AssetStorage<SpriteSheetData>,
+    ) -> Result<(), ()> {
+        let mut frame_time = self.current_frame(sheet_storage).ok_or(())?.duration;


? works with Option you know.

Yeah it does, I just wanted to return a result here.

So, with the recent additions to the core animation system this should be quite easy to add now.
The needed parts goes as follows:

SpriteSheetData can be reduced to just a list of Frame, possible rename to SpriteSheet?
Frame will no longer need the duration, possibly rename to Sprite ?
MaterialAnimation can have Option<usize> for all attributes, and also needs a "rest pose index" per attribute (so the animation systems know what to use if there are no animations running), possibly rename to something like MaterialSpriteRef
Define MaterialChannel that contains all the attributes that should be possible to animate on MaterialAnimation
Implement AnimationSampling for MaterialAnimation, primitive will be usize
The vertex shader updates need to stay
The passes should just do sprite sheet data lookup using the indices from MaterialAnimation and hand over to the shaders

Rhuagh · Feb 14, 2018

So, with the recent additions to the core animation system this should be quite easy to add now.
The needed parts goes as follows:

SpriteSheetData can be reduced to just a list of Frame, possible rename to SpriteSheet?
Frame will no longer need the duration, possibly rename to Sprite ?
MaterialAnimation can have Option<usize> for all attributes, and also needs a "rest pose index" per attribute (so the animation systems know what to use if there are no animations running), possibly rename to something like MaterialSpriteRef
Define MaterialChannel that contains all the attributes that should be possible to animate on MaterialAnimation
Implement AnimationSampling for MaterialAnimation, primitive will be usize
The vertex shader updates need to stay
The passes should just do sprite sheet data lookup using the indices from MaterialAnimation and hand over to the shaders

I can provide the necessary parts to get this working with the core animation system, I just need some feedback on naming.

Rhuagh · Feb 14, 2018

I can provide the necessary parts to get this working with the core animation system, I just need some feedback on naming.

@Rhuagh Those names sound fine to me. I was planning on doing this myself if you have something else you'd rather be doing.

Xaeroxe · Feb 14, 2018

@Rhuagh Those names sound fine to me. I was planning on doing this myself if you have something else you'd rather be doing.

I've already started, I'll push the branch tonight and you can take it from there

Rhuagh · Feb 14, 2018

I've already started, I'll push the branch tonight and you can take it from there

Oh thanks!

Xaeroxe · Feb 14, 2018

Oh thanks!

This has been largely eclipsed by work from @Rhuagh so this PR is going to be closed as we work on bringing his work into the repo.

Xaeroxe · Feb 14, 2018

This has been largely eclipsed by work from @Rhuagh so this PR is going to be closed as we work on bringing his work into the repo.

Xaeroxe added project: animation project: rendering status: working type: feature labels Dec 7, 2017

bors bot closed this Jan 6, 2018

Xaeroxe reopened this Jan 6, 2018

mtsr added some commits Jan 8, 2018

Adds TexAnimationSystem to Renderer bundle and makes TexAnimation opt…
…ional (default on)

2f58228

Adds fn without_texture_animation

Loading status checks…

e41a8c4

Xaeroxe added some commits Jan 12, 2018

Add better error handling

Loading status checks…

de15267

Formatting pass for tex_animation.rs

Loading status checks…

cb24d45

Merge branch 'develop' into texture-animation

Verified

This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.

GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits

Loading status checks…

dde689e

Xaeroxe closed this Feb 14, 2018

Xaeroxe deleted the Xaeroxe:texture-animation branch Apr 23, 2018

amethyst/amethyst

Join GitHub today

[WIP] Texture animation #505

Conversation

Xaeroxe commented Dec 7, 2017 • edited Edited 2 times Xaeroxe edited Jan 6, 2018 (most recent) torkleyy edited Dec 7, 2017

Xaeroxe added project: animation project: rendering status: working type: feature labels Dec 7, 2017

This comment has been minimized.

omni-viral commented Dec 9, 2017

This comment has been minimized.

Xaeroxe commented Dec 9, 2017

This comment has been minimized.

omni-viral commented Dec 10, 2017

This comment has been minimized.

Xaeroxe commented Dec 17, 2017

This comment has been minimized.

Xaeroxe commented Dec 17, 2017

This comment has been minimized.

torkleyy commented Dec 22, 2017

This comment has been minimized.

Xaeroxe commented Dec 22, 2017

This comment has been minimized.

torkleyy commented Dec 25, 2017

This comment has been minimized.

Rhuagh commented Dec 28, 2017

This comment has been minimized.

Xaeroxe commented Jan 4, 2018

This comment has been minimized.

Rhuagh commented Jan 4, 2018

Xaeroxe added some commits Dec 5, 2017

bors bot closed this Jan 6, 2018

This comment has been minimized.

Xaeroxe commented Jan 6, 2018

Xaeroxe reopened this Jan 6, 2018

mtsr added some commits Jan 8, 2018

omni-viral requested changes Jan 9, 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.

omni-viral reviewed Jan 9, 2018 View changes

This comment has been minimized.

omni-viral reviewed Jan 9, 2018 View changes

This comment has been minimized.

omni-viral reviewed Jan 9, 2018 View changes

This comment has been minimized.

omni-viral Jan 9, 2018 • edited Edited 1 time omni-viral edited Jan 9, 2018 (most recent)

This comment has been minimized.

mtsr commented Jan 10, 2018

Xaeroxe added some commits Jan 12, 2018

torkleyy reviewed Jan 12, 2018 View changes

This comment has been minimized.

This comment has been minimized.

omni-viral requested changes Jan 12, 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.

Rhuagh commented Feb 14, 2018 • edited Edited 1 time Rhuagh edited Feb 14, 2018 (most recent)

This comment has been minimized.

Rhuagh commented Feb 14, 2018

This comment has been minimized.

Xaeroxe commented Feb 14, 2018

This comment has been minimized.

Rhuagh commented Feb 14, 2018

This comment has been minimized.

Xaeroxe commented Feb 14, 2018

This comment has been minimized.

Xaeroxe commented Feb 14, 2018

Xaeroxe closed this Feb 14, 2018

Xaeroxe deleted the Xaeroxe:texture-animation branch Apr 23, 2018

Xaeroxe commented Dec 7, 2017 •

edited

Edited 2 times

Xaeroxe edited Jan 6, 2018 (most recent)

torkleyy edited Dec 7, 2017

omni-viral requested changes Jan 9, 2018

View changes

omni-viral reviewed Jan 9, 2018

View changes

omni-viral reviewed Jan 9, 2018

View changes

omni-viral reviewed Jan 9, 2018

View changes

omni-viral Jan 9, 2018 •

edited

Edited 1 time

omni-viral edited Jan 9, 2018 (most recent)

torkleyy reviewed Jan 12, 2018

View changes

omni-viral requested changes Jan 12, 2018

View changes

Rhuagh commented Feb 14, 2018 •

edited

Edited 1 time

Rhuagh edited Feb 14, 2018 (most recent)