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

Adding resetGame functionality #1186

Merged
merged 6 commits into from
Feb 22, 2017
Merged

Adding resetGame functionality #1186

merged 6 commits into from
Feb 22, 2017

Conversation

JoschuaSchneider
Copy link
Contributor

setGame method would always result in an Object passed to setPresence.
Passing { game: null } (supported by discords WebSocket gateway to reset the current Game) to setPresence would still result in a Game Object sent to the endpoint.
Explicitly setting game to null should overwrite the game object provided by localPresence or client.presence.
This was neither supported by setGame or setPresence.

Discord Documentation: Gateway#gateway-status-update

`setGame` method would allways result in an Object passed to `setPresence`.
Passing { game: null } (supported by discords WebSocket gateway to reset the current Game) to `setPresence` would still result in a Game Object sent to the endpoint.
Explicitly setting `game` to null should overwrite the `game` object provided by `localPresence` or `client.presence`.
This was neither supported by `setGame` or `setPresence`.
@JoschuaSchneider JoschuaSchneider changed the title Adding resetGame functionallity Adding resetGame functionality Feb 14, 2017
@JoschuaSchneider
Copy link
Contributor Author

I decided not to extend the existing setGame method, because it accepts two parameters by default.
Setting the first to null would be an option, but I think it would not be as convenient as a resetGame method when debugging.

@devsnek
Copy link
Member

devsnek commented Feb 15, 2017

// ClientUser#setPresence 181:6
if (data.game === null) game = null;

// ClientUser#setGame
setGame(game, streamingURL) {
  if (game === null) return this.setPresence({ game });
  return this.setPresence({ game: {
    name: game,
    url: streamingURL,
  } });
}

@JoschuaSchneider
Copy link
Contributor Author

Was thinking about that, but for debug readons, if you set a variable myGame for example and gameUrl, if you intend to change the game but your myGame variable is null for some reason, the setGame(name, url) syntax could be misleading.

@devsnek
Copy link
Member

devsnek commented Feb 15, 2017

that would be your fault, not ours

@JoschuaSchneider
Copy link
Contributor Author

I dindn't say its anyones fault 🙂

I get your point, thought alot about the two ways.

Having a seperate method for resetting the game might be helpful in a few cases, but if documented not necessary. And its not quite as convenient when setting/resetting the game in a dynamic way.

Also the naming is a problem because it does not fit in the get/set pattern.

I'll push your changes, including the extended setGame(game, ?streamingUrl) and the simplification of the if statement.

Thanks for the feedback!

Minification of if statement in setPresence.
Removing resetGame method and adding a case for `game === null` to setGame method
@JoschuaSchneider
Copy link
Contributor Author

Just a quick question, I extended the comment above the setGame method like this:
* @param {string|null} game Game being played

Is that format correct?

@Gawdl3y
Copy link
Member

Gawdl3y commented Feb 18, 2017

No, you'd do @param {?string} game Game being played. The ? indicates nullability.

@amishshah amishshah merged commit 5c2086b into discordjs:master Feb 22, 2017
devsnek pushed a commit to devsnek/discord.js that referenced this pull request May 14, 2017
* Adding resetGame functionallity

`setGame` method would allways result in an Object passed to `setPresence`.
Passing { game: null } (supported by discords WebSocket gateway to reset the current Game) to `setPresence` would still result in a Game Object sent to the endpoint.
Explicitly setting `game` to null should overwrite the `game` object provided by `localPresence` or `client.presence`.
This was neither supported by `setGame` or `setPresence`.

* Missing semicolons to resetGame and setPresence

* Fixing trailing spaces, commas and semicolons

* Moving resetGame functionality into setGame method

Minification of if statement in setPresence.
Removing resetGame method and adding a case for `game === null` to setGame method

* Adding missing space in setGame method

* Fix docs
devsnek pushed a commit to devsnek/discord.js that referenced this pull request May 14, 2017
* Adding resetGame functionallity

`setGame` method would allways result in an Object passed to `setPresence`.
Passing { game: null } (supported by discords WebSocket gateway to reset the current Game) to `setPresence` would still result in a Game Object sent to the endpoint.
Explicitly setting `game` to null should overwrite the `game` object provided by `localPresence` or `client.presence`.
This was neither supported by `setGame` or `setPresence`.

* Missing semicolons to resetGame and setPresence

* Fixing trailing spaces, commas and semicolons

* Moving resetGame functionality into setGame method

Minification of if statement in setPresence.
Removing resetGame method and adding a case for `game === null` to setGame method

* Adding missing space in setGame method

* Fix docs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants