Johni0702 / mumble-client

Modular Node.js Mumble protocol client library
27 stars 24 forks source link

add channel on update return value #2

Closed minghan9456 closed 4 years ago

minghan9456 commented 4 years ago

add channel on update return value return value should include self object (Channel).

Listener can't get channel id when channel on update. cause changes only have channel name without channel id

Johni0702 commented 4 years ago

I don't quite get the purpose of this change. You already must have a reference to the channel when you registered a listener for it:

const channel = …
channel.on('update', (changes) => {
    console.log(channel.id)
})

and additionally via this if you're not using an arrow function (ref):

const channel = …
channel.on('update', function (changes) {
    console.log(this.id)
})
minghan9456 commented 4 years ago
worker.js
function setupChannel (id, channel) {
    id = Object.assign({}, id, { channel: channel.id })

    registerEventProxy(id, channel, 'update', (actor, props) => {
      if (actor) {
        actor = actor.id
      }
      if (props.parent) {
        props.parent = props.parent.id
      }
      if (props.links) {
        props.links = props.links.map((it) => it.id)
      }
      return [actor, props]
    })
  ......

When listen add channel, this part will have error. cause on update only return 1 value(change). props will be undefined. and then at worker-client.js WorkerBasedMumbleChannel._dispatchEvent can not get correct args, so can not add new channel.

 _dispatchEvent (name, args) {
    if (name === 'update') {
      let [actor, props] = args
      Object.entries(props).forEach((entry) => {
        this._setProp(entry[0], entry[1])
      })
      if (props.parent != null) {
        props.parent = this.parent
      }
      if (props.links != null) {
        props.links = this.links
      }
      args = [
        this._client._user(actor), // another problem : this should use this._client._channel. it is correct?
        props
      ]
    } else if (name === 'remove') {
      delete this._client._channels[this._id]
    }
    args.unshift(name)
    this.emit.apply(this, args)
  }
Johni0702 commented 4 years ago

The "actor" is the one who is acting on something, e.g. the one who's moving something around or renaming something. It is not necessarily the object itself, though in the case of a user, it can be (it cannot be a channel because channels do not have agency).

Mumble does not transmit the actor for changes to a channel (not sure why that's the case but it just doesn't), it only does so for changes to a user.

Therefore this looks like an issue with the code you posted (probably sloppy copy/paste from the code for user event handling).