Grunt and Lint

[jnovack]

Added Gruntfile.js for automatic development.
Linted a few files that showed up as violations.

(If you are not familiar with Grunt, it is a development tool to constantly lint/build/run your application while you are editing code. Currently, the default task does nothing, however grunt lint will continuously lint your files while editing them to make sure you did not miss a comma or other silliness. npm install to install all devDependencies).

[jnovack]

There are still a number of linting violations, however, I was unsure how you wanted to handle them (or how the automated build should handle them ;))

   heroprotocol.js
     69 |  Object.keys(this._eventStats).sort().forEach(key => {
           ^ Unreachable 'Object' after 'throw'.
    115 |    return this.details = prepare(this.protocol.decodeReplayDetails(this.archive.readFile('replay.details')));
                                  ^ Did you mean to return a conditional instead of an assignment?
    117 |    return this.initdata = prepare(this.protocol.decodeReplayInitdata(this.archive.readFile('replay.initData')));
                                   ^ Did you mean to return a conditional instead of an assignment?
    131 |      for (var event of this.protocol.decodeReplayMessageEvents(this.archive.readFile('replay.message.events'))) {
                        ^ 'event' is already defined.
    141 |      for (var event of this.protocol.decodeReplayTrackerEvents(this.archive.readFile('replay.tracker.events'))) {
                        ^ 'event' is already defined.
    149 |    return this.attributesevents = prepare(this.protocol.decodeReplayAttributesEvents(this.archive.readFile('replay.attributes.events')));
                                           ^ Did you mean to return a conditional instead of an assignment?
   lib/decoders.js
    356 |    var field = fields.find(field => field[2] === tag);
                                           ^ Don't make functions within a loop.
   lib/protocol29406.js
    498 |      value.value = buffer.readAlignedBits(4).reverse();
               ^ Unreachable 'value' after 'throw'.
   lib/protocol39951.js
    423 |    if (decodeUserId) event._userid = userid;
                                               ^ 'userid' used out of scope.
   lib/protocol40087.js
    423 |    if (decodeUserId) event._userid = userid;
                                               ^ 'userid' used out of scope.
   lib/protocol40336.js
    441 |    if (decodeUserId) event._userid = userid;
                                               ^ 'userid' used out of scope.
   replay.js
     65 |const Map = class {
               ^ Redefinition of 'Map'.

[Farof]

Looks great!

I had to install grunt-cli globally to use it, wouldn't it make more sense to add it as a dev dependency?

Running grunt lint and modifying a file got me this error:

$ grunt lint
Running "concurrent:lint" (concurrent) task
Running "jshint:all" (jshint) task
Running "watch" task
Waiting...
>> File "heroprotocol.js" changed.
Running "jshint:all" (jshint) task
FATAL ERROR: CALL_AND_RETRY_LAST Allocation failed - process out of memory
Warning: FATAL ERROR: CALL_AND_RETRY_LAST Allocation failed - process out of memory Use --force to continue.

Aborted due to warnings.

Am I missing something? That's on Windows, will test on OS X later.

That's the first time I use pull requests, is it possible to separate the grunt addition and lint corrections into two separate commits?

[jnovack]

Sorry, sometimes I have remind myself I'm talking outside my dev department. Forgive me.

grunt-cli is supposed to be a machine-wide install (npm install -g grunt-cli), the devDependencies are specifically all that's needed for this project. You normally wouldn't want to package grunt-cli with the package.

I'm exclusively on OSX and Linux.

[jnovack]

@Farof I can finish the rest of the protocols today. Can you verify a few findings (I believe the following are true, just need a second set of eyes).

EVERY protocol from (and including) "tracker_eventid_typeid" and below is EXACTLY the same in the .py repository for EACH PROTOCOL.

The only TRUE differences between each file is the enum arrays have different structures (and we want to export a version number).

[Farof]

I've been reworking heroprotocol.js and scripts that use it. I'll look at everything after that push, thanks.

[jnovack]

I think this is all I want to do with this commit.

All protocols with the exception of protocol29406 are complete and linted, 1 errors.

The only exception is:

   lib/protocol29406.js
    498 |      value.value = buffer.readAlignedBits(4).reverse();
               ^ Unreachable 'value' after 'throw'.

Not sure how you wanted to handle that, we can merge it, then fix it.

[Farof]

Merging the doc and then looking at this.

[Farof]

I'm looking at grunt, if you know how to remove the protocols addition from this pull request and put them in another dedicated one I would appreciate it and would love to have a quick rundown on how to do that.

[jnovack]

You would like two PRs? 1 for Grunt/Lint, 1 for Protocols?

[Farof]

Yes, I could merge the protocols right away and get grunt working separately. Thank you.

[jnovack]

Grunt is just a devDependency. That's all I added there. It's just a tool to help develop, never gets to the NPM package.

[jnovack]

Honestly, it's a pain in the split at this point because there have been so many merges in from master.

grunt is 1 file (Gruntfile.js) and 6 lines in package.json (devDependencies ONLY) which defines some development helpers (grunt lint being one of them).

[Farof]

Merged the protocols separately. Grunt gives me an "out of memory" error on OS X as well. See #16.