Update to node-steam 1.1.0

[Crazy-Duck]

Started from the yasp-dota branch which upgraded to protobufjs:
I updated index.js and all handlers to work with the latest version of node-steam. I've done some tests with creating/starting lobbies and chat and everything seems to work

[howardchung]

The commented-out handlers probably need to be enabled if you've tested that they work.

[Crazy-Duck]

I think those two are still broken, at least that's what i gather from
their contents. I haven't done any debugging on things that didn't work
before/were only partially implemented. As for the other handlers, I'm still
testing them as i go, for the moment I'm pretty sure lobbies and chat are
ok: I'm able to create a lobby, detect changes, start it, ... for chat i'm
able to retrieve the list of open channels. In the following days i'll
probably do some more test, but i wanted at least to already make the pull
request so we're not all doing the same work.

On 20 August 2015 at 10:53, Howard Chung notifications@github.com wrote:

In index.js
#90 (comment):

@@ -177,13 +206,13 @@ handlers[Dota2.EGCBaseClientMsg.k_EMsgGCClientConnectionStatus] = function gcCli

Dota2.Dota2Client = Dota2Client;

-require("./handlers/cache");
+//require("./handlers/cache");

The commented-out handlers probably need to be enabled if you've tested
that they work.

—
Reply to this email directly or view it on GitHub
https://github.com/RJacksonm1/node-dota2/pull/90/files#r37507774.

[Crazy-Duck]

Tested methods:

I will try and update this list as I go. Feel free to add in the comments then I'll update it here

match.js

• matchDetailsRequest
• matchmakingStatsRequest (response is received and handled, however I've no clue whether contents are correct, though they seem sensible)

lobbies.js

• createPracticeLobby
• balancedShuffleLobby
• leavePracticeLobby
• launchPracticeLobby

chat.js

• requestChatChannels
• sendMessage
• joinChat
• All events are working, chat channels are cached

community.js

• getPlayerMatchHistory
• profileRequest
• passportDataRequest
• hallOfFameRequest
  • NOK : k_EMsgGCHallOfFameRequest is sent to GC, however no response is received, whether a week was specified or not.

leagues.js

• leaguesInMonthRequest
  • Received leagues for given month:year and when omitted. Live league game events are received.

guild.js

• requestGuildData
• setGuildAccountRole
• handlers all working

[howardchung]

For best practices Travis CI should probably be set up with STEAM_USER, STEAM_PASS set up by the maintainer and added to Travis as secrets. Then we can write unit tests for each method and ensure they don't break with future updates.

I have no idea how the shift to the Reborn client will change things though, so it may be better to wait rather than do all the work now.

For YASP purposes only matchDetails and playerProfile are required, but all methods should probably be tested before merging into node-dota2.

[Crazy-Duck]

Some form of automated tests would indeed be nice, whatever the format. As for Reborn, I've seen SteamKit already has protobuf files for source 2, however I don't know how much difference there is in comparison with the existing source 1 files.

[howardchung]

I actually recommend https://github.com/SteamDatabase/GameTracking as a protobuf source, it appears to be updated automatically :)

[howardchung]

I recommend not changing the API and continuing to pass a steamClient here. You can construct a steamUser from a client.

var steamUser = new Steam.SteamUser(steamClient);

[Crazy-Duck]

Yesss, this part bugged me the most in the whole migration processus, didn't know you could create the user from the client. I've changed the method signature back to the original and committed the changes.

[Crazy-Duck]

Those protobufs are already those which are used in the library it seems :) The update seems to be done by means of a shell script -> update_proto.sh:

#!/bin/sh
pushd proto
rm *.proto
wget https://raw.githubusercontent.com/SteamDatabase/GameTracking/master/Protobufs/dota/base_gcmessages.proto
wget https://raw.githubusercontent.com/SteamDatabase/GameTracking/master/Protobufs/dota/gcsdk_gcmessages.proto
wget https://raw.githubusercontent.com/SteamDatabase/GameTracking/master/Protobufs/dota/dota_gcmessages_client.proto
wget https://raw.githubusercontent.com/SteamDatabase/GameTracking/master/Protobufs/dota/dota_gcmessages_client_fantasy.proto
wget https://raw.githubusercontent.com/SteamDatabase/GameTracking/master/Protobufs/dota/dota_gcmessages_common.proto
wget https://raw.githubusercontent.com/SteamDatabase/GameTracking/master/Protobufs/dota/steammessages.proto
popd

Before I made the initial pull request i used it to get the latest version of the files, as far as I see, there haven't been new updates to those we use in the last few days.

[howardchung]

Yeah, I wrote that script as part of my edits to node-dota2. The url will need to be modified for dota_s2 though.

[Crazy-Duck]

If anyone has any suggestions on how to make the equality in the filter work without resorting to these shenanigans, feel free to let me know because this bugs the crap out me

[jimmydorry]

Don't you need to explicitly return False if you don't want the filter function to keep the element you iterated over?

Also, if the types are not matched... why not just use the "==" operator instead of the "===" that tests for value and type?

[Crazy-Duck]

I don't know if it's actually absolutely necessary to explicitly return false in order to discard (doc isn't really clear on that), but I'll change it to just return the result of the equality.
As for the equality itself, I tried with both "==" and "===" and neither one detects it when the objects have the same value. I think the problem lies in the fact that the channelIds are compound objects rather than primitives, which explains why using their string representation gives the intended result.

[jimmydorry]

That is annoying.

I don't know of a better place to read about nodeJS functions. If you of a better one, let me know, otherwise I have been referring to below.

https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Array/filter

It says "constructs a new array of all the values for which callback returns a true value or a value that coerces to true".

It's not particularly the end of the world though, if they need to be implicitly cast as strings.

[rjackson]

If it works then it's fine. I think it would be a good idea to throw a comment in the code explaining why they are cast to strings though, for any future contributor's sanity.

[Crazy-Duck]

Alright, I changed the filters to directly return the result of the comparison, so there is always either true or false. Furthermore, future contributor's sanity is safe, comment has been added :)

[howardchung]

s2 is now the only client, s1 clienthellos appear to be rejected.

@Crazy-Duck you may want to pull https://github.com/yasp-dota/node-dota2/commits/steamv1

I changed update_proto.sh to use s2 protos (not sure if necessary), and manually set the engine in ClientHello to 1 (source 2).

[Crazy-Duck]

I made the pull and ran a first basic test: stuff seems to work (even guilds seem to still be working, even though current client has no such thing).

[jimmydorry]

Do you want to close this PR, and do a PR into a new branch in node-dota2 ? The PR is getting pretty big, and more people may see it if it's in the repo.

If you rebase, you could merge into RJacksonm1/node-dota2:dev

[Crazy-Duck]

Give me a day or so to test if everything is still working as it should (I've only been able to do a quick sanity test) and then I think we can close this PR :)

[jimmydorry]

Okey dokey. Great work so far!

[Crazy-Duck]

I went through about half the functions and their handlers, I haven't found any apparent problems yet. Tonight I'm gonna verify if chat and lobbies actually pop up inside the game client.

In the mean time, seeing as we're breaking shit anyways, I was wondering what you guys think of a very small refactoring before closing the pull. In short, I'd like to make the method names a little more uniform. For the moment we have a mix of nouns and actions for the method names, e.g. matchDetailsRequest and requestMatches. What I'd like to do is change all nouns to equivalent actions, so that the API becomes a little more intuitive; this would for example turn matchDetailsRequest into requestMatchDetails. This would however mean that the method names won't correspond with the underlying protomessage (which wasn't 100% the case anyway, see chat.js)
Thoughts?

[jimmydorry]

You can keep working on it. Just close this this pull and re-pull into the
dev branch or something.

Feel free to rename. I'll go over the readme to try and spot anything
orphaned after. Having things start with request etc. will help proper IDEs
with intellisense. It doesn't make much sense adhering to protobuff naming
when most users probably won't look at those.

On Thu, 10 Sep 2015 at 9:16 pm Crazy-Duck notifications@github.com wrote:

I went through about half the functions and their handlers, I haven't
found any apparent problems yet. Tonight I'm gonna verify if chat and
lobbies actually pop up inside the game client.

In the mean time, seeing as we're breaking shit anyways, I was wondering
what you guys think of a very small refactoring before closing the pull. In
short, I'd like to make the method names a little more uniform. For the
moment we have a mix of nouns and actions for the method names, e.g.
matchDetailsRequest and requestMatches. What I'd like to do is change all
nouns to equivalent actions, so that the API becomes a little more
intuitive; this would for example turn matchDetailsRequest into
requestMatchDetails. This would however mean that the method names won't
correspond with the underlying protomessage (which wasn't 100% the case
anyway, see chat.js)
Thoughts?

—
Reply to this email directly or view it on GitHub
#90 (comment).

[rjackson]

Yes, if we're breaking backwards compatibility with this update then please do take this opportunity to clean things up. We should minimise the amount of backward-incompatible updates we put out, so if we can get everything cleaned up now we absolutely should.

On that note, would anybody mind summarising the backward-incompatible changes? We should put together a quick upgrade guide to help any developers migrate their codebase from 0.7.1. I assume once this update is finished we will push it as version 1.0.0?

[howardchung]

Quick list that comes to mind of breaking changes:

• Whatever methods @Crazy-Duck decides to rename
• We no longer camelCase protobuf fields (parity with node-steam)
• Swap over to s2 protos (I think these are mostly the same as s1)

New functionality:

• New functions: leagueInfo, requestMatches, playerMatchHistory?

Optimizations/TODO:

• Write some code that will generate the messages.js file (probably easier to generate .json) https://github.com/yasp-dota/rapier/blob/master/generateTypes.js may be useful as a reference.

[rjackson]

So the only changes to the public API would be the renaming of methods, and the keys in the objects returned from the GC (snake_case instead of CamelCase)?

Everything else is either internal code (which we can change as much as we like) or new features, which don't need to mentioned in any sort of upgrade instructions?

[howardchung]

Yeah. The keys in the returned objects may differ due to the updated protobufs. That could be a breaking change depending on what the user was doing with that data.

[Crazy-Duck]

I've listed my proposition for name changes further on, but basically the gist of it is that all methods which aim to obtain a certain piece of information from the GC become "requestSomething" and all handler methods become "onSomethingResponse". The corresponding events which are thrown become "somethingData". A lot of the methods/handlers/events already followed this syntax, I just aimed to make this a uniform standard throughout the API. I've got the commit ready, if you guys agree, I'll add it to the PR.
Number of breaking changes: 8 events + 10 methods

chat.js

Old	New
onRequestChatChannelListResponse	onChatChannelsResponse
this.emit('chatChannelsReceived', channels)	this.emit('chatChannelData', channels)

community.js

Old	New
getPlayerMatchHistory	requestPlayerMatchHistory
profileRequest	requestProfile
passportDataRequest	requestPassportData
hallOfFameRequest	requestHallOfFame

guild.js

Old	New
onGuildData	onGuildDataResponse
this.emit("guildInvite", ...)	this.emit("guildInviteData", ...)

leagues.js

Old	New
leaguesInMonthRequest	requestLeaguesInMonth
onResponseLeagueInfo	onLeagueInfoResponse
this.emit("leaguesInMonthResponse", ...)	this.emit("leaguesInMonthData", ...)
this.emit("leagueInfo", ...)	this.emit("leagueInfoData", ..)

lobbies.js

Old	New
practiceLobbyListRequest	requestPracticeLobbyList
friendPracticeLobbyListRequest	requestFriendPracticeLobbyList
this.emit("practiceLobbyListResponse", ...)	this.emit("practiceLobbyListData", ...)
this.emit("friendPracticeLobbyListResponse", ...)	this.emit("friendPracticeLobbyListData", ...)

match.js

Old	New
matchDetailsRequest	requestMatchDetails
matchmakingStatsRequest	requestMatchmakingStats
onRequestMatchesResponse	onMatchesResponse
this.emit("matches", ...)	this.emit("matchesData", ...)
this.emit("matchData", ...)	this.emit("matchDetailsData", ...)

sourceTV.js

Old	New
findSourceTVGames	requestSourceTVGames
nothing	this.emit("sourceTVGamesData", ...

[jimmydorry]

Alright, there is a branch for this code in the repo now. All of the commits can be merged as we go into that one... but we have somewhere we can point people that want to contribute now at.