Use promises for getProperty

[notpushkin]

This would make it a bit easier to use:

let trackCount = await mpv.getProperty("playlist/count");
console.log(trackCount, "tracks in playlist!");

– or –

mpv.getProperty("playlist/count").then((trackCount) => {
  console.log(trackCount, "tracks in playlist!");
});

More info on Promises

[j-holub]

Hey that sounds really good. Actually what I was looking for when I created this API. I'm usually not a Node developer and this was my first contact with Node. I will look into it and implement it as soon as I have the time for it (within next week).

Thank you very much :)

[notpushkin]

Nice :) Here's a simple wrapper I'm currently using if it can be any help:

class MPV extends BaseMPV {
  constructor(options={}, args=[]) {
    super(options, args);
    this._getPropertyCallbacks = {};
    this.on("getrequest", (id, result) => {
      if (this._getPropertyCallbacks.hasOwnProperty(id)) {
        this._getPropertyCallbacks[id](result);
        delete this._getPropertyCallbacks[id];
      }
    })
  }

  getProperty(property) {
    let id = cuid();
    return new Promise((resolve, _) => {
      this._getPropertyCallbacks[id] = resolve;
      super.getProperty(property, id);
    });
  }
}

[j-holub]

So I've checked a little into the matter.

It seems, that Promises are only available in Node 7.0 upwards with a command line flag. Another options is using Babel and the like. This would cause more dependencies on the library for other users.

That's why I'm not sure if it is a good idea to include this as an exclusive option. Maybe something like a getPropertyPromise would be a good option, for users that want to use them.

Including a 'getPropertyCallback` might as well be a nice option. Again I was not very proficient with node when creating this API, so I wasn't too familiar with callbacks. I wrote this API for my MusicServer Meteor project.

What's your opinion on the matter?

[notpushkin]

There are promise polyfills for older versions of Node, and many projects already use them (and there are many libraries that use Promises exclusively, I believe); but I think a simple way to introduce Promises without breaking anything would be using Promises only getProperty() is called without request_id, like this:

getProperty: function(property, request_id) {
  if (!request_id) {
    var id = ...;
    return new Promise((resolve, _) => {
      this._getPropertyCallbacks[id] = resolve;
      this.getProperty(property, id);
    });
  }
  // ...send message to socket...
}

Also, you can look into any-promise which allows users to choose their preferred implementation (i. e. in case they need additional features).

[j-holub]

Finally implemented it! Sorry it took so long, but you know, University, Job, Final Fantasy XV ;) Took me a while to get my head around promises, especially in this tricky situation with messages sent over sockets and so on.

Thanks again for the great contribution to Node-MPV , you're detailed code helped me a lot, could've almost been a pull request. THANKS!

One note: I changed the behavior of mute() to set the player to mute. It doesn't toggle anymore. You can now use toggleMute() for that. I wanted to make it consistent with pause / unpause / togglePause.

Good Luck with your Songbee project. I'll check by from time to time :)

[notpushkin]

Thank you, awesome job there. And congrats on 1.0.0, by the way! :beers:

[j-holub]

Thank you very much :) I thought after half a year the thing is stable enough to go 1.0.0. . I was proven wrong by a Pull Request a few hours afterwards :D