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

feat: Animation improvements fromSpriteSheetCoordinates + EventEmitter #2666

Merged
merged 3 commits into from
Jun 25, 2023

Conversation

eonarheim
Copy link
Member

===:clipboard: PR Checklist :clipboard:===

  • 📌 issue exists in github for these changes
  • 🔬 existing tests still pass
  • 🙈 code conforms to the style guide
  • 📐 new tests written and passing / old tests updated with new scenario(s)
  • 📄 changelog entry added (or not needed)

==================

This PR adds a few improvements to animations that have been discussed offline

  • Adds new EventEmitter type which is a better typed event implementation
  • Updates the .events type to be the new EventEmitter
  • Adds the frameIndex of the current frame to the animation event
  • Adds a new static builder for constructing animations from SpriteSheets
     const spriteSheet = SpriteSheet.fromImageSource({...});
     const anim = Animation.fromSpriteSheetCoordinates({
      spriteSheet,
      frameCoordinates: [
        {x: 0, y: 5, duration: 100},
        {x: 1, y: 5, duration: 200},
        {x: 2, y: 5, duration: 100},
        {x: 3, y: 5, duration: 500}
      ],
      strategy: AnimationStrategy.PingPong
     });

@github-actions github-actions bot added the enhancement Label applied to enhancements or improvements to existing features label Jun 25, 2023
@eonarheim eonarheim merged commit 4b72691 into main Jun 25, 2023
@eonarheim eonarheim deleted the feature/animation-improvements branch June 25, 2023 15:46
Comment on lines +83 to +86
let i = -1;
if ((i = this._pipes.indexOf(emitter)) > -1) {
this._pipes.splice(i, 1);
}
Copy link
Contributor

@tonivj5 tonivj5 Jul 1, 2023

Choose a reason for hiding this comment

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

Hey @eonarheim! I saw this code some days ago... And I was a bit curious of why this way 😅

could it be rewritten in this way or there's something I don't see?

const i = this._pipes.indexOf(emitter);
if (i > -1) {
   this._pipes.splice(i, 1);
}

Copy link
Member Author

Choose a reason for hiding this comment

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

@tonivj5 good catch, I like yours better! (I'd support a PR 😄)

I'm not sure why I did that, normally I'd do what you suggested (I originally wrote this code a while ago and was holding on to the snippet)

Copy link
Contributor

Choose a reason for hiding this comment

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

here we go #2707 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Label applied to enhancements or improvements to existing features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants