Phaser.Weapon's fire method not returning anything

[JarLowrey]

Currently, Phaser.Weapon's fire method does not return any value (look for Phaser.Weapon.prototype.fire in the weapons file). The documentation says it should return false if a bullet was not found and the pool cannot be extended. It does not say what is returned if the bullet is found, presumably it is true?

I propose that instead of returning true/false, the method returns the fired bullet or null. This change will not break current code in the format if(bullet)... However, it will provide the new functionality of potentially manipulating the bullet after it has been fired (like setting tint). Currently, if you want to do this you would have to set all the bullets to the same tint, have texture for each tint, or try to dive into the guts of the engine and manually manipulate variables.

[JarLowrey]

This should be a simple fix, I'm working on the contribution right now. Will submit a merge request if ya'll think it's a good idea.

https://github.com/JTronLabs/phaser/commit/018b4b097ecd286cade9bf374a905d626f08b31b

[JarLowrey]

After looking at the code, making this change will also affect fireAtPointer, fireAtSprite, and fireAtXY, which is a good thing and makes for consistent behavior.

[JarLowrey]

I made the monumental return bullet; addition in the fire method. I would like to update the docs to reflect this change, but I'm not sure where to get started on the documentation. Is there any sort of compilation to the HTML pages, or are they all just static pages?

[photonstorm]

You just edit the jsdoc block in the source files. All the html etc is
generated automatically on build. Don't forget the TypeScript defs file too
(phaser.d.ts in this case, not the comments one)

On Wednesday, 17 August 2016, James [email protected] wrote:

I made the monumental return bullet; addition in the fire method. I would
like to update the docs to reflect this change, but I'm not sure where to
get started on the documentation. Is there any sort of compilation to the
HTML pages, or are they all just static pages?

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#2693 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAKCfOuyFjK1QczhCT1KddFiPnwjOsPlks5qgmUkgaJpZM4Jl_r_
.

Photon Storm Ltd.
http://www.photonstorm.com

Skype: richard.davey
Twitter: @photonstorm

Registered in England and Wales.
Company ID: 8036404
VAT Number: 136 4333 27

[JarLowrey]

Thanks Rich! I'll update the TypeScript defs in ~10 hours. Just to clarify, the changes I've made in this commit https://github.com/JTronLabs/phaser/commit/018b4b097ecd286cade9bf374a905d626f08b31b are enough to update the docs? And you agree that overall, would be a good change, correct?

[photonstorm]

Yes I'm happy that fire should return the Bullet, that's fine.

Your jsdoc updates are enough, yes.

[JarLowrey]

Awesome, thanks. What npm command do I need to run for the new source to be reflected in the docs - maybe npm build?

[photonstorm]

No, please don't. I can't merge PRs that contain any build files or docs
in. Source only. If you want to build locally for your own testing then go
for it, but don't include it with the pull request please.

On Wednesday, 17 August 2016, James [email protected] wrote:

Awesome, thanks. What npm command do I need to run for the new source to
be reflected in the docs - maybe npm build?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#2693 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAKCfIv0yJPCsmjXONhzB8csmOJDkhdRks5qg1LngaJpZM4Jl_r_
.

Photon Storm Ltd.
http://www.photonstorm.com

Skype: richard.davey
Twitter: @photonstorm

Registered in England and Wales.
Company ID: 8036404
VAT Number: 136 4333 27

[JarLowrey]

submitted PR

#2696

[JarLowrey]

Do I need to do anything to help fix this?

[photonstorm]

No, the CI fail is unrelated to your source.

[JarLowrey]

Thanks rich. The pr should be good to go then. Just FYI, i updated the typescript defs for 2.7.0 but not 2.6.1. I assumed since its already released we wouldnt want to change it. Is that right? If not i can update the PR. I appreciate you letting me make the changes so i can learn how to do it!

[photonstorm]

You're right - we only ever update the dev branch defs, never master.