Documentation for new Built-in Events

[Ionshard]

Prelude: While attempting to learn Phaser 3, I came across the listen to game object event.js example, which involved listening to the 'gameobjectup' event from the InputPlugin. This was exactly what I needed but I couldn't find anything in the API Documentation regarding this Event or others like it.

With the introduction of EventEmitter3 a lot of Event Classes were removed and replaced with the new EventEmitter versions based on simple strings. They are obviously very versatile but the removal of the Event Classes also means that information on the replacement events themselves are not documented anywhere except seemingly in the code itself.

Ideally there should be somewhere in the documentation that covers these new events and lists all the currently built-in events used by the Phaser 3 internals (eg. gameobjectup, pointerup etc)

I would be happy to assist with this effort, but as I have only just arrived here, I feel I like the scope and understanding of how the best way to document these free-form events is beyond me. Thus this issue. If someone comes up with how to document the new EventEmitter3 events I would be happy to assist with the workload of actually documenting the events themselves.

--- Want to back this issue? **[Post a bounty on it!](https://www.bountysource.com/issues/59684620-documentation-for-new-built-in-events?utm_campaign=plugin&utm_content=tracker%2F283654&utm_medium=issues&utm_source=github)** We accept bounties via [Bountysource](https://www.bountysource.com/?utm_campaign=plugin&utm_content=tracker%2F283654&utm_medium=issues&utm_source=github).

[photonstorm]

Lots of them are documented already, for example: https://photonstorm.github.io/phaser3-docs/global.html#event:CameraFadeInStartEvent

In the jsdocs they look like this:

    /**
     * This event is fired when the fade in effect begins to run on a camera.
     *
     * @event CameraFadeInStartEvent
     * @param {Phaser.Cameras.Scene2D.Camera} camera - The camera that the effect began on.
     * @param {Phaser.Cameras.Scene2D.Effects.Fade} effect - A reference to the effect instance.
     * @param {integer} duration - The duration of the effect.
     * @param {integer} red - The red color channel value.
     * @param {integer} green - The green color channel value.
     * @param {integer} blue - The blue color channel value.
     */

and then in the corresponding method you add the @fires call:

    /**
     * Fades the Camera to or from the given color over the duration specified.
     *
     * @method Phaser.Cameras.Scene2D.Effects.Fade#start
     * @fires CameraFadeInStartEvent
     * @fires CameraFadeInCompleteEvent
     * @fires CameraFadeOutStartEvent
     * @fires CameraFadeOutCompleteEvent
     * @since 3.5.0

The only thing I've not decided upon is how they should be named (i.e. fully namespaced, or based on just the event name, so it appears in the globals list) - either way, it's an easy change once picked.

[Ionshard]

Ah awesome that is exactly what I was looking for.

Alright, though before I go thrashing around. For the given example in the Phaser.Input.InputPlugin#processUpEvents it is responsible for emitting a pointerup event on the GameObject as well as a gameobjectup event on itself.

Would I be correct in assuming that the gameobjectup event would be a Phaser.Input.InputPlugin#event:GameObjectUp event documented in the InputPlugin file while the pointerup event would be a Phaser.GameObjects.GameObject#event:PointerUp event documented in the GameObject file?

Finally, just out of curiosity, why the case mismatch? The example you linked is documented as CameraFadeInStartEvent but when actually emitted it's 'camerafadeinstart'

[photonstorm]

Yes, they would need splitting so that the right object has the @fires tag on it.

DOM events are always lower-case, so the event strings are lower-case too, to keep it consistent with how the browser works in general. That doesn't make for easy reading docs though, so it doesn't matter too much what the internal event name is (i.e. CameraFadeInStartEvent as long as the docs include an example of the correct version, which I guess they don't in this case.

Could do it like:

    /**
     * This event is fired when the fade in effect begins to run on a camera.
     *
     * @example
     * this.cameras.main.on('camerafadeinstart', callback, context)
     *
     * @event CameraFadeInStartEvent
     * @param {Phaser.Cameras.Scene2D.Camera} camera - The camera that the effect began on.
     * @param {Phaser.Cameras.Scene2D.Effects.Fade} effect - A reference to the effect instance.
     * @param {integer} duration - The duration of the effect.
     * @param {integer} red - The red color channel value.
     * @param {integer} green - The green color channel value.
     * @param {integer} blue - The blue color channel value.
     */

[Ionshard]

Sorry to bother you again, but after forking the repo and looking into it further I found 207 instances of the emit() function. That's a pretty small number. If it would be beneficial I could go through and standardize the documentation surrounding the events. If so, the following 4 questions should help determine the standard at play, (based on current conflicts in the doc)

1. Namespace or Global?

The namespace really assists knowing where you have to register your listener to receive the event, where as global seems to be less "cluttered"

2. camelCase or PascalCase?

PascalCase seems better (especially if paired with the Event suffix below) when used for multiple word events e.g. CameraFadeInStartEvent but camelCase allows single word events like boot or destroy

3. on Prefix?

Some current event's documentation include an 'on' prefix e.g. Phaser.GameObjects.Components.Animation#event:onCompleteEvent

4. Event Suffix?

Some documented events have an added Event suffix e.g. Phaser.Game#event:pauseEvent while others do not e.g. Phaser.Game#event:boot

Bonus Question: Should private functions ideally have @fires documentation as well?

EDIT: I guess I should specify that I am offering to only change the JSDocs ... I will obviously leave the code completely alone!

[photonstorm]

1. I think namespace makes most sense, otherwise we may end up with conflicts where two different systems emit the same event string (i.e. oncomplete) and it'd look wrong in the docs. I don't like having the namespace in there, because it looks really cluttered on the drop-down, but it does make more sense.

2. PascalCase because it's an extension of the namespace.

3. No, let's remove / ignore the 'on' part. It feels superflous, and everything would start with it on the list anyway, which feels pointless.

4. Event Suffix, yes.

5. It doesn't matter if the method is public or private, if it emits an event, it should be documented as doing so.

I'm planning on moving all event strings to consts in their own files soon, so the code will not have strings in it, but const references. This won't change anything re: docs though.

If you do help it would be extremely useful, thank you :)

[photonstorm]

Actually, thinking about this some more, I wonder if it would make sense to create an Events section within the namespace. I.e. Loader.Events and within that you have the consts (which would also be the event names) like Loader.Events.FileComplete, or maybe Loader.Events.FILE_COMPLETE - then we wouldn't need the Event suffix, because it'd be part of the name anyway.

[Ionshard]

I think this would also lend itself well to the issue I am experiencing where the Event's JSDocs block is inconsistently placed within the file itself. Sometimes it's placed near it's emit() while other times it appears at the top of the file.

If this is something you would like to go towards, if I am going to audit the entire events system this might be the time to attempt it.

Do you have an example of how you would like this Events section to look?

EDIT: Looking around I noticed the const.js files which looks like what you are referring to except instead of @name JSDocs they'd be @event.

However, from my outside perspective (which could be very wrong) the events are very dependent on their emitters and there could be multiple EventEmitters located in a single namespace. So if you store all the events in a new Events segment you will be co-locating events that actually don't have anything to do with each other. If you had a global event system having unique namespaces is all that matters but for the proposal above I contend that the namespace is useful not because it makes the event unique, but because it describes which emitter you have to listen to in order to receive the event.

Though of course, the code is so well organized you don't have any namespaces with more than one EventEmitter so it's not really a current problem.

[Ionshard]

Alright, I have completed some examples of both for you to choose.

Here is an example of all the events that flow through Phaser.Game gathered to the top of the Game.js file

and here is all the same events gathered into a new Events.js

This is an interesting case too, because technically this Events file is not part of the Phaser.Game namespace. Should it be considered Phaser.Events or overridden to Phaser.Game.Events?

Additionally I apologize if this is taking up too much of your time.

[photonstorm]

For me the 2nd one is what I meant in my comments above. I feel this will map better to TypeScript and will be cleaner in the docs. Also, it means I can use the CONSTs in the source code, instead of strings, which will avoid any potential typos and help with minification.

I think it makes sense to create an Events section in every namespace. Phaser.Game is a bit of an edge-case, because it's really part of the Phaser.Boot namespace, but for the constructors sake isn't included in there. For everything else, like Phaser.Animations.Events it's much cleaner.

[Ionshard]

Alright, this is coming along nicely, and I have been converting everything to the new Events namespace.

However, when I created Phaser.Input.Events I have come across the problem I feared. Under the Phaser.Input namespace there are actually 3 EventEmitters each managing their own set of events.

1. Phaser.Input.InputManager#events
2. Phaser.Input.InputPlugin
3. Phaser.Input.InputPlugin#pluginEvents

Should all the events across those 3 emitters appear in the same Phaser.Input.Events namespace?

An even stronger issue arises from the Phaser.Input.Gamepad namespace. Here you have multiple emitters as well.

1. Phaser.Input.Gamepad.GamepadPlugin
2. Phaser.Input.Gamepad.Gamepad

This is an issue because both of these event emitters have a 'down' event. They are both fired here https://github.com/photonstorm/phaser/blob/master/src/input/gamepad/Button.js#L109 and under the current configuration they would both be considered Phaser.Input.Gamepad.Events#DOWN events. But the DOWN event that fires on the Gamepad EventEmitter has different parameters than the DOWN event that fires on the GamepadPlugin, most notably the index vs pad parameter, which means I wouldn't be able to document them accurately.

[photonstorm]

A single file is fine, even when dealing with multiple emitters, as long as the consts declare the emitter in them. For example Phaser.Input.Gamepad.Events could contain GAMEPAD_DOWN and GAMEPAD_PLUGIN_DOWN which could both map to the string down and have their own unique jsdocs.

[Ionshard]

Hey, I haven't forgotten about this, things are still progressing nicely. It's just quite a bit of work now that there is a lot of reorganization happening as well. I have 114 instances of emit left to document. (Of course I will have to do some git magic to ensure I also update any new events that have arisen since I branched, but I'll deal with that when I finish what's here)

However, I have run into another situation that would require your input.

There are several instances of what I would call "dynamic" events, where the event is created programatically.

• KeyboardPlugin Key Code events
• File loader complete events
• DataManager data change events

Any thoughts on the preferred way to document these style of events?

For the Key code events, I can definitely write out all the events since it is driven from a predetermined list, so it would just be a bunch of copy and paste (most likely what I'll do)

However for things like the File loader which is driven by user defined data at runtime, there is nothing I can do. My instinct would be to provide some kind of function that creates the dynamic event name, so that it's parameters could be documented, though my background is functional programming so that could be a bias :P

For now, I will skip these events and continue with the static event documentation.

[photonstorm]

It's surely impossible to cover those in TypeScript at all. They'll have to just be mentioned in the JSDocs and dealt with from there I think.

[photonstorm]

I'm going to close off this issue as I've now completed all of this work myself. Every Event is properly namespaced, fully documented and marked against the methods that dispatch it. Will be part of the 3.16 release but is in master branch already.

[Ionshard]

Thanks, sorry about dropping the ball, I got about 50% of the way through all the events when I had some pressing changes at work and couldn't find the time to finish it! I did learn a lot about Phaser through what I did accomplish, and one thing I would like to say is that you write some quality Javascript! Man was it ever a dream to explore! Keep up the great work.

[photonstorm]

No worries, stuff happens. Was just leaving a comment here for anyone who may have been tracking it (and it's good to close another issue off!)