Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

flipHorizontal should work for actors whose current drawing is an animation #1258

Closed
jweissman opened this issue Sep 18, 2019 · 1 comment · Fixed by #1264
Closed

flipHorizontal should work for actors whose current drawing is an animation #1258

jweissman opened this issue Sep 18, 2019 · 1 comment · Fixed by #1264
Labels
bug This issue describes undesirable, incorrect, or unexpected behavior

Comments

@jweissman
Copy link

Steps to Reproduce

flipHorizontal doesn't seem to correctly flip the underlying sprites flipHorizontal values.

  1. Extend a class from Actor. In my example it's a Player class.
  2. Setup some drawings with animations
  3. Set one of these animations to the current drawing
  4. Call .flipHorizontal = true on the currentDrawing

export class Player extends Actor {
    constructor(engine: Engine) {
        super(0, 0, 6, 18, Color.White);
      // ...
        let idle = SpriteSheets.Wandering.getSprite(0);
        let walk = SpriteSheets.Wandering.getAnimationBetween(engine,0,2,150);
        this.addDrawing('idle', idle);
        this.addDrawing('walk', walk);
    }

   // set currentDrawing somewhere; facing is the input vector

    onPreDraw() {
        let facingRight = this.facing.x > 0;
        // doesn't seem to change underlying sprite flipHorizontal attributes
            this.currentDrawing.flipHorizontal = facingRight;

       // but this hack works:
            //@ts-ignore
        if (this.currentDrawing.sprites) {
            //@ts-ignore
            this.currentDrawing.sprites.forEach(sprite => sprite.flipHorizontal = facingRight)
        } else {
            this.currentDrawing.flipHorizontal = facingRight;
        }
    }
}

Expected Result

When currentDrawing is an animation, I should be able to set flipHorizontal to true or false and have this value propagate to the underlying sprites in the animation.

So something like this.currentDrawing.flipHorizontal = true should flip an animation by flipping each of the sprites individually.

Actual Result

Just setting flipHorizontal on the animation doesn't (appear to) flip the underlying sprites' flipHorizontal values. The top-level animation drawable does have the correct value after being set, but it doesn't seem to propagate down. I'm likely "using it wrong" but maybe the docs could be added to hint at this scenario and provide some guidance on how to do this correctly?

Environment

  • browsers and versions:
    Chrome
  • operating system:
    Windows
  • Excalibur versions:
    23
  • (anything else that may be relevant)

Current Workaround

Something like this works, but is a hack around it.

//@ts-ignore
        if (this.currentDrawing.sprites) {
            //@ts-ignore
            this.currentDrawing.sprites.forEach(sprite => sprite.flipHorizontal = facingRight) 
        } else {
            this.currentDrawing.flipHorizontal = facingRight;
        }

The docs seem to suggest Animation should be doing something like this (setting flipHorizontal on underlying sprites) when this value is set. It would be really interesting for me to know the "correct way" to handle this case!

(Really love Excalibur by the way, thanks for all the amazing work!)

@eonarheim eonarheim added the bug This issue describes undesirable, incorrect, or unexpected behavior label Sep 18, 2019
@eonarheim eonarheim added this to the 0.24.0 Release milestone Sep 18, 2019
@eonarheim
Copy link
Member

@jweissman Thanks for finding this, we'll take a look as soon as we can!

@jedeen jedeen modified the milestones: 0.24.0 Release, 0.25.0 Nov 24, 2019
eonarheim added a commit that referenced this issue Dec 3, 2019
Closes #1258 

## Changes:

- Update sprites to conform to parent animation without mutating state
@jedeen jedeen modified the milestones: 0.25.0 Release, 0.24.0 Release Apr 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue describes undesirable, incorrect, or unexpected behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants