collinhover / impactplusplus

Impact++ is a collection of additions to ImpactJS with full featured physics, dynamic lighting, UI, abilities, and more.
http://collinhover.github.com/impactplusplus
MIT License
276 stars 59 forks source link

No dynamic animations on entities that are using the shoot ability #145

Closed Pattentrick closed 10 years ago

Pattentrick commented 10 years ago

Hi @collinhover,

I have an entity with canFlipX: false and canFlipY: false. That entity uses the shoot ability ig.AbilityShoot for shooting projectiles. I noticed that the shootUp, shootDown and shootLeft animations won't show up. Only the shootRight animation gets triggered, no matter in which direction I move the entity.

If I add a check at this line:

https://github.com/collinhover/impactplusplus/blob/master/lib/plusplus/abilities/ability-shoot.js#L372

javascript if( this.entity.canFlipX && this.entity.canFlipX ){

this.entity.lookAt(projectile);

}



It fixes the issue and everything works as expected. Not sure though if that is the best solution. Maybe we should add a check based on the `faceTarget` property of the ability class as seen here:

https://github.com/collinhover/impactplusplus/blob/master/lib/plusplus/abilities/ability.js#L630

What do you think about this behavior?
collinhover commented 10 years ago

Sorry for the delay, been super busy this week. Can you explain why your fix double checks for canFlipX? I think the lookAt method already checks that... I'm not sure why it is not working. However, I think you are right that we maybe should check for the faceTarget property before calling the lookAt method. Is your setup using sidescrolling or top-down mode?

Pattentrick commented 10 years ago

The double check is of course nonsense! This was a typo, sorry!

The game is in top-down mode. I am not sure about the check of canFlipX at the lookAt method. There are only two parts that mention the canFlipX property at that method:

https://github.com/collinhover/impactplusplus/blob/master/lib/plusplus/core/entity.js#L3990 https://github.com/collinhover/impactplusplus/blob/master/lib/plusplus/core/entity.js#L4032

This will only change the flip of the entity, right? The entity class is quite big, so i am unsure what other consequences the change of the flip property has at that point.

Adding a check for the faceTarget property at the ability.js fixes the problem in my case. However, I have to mention that my game may be a special case. Because it's following shoot em up conventions, the player is always facing right and is always moving right (boss fights excluded).

I could also provide a demo if that helps. If you think a check for the faceTarget property is a good idea, I could send you a pull request. What is easier for you? Adding the check for yourself, or merging in a pull request?

Off topic: I am using particles a lot in my game right now for effects and explosions. The particle classes are awesome and highly useful. Thanks for that!

collinhover commented 10 years ago

Adding a check for faceTarget seems like a good idea. No need for a demo, but would you mind making a pull request?

On the original problem, it is strange and I will try to look into it. I'm glad the particle class is working for you :-)

Pattentrick commented 10 years ago

@collinhover I will close this issue since the fix was merged in. I can reopen this issue, if you need it as reminder for the original problem. But as far as I am concerned everything works fine right now.