Removing addRandom and arrays from the multiplayer code

JavaScript
1

I started off with this code:
https://jsfiddle.net/toyk5d7p/

Removing addRandom and arrays I am left with this:

I know that everything works in the code.
It looks right to me.
https://jsfiddle.net/orydf57j/

Did I do a good job?

Is there anything that needs to be changed?

const videoPlayer = (function makeVideoPlayer() {
    const players = [];

    const tag = document.createElement("script");
    tag.src = "https://www.youtube.com/player_api";
    const firstScriptTag = document.getElementsByTagName("script")[0];
    firstScriptTag.parentNode.insertBefore(tag, firstScriptTag);

    function onPlayerReady(event) {
        const player = event.target;
        player.setVolume(100);
    }

    function addPlayer(video, settings) {
        const defaults = {
            height: 360,
            host: "https://www.youtube-nocookie.com",
            videoId: video.dataset.id,
            width: 640
        };
        defaults.events = {
            "onReady": onPlayerReady
        };

        const playerOptions = combinePlayerOptions(defaults, settings);
        const player = new YT.Player(video, playerOptions);
        players.push(player);
        return player;
    }

    return {
        addPlayer
    };
}());

const managePlayer = (function makeManagePlayer() {
    const defaults = {};
    defaults.playerVars = {
        autoplay: 0,
        controls: 0,
        disablekb: 1,
        enablejsapi: 1,
        fs: 0,
        iv_load_policy: 3
    };

    function show(el) {
        el.classList.remove("hide");
    }

    function createPlayer(videoWrapper, settings = {}) {
        const video = videoWrapper.querySelector(".video");
        const playerOptions = combinePlayerOptions(defaults, settings);
        return videoPlayer.addPlayer(video, playerOptions);
    }

    function createCoverClickHandler(playerSettings) {
        return function coverClickHandler(evt) {
            const cover = evt.currentTarget;
            const wrapper = cover.nextElementSibling;
            show(wrapper);
            const player = createPlayer(wrapper, playerSettings);
            wrapper.player = player;
        };
    }

    function addPlayer(coverSelector, playerSettings) {
        const clickHandler = createCoverClickHandler(playerSettings);
        manageCover.addCoverHandler(coverSelector, clickHandler);
    }

    function init(playerOptions) {
        Object.assign(defaults, playerOptions);
    }
    return {
        add: addPlayer,
        init
    };
}());

function onYouTubeIframeAPIReady() {
    managePlayer.init({
        playerVars: {
            controls: 1
        }
    });
    managePlayer.add(".playa", {
        playerVars: {
            start: 45
        }
    });

    managePlayer.add(".playb", {
        playerVars: {
            playlist: "0dgNc5S8cLI,mnfmQe8Mv1g,-Xgi_way56U,CHahce95B1g"
        }
    });
    managePlayer.add(".playc", {
        playerVars: {
            start: 45
        }
    });
    manageCover.init({
        container: ".container",
        playButton: ".thePlay"
    });
}
2

The init function cannot use object.assign, as that destroys previous default options. Not only is combinePlayerOptions needed, but over writing defaults is a bad thing. Better, is to have an options variable, inside of which is both the default options and the preferred options instead.

3

Doing it this way am I still overwriting defaults?

What if I do this?

Is that better?

Only having playerVars in one spot.
https://jsfiddle.net/ozg8ky2m/

This at the top:

const managePlayer = (function makeManagePlayer() {
  const defaults = {};

This at the bottom.

function onYouTubeIframeAPIReady() {
  managePlayer.init({
    playerVars: {
      autoplay: 0,
      controls: 0,
      disablekb: 1,
      enablejsapi: 1,
      fs: 0,
      iv_load_policy: 3
    }
  });

Full Code:

const managePlayer = (function makeManagePlayer() {
  const defaults = {};

  function show(el) {
    el.classList.remove("hide");
  }

  function createPlayer(videoWrapper, settings = {}) {
    const video = videoWrapper.querySelector(".video");
    const playerOptions = combinePlayerOptions(defaults, settings);
    return videoPlayer.addPlayer(video, playerOptions);
  }

  function createCoverClickHandler(playerSettings) {
    return function coverClickHandler(evt) {
      const cover = evt.currentTarget;
      const wrapper = cover.nextElementSibling;
      show(wrapper);
      const player = createPlayer(wrapper, playerSettings);
      wrapper.player = player;
    };
  }

  function addPlayer(coverSelector, playerSettings) {
    const clickHandler = createCoverClickHandler(playerSettings);
    manageCover.addCoverHandler(coverSelector, clickHandler);
  }

  function init(playerOptions) {
    Object.assign(defaults, playerOptions);
  }
  return {
    add: addPlayer,
    init
  };
}());

function onYouTubeIframeAPIReady() {
  managePlayer.init({
    playerVars: {
      autoplay: 0,
      controls: 0,
      disablekb: 1,
      enablejsapi: 1,
      fs: 0,
      iv_load_policy: 3
    }
  });
4

A good way to figure out if the defaults are being overwritten, is with 3 or more players.

That way you can use managePlayer.init before adding two of the players, then use managePlayer.init again to set a different type of setting that should be used by the third player.

The expected behaviour is that the settings from the first init should not appear after doing the second init.

Any variation from that expected behaviour (such as defaults getting destroyed, or previous init options living on through different init commands), is a problem.

How that is fixed is by not changing the defaults at all. Instead, you have init create a preferred set of options that combines the default options and the init options, and the rest of the code in there doesn’t use defaults at all, but uses that preferred combination instead.

1 Like
5

Yes, I want to implement that in the code.

How would I do that, as what you suggested I do?

you have init create a preferred set of options that combines the default options and the init options, and the rest of the code in there doesn’t use defaults at all, but uses that preferred combination instead.

In steps, how do I set the code up to do that?

7

Well right now you have manage.init followed by three managePlayer.add statements, and a final manage.init.

I recommend that you move that final manage.init up so that it’s somewhere inbetween the manage.add statements, and update the init statement so that it causes a default setting to occur that’s different from controls.

1 Like
8

Like this? Did I do that right?
https://jsfiddle.net/spc9wz0u/

If that is good, what do I do next?

function onYouTubeIframeAPIReady() {
  managePlayer.init({
    playerVars: {
      autoplay: 0,
      controls: 1,
      disablekb: 1,
      enablejsapi: 1,
      fs: 0,
      iv_load_policy: 3
    }
  });

  manageCover.init({
    container: ".container",
    playButton: ".thePlay"
  });
}

managePlayer.add(".playa", {
  playerVars: {
    start: 45
  }
});

managePlayer.add(".playb", {
  playerVars: {
    playlist: "0dgNc5S8cLI,mnfmQe8Mv1g,-Xgi_way56U,CHahce95B1g"
  }
});
managePlayer.add(".playc", {
  playerVars: {
    start: 45
  }
});
9

No, you didn’t do it right at all.

10

Step 1

move that final manage.init up so that it’s somewhere inbetween the manage.add statements

If this way is wrong:

 managePlayer.init({
    playerVars: {
      autoplay: 0,
      controls: 1,
      disablekb: 1,
      enablejsapi: 1,
      fs: 0,
      iv_load_policy: 3
    }
  });

  manageCover.init({
    container: ".container",
    playButton: ".thePlay"
  });
}

managePlayer.add(".playa", {
  playerVars: {
    start: 45
  }
});

I can try this again.

Doing this would place: manageCover.init({ in-between managePlayer.add(".playa", and managePlayer.add(".playb",

Like this?

Is that right?
https://jsfiddle.net/ho1t2sgb/

  managePlayer.init({
    playerVars: {
      autoplay: 0,
      controls: 1,
      disablekb: 1,
      enablejsapi: 1,
      fs: 0,
      iv_load_policy: 3
    }
  });

  managePlayer.add(".playa", {
    playerVars: {
      start: 45
    }
  });

  manageCover.init({
    container: ".container",
    playButton: ".thePlay"
  });

  managePlayer.add(".playb", {
    playerVars: {
      playlist: "0dgNc5S8cLI,mnfmQe8Mv1g,-Xgi_way56U,CHahce95B1g"
    }
  });

  managePlayer.add(".playc", {
    playerVars: {
      start: 45
    }
  });
}
11

If that is right, how I fixed it.
https://jsfiddle.net/ho1t2sgb/

Step 2

update the init statement so that it causes a default setting to occur that’s different from controls.

How do I do that?

What do I add, change/remove in the code to reflect that?

const managePlayer = (function makeManagePlayer() {
  const defaults = {};

  function show(el) {
    el.classList.remove("hide");
  }

  function createPlayer(videoWrapper, settings = {}) {
    const video = videoWrapper.querySelector(".video");
    const playerOptions = combinePlayerOptions(defaults, settings);
    return videoPlayer.addPlayer(video, playerOptions);
  }

  function createCoverClickHandler(playerSettings) {
    return function coverClickHandler(evt) {
      const cover = evt.currentTarget;
      const wrapper = cover.nextElementSibling;
      show(wrapper);
      const player = createPlayer(wrapper, playerSettings);
      wrapper.player = player;
    };
  }

  function addPlayer(coverSelector, playerSettings) {
    const clickHandler = createCoverClickHandler(playerSettings);
    manageCover.addCoverHandler(coverSelector, clickHandler);
  }

  function init(playerOptions) {
    Object.assign(defaults, playerOptions);
  }
  return {
    add: addPlayer,
    init
  };
}());

function onYouTubeIframeAPIReady() {
  managePlayer.init({
    playerVars: {
      autoplay: 0,
      controls: 1,
      disablekb: 1,
      enablejsapi: 1,
      fs: 0,
      iv_load_policy: 3
    }
  });

  managePlayer.add(".playa", {
    playerVars: {
      start: 45
    }
  });

  manageCover.init({
    container: ".container",
    playButton: ".thePlay"
  });

  managePlayer.add(".playb", {
    playerVars: {
      playlist: "0dgNc5S8cLI,mnfmQe8Mv1g,-Xgi_way56U,CHahce95B1g"
    }
  });

  managePlayer.add(".playc", {
    playerVars: {
      start: 45
    }
  });
}
12

I was recommending that you have an init, add, add, init, and add, in that order, with the inits being for different types of player options.

13

You never said explicitly what each of the init’s you were referring to.

I was confused because of that.

Like this? 1st
https://jsfiddle.net/rw7ukgaf/

  managePlayer.init({
    playerVars: {
    }
  });
managePlayer.add
managePlayer.add
  manageCover.init({
    container: ".container",
    playButton: ".thePlay"
  });
managePlayer.add

or this way? 2nd
https://jsfiddle.net/qb8pydrn/

You would be referring to this way then, right?

with the inits being for different types of player options.

This one has player options between the add’s

  manageCover.init({
    container: ".container",
    playButton: ".thePlay"
  });
managePlayer.add
managePlayer.add
  managePlayer.init({
    playerVars: {
    }
  });
managePlayer.add

After that how do I do this?

update the init statement so that it causes a default setting to occur that’s different from controls.

Which is what I would be doing, next, right?

I don’t know how that would be done.

14

Are either one of those correct, or am I still misunderstanding what you want me to do?

1st Way:
https://jsfiddle.net/rw7ukgaf/

managePlayer.init({
managePlayer.add
managePlayer.add
manageCover.init({
managePlayer.add

2nd Way:
https://jsfiddle.net/qb8pydrn/

manageCover.init({
managePlayer.add
managePlayer.add
managePlayer.init({
managePlayer.add

If one of those is what you were recommending, after that, I am supposed to do this which I don’t know how to do.

update the init statement so that it causes a default setting to occur that’s different from controls.

I don’t understand how to write that into the code.

15

It should all be managePlayer.

That way with the first init you can set start to be 45, and with the second init you can set a different option such as playlist to be some videos.

That way you can easily examine the resulting behaviour of the third video that loads after the last init.
If that last video still starts at 45 seconds, that is confirmation that earlier init settings are inappropriately being maintained.

The expected behaviour is that the managePlayer defaults are being added only to the most recent init. Anything else such as what I’ve just explained, is a problem that needs to be fixed. It’s an easy fix too, but in my experience you need very easy ways to observe and experience the problem, which what I’m directing you to achieve.

1 Like
17

Can I do this, is this better?

Is there anything wrong here?
https://jsfiddle.net/fdujqvo6/

You keep telling me defaults are being destroyed/overwritten, but if I can’t see that without using console.log(), I am going to keep making the same mistakes, over and over again.

I don’t know how to use console.log() to be able to know if defaults are or are not being destroyed here.

Can you show me how to do that?

How many console logs am I adding?

Like this?
https://jsfiddle.net/xohctk17/1/

  defaults.playerVars = {
    autoplay: 0,
    controls: 0,
    disablekb: 1,
    enablejsapi: 1,
    fs: 0,
    iv_load_policy: 3
  };console.log(defaults);

Placing a console.log() in here like this?

  managePlayer.add(".playa", {
    playerVars: {
      start: 45
    }
  });console.log(managePlayer);

This comes up in console.

Also, inside the console I don’t know what behavior I am supposed to be looking for in terms of what behavior I want to avoid, and the behavior I am looking to achieve.

Inside console, can you show me the behavior I want to avoid, and the behavior it should be saying instead?

Example:

Defaults being destroyed looks like this inside console.
This is the behavior I want to avoid.

Defaults not being destroyed, looks like this inside console.
This is the behavior that should be happening in the code.

const managePlayer = (function makeManagePlayer() {
  const defaults = {};
  defaults.playerVars = {
    autoplay: 0,
    controls: 0,
    disablekb: 1,
    enablejsapi: 1,
    fs: 0,
    iv_load_policy: 3
  };

  function show(el) {
    el.classList.remove("hide");
  }

  function createPlayer(videoWrapper, settings = {}) {
    const video = videoWrapper.querySelector(".video");
    const playerOptions = combinePlayerOptions(defaults, settings);
    return videoPlayer.addPlayer(video, playerOptions);
  }

  function createCoverClickHandler(playerSettings) {
    return function coverClickHandler(evt) {
      const cover = evt.currentTarget;
      const wrapper = cover.nextElementSibling;
      show(wrapper);
      const player = createPlayer(wrapper, playerSettings);
      wrapper.player = player;
    };
  }

  function addPlayer(coverSelector, playerSettings) {
    const clickHandler = createCoverClickHandler(playerSettings);
    manageCover.addCoverHandler(coverSelector, clickHandler);
  }

  return {
    add: addPlayer
  };
}());

function onYouTubeIframeAPIReady() {
  managePlayer.add(".playa", {
    playerVars: {
      start: 45
    }
  });

  managePlayer.add(".playb", {
    playerVars: {
      playlist: "0dgNc5S8cLI,mnfmQe8Mv1g,-Xgi_way56U,CHahce95B1g"
    }
  });
  managePlayer.add(".playc", {
    playerVars: {
      start: 45
    }
  });
  manageCover.init({
    container: ".container",
    playButton: ".thePlay"
  });
}
18

The whole point if doing it is because there is a problem when managePlayer.init is used more than once.

Is that a problem that you are wanting to be solved?

1 Like
19

Are you saying that, if managePlayer.init is not being used in here, then,
there is no issue at all?

The issue only arises when managePlayer.init is being used?

Which is not the case in this code:

This code is fine is what you are saying:
https://jsfiddle.net/zfux48qn/

function onYouTubeIframeAPIReady() {
  managePlayer.add(".playa", {
    playerVars: {
      start: 45
    }
  });

  managePlayer.add(".playb", {
    playerVars: {
      playlist: "0dgNc5S8cLI,mnfmQe8Mv1g,-Xgi_way56U,CHahce95B1g"
    }
  });
  managePlayer.add(".playc", {
    playerVars: {
      start: 45
    }
  });
  manageCover.init({
    container: ".container",
    playButton: ".thePlay"
  });
}
20

Thank you for the clarification

The way these are set up, one of these would need to be fixed is what you are saying.

How is managePlayer.init being used more than once here?

You’re referring to just these?

managePlayer.init({
managePlayer.add
managePlayer.add
managePlayer.add

This should be the last init, at the end, unless I am wrong.
manageCover.init({ is separate by itself, it’s not part of the others.

This only deals with the cover and should remain at the end.

This shouldn’t be touched, or changed at all, right?

  manageCover.init({
    container: ".container",
    playButton: ".thePlay"
  });

To this point, I am so confused on what I should be doing, and what you said in this post does not make sense to me. Post # 15

This is what I am thinking:

managePlayer.add
managePlayer.add
managePlayer.init({
managePlayer.add

This one should stay at the end and not be touched.
manageCover.init({

I am not going to be able to understand this because this is all so confusing to me.

1st Way:
https://jsfiddle.net/rw7ukgaf/

managePlayer.init({
managePlayer.add
managePlayer.add
manageCover.init({
managePlayer.add

2nd Way:
https://jsfiddle.net/qb8pydrn/

manageCover.init({
managePlayer.add
managePlayer.add
managePlayer.init({
managePlayer.add
21

Asking for further clarification.

managePlayer or managePlayer.init?

Being used more than once?

This is still not good is what you are saying?

or is this fine, as long as managePlayer.init is not also being used in there?
https://jsfiddle.net/zfux48qn/

  managePlayer.add(".playa", {
    playerVars: {
      start: 45
    }
  });

  managePlayer.add(".playb", {
    playerVars: {
      playlist: "0dgNc5S8cLI,mnfmQe8Mv1g,-Xgi_way56U,CHahce95B1g"
    }
  });
  managePlayer.add(".playc", {
    playerVars: {
      start: 45
    }
  });
  manageCover.init({
    container: ".container",
    playButton: ".thePlay"
  });

Or is it only not good if this is being used also?

managePlayer.init ?

Meaning: managePlayer.init in conjunction with 3 of these is not good, and should be avoided.

managePlayer.add
managePlayer.add
managePlayer.add

These are fine by themselves and there is no issue with that?

managePlayer.add
managePlayer.add
managePlayer.add

The issue only arises when managePlayer.init is being used in there also?

Is that a better understanding of what you mean?

22

Okay then. I need to try and simplify things even further.

Can you please pick just one set of code that you think is reliably working, and I will try to find a simple test that demonstrates the problem.

1 Like