Adding combinePlayerOptions into existing code

[Paul_Wilkins]

@asasass

Starting from this code, how would that be fixed?
https://jsfiddle.net/ae0Lwj5f/

It can help to get some visibility on the issue. Just before the managePlayer.init line add a new line that says debugger;
With the browser console open, run the code, and the debugger should open in the console. You can then click on line numbers to set breakpoints. I find it useful to set breakpoints at the start of functions, so I can see what happening there. In this case, breakpoints at the start of the init function, the addPlayer functions, the createCoverClickHandler function, and the createPlayer function are good places.

Then when you press the ResumeScriptExecution button code execution should stop at the start of the init function. This is where a second useful thing can be done, which is to watch a variable. In this case we want to add a watch to the playerOptions variable, so that we can easily see what happens to it.

After addng a watch for the playerOptions variable, I also unfurl it, so that all parts of playerOptions are easily visible in the watch area. I can then resume script execution.

When scrit execution pauses at the next breakpoint in the addPlayerRandomVideo function, I find that settings was used in there instead of playerOptions. Let’s rename that to playerOptions right now in both of the addPlayer functions.

On running the code again we don’t need that debugger statement thanks to the breakpoints we have in the code, so remove that debugger statement from earlier and run the code again.

The debugger pauses the script at the start of the init function, and we see in the watch area that playerVars has controls and fs properties. We can then press the step over next function call so that playerOptions is added to defaults, and we can hover over that defaults variable to investigate it.

There we find a problem. The defaults object inappropriately has a playerOptions object inside of it. We resolved this earlier by reducing that to be just a defaultOptions object, so let’s restructure that now. In the jsFiddle JS code pane update the defaults code structure so defaults and playerOptions are combined to be just defaultOptions, and defaults elsewhere lower in the code is renamed to be defaultOptions, and we can run the code again.

On running the code again the breakpoints are in different places. They are still at the same line numbers, but we’ve changed the lines in the code, so we need to update those breakpoints so that they are at the top of the function again. After updating that we can run the code again and step over the object.assign.

On doing that we can hover on the defaultOptions variable and confirm that playerVars has been updated. It hasn’t kept the previous default values that were in there. Object.assign is not the right way to do things when we have nested objects. That is why we have the combinePlayerOptions function instead. So replace Object.assign with using combinePlayerOptions, and use let instead of const on playerDefaults so that the init function can update that variable.

Run the code again, the breakpoint stops at the start of the init function. Step over that function call and confirm that defaultOptions is correct, and the process continues from there.

[asasass]

Step 1
https://jsfiddle.net/5ukgdmwa/

You want me to change playerSettings in both of these to playerOptions?

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

  function addPlayerRandomVideo(coverSelector, playerSettings, videoIds) {
    const index = Math.floor(Math.random() * videoIds.length);
    const videoId = videoIds[index];
    const clickHandler = createCoverClickHandler(playerSettings, videoId);
    manageCover.addCoverHandler(coverSelector, clickHandler);
  }

Step 2
https://jsfiddle.net/qamgtopL/

defaults get changed to defaultOptions

  const defaultOptions = {
      events: {
        "onReady": onPlayerReady
      },
      height: 360,
      host: "https://www.youtube-nocookie.com",
      playerVars: {
        playlist: playlist || undefined
      },
      videoId,
      width: 640
}

 const defaultOptions = {
      playerVars: {
        autoplay: 0,
        controls: 0,
        disablekb: 1,
        enablejsapi: 1,
        fs: 0,
        iv_load_policy: 3,
        rel: 0
      }
    }

What does this line become?
https://jsfiddle.net/qamgtopL/

const defaultOptions = defaults.playerOptions;

so defaults and playerOptions are combined to be just defaultOptions

This can’t work in the code like this though?

const defaultOptions = defaultOptions;

That line then gets deleted?
https://jsfiddle.net/289somkh/

    const defaultVars = defaultOptions.playerVars;
    const playerVars = settings;
    const playerOptions = combinePlayerOptions(defaultOptions, settings);
    playerOptions.playerVars = Object.assign({}, defaultVars, playerVars);
    const player = new YT.Player(video, playerOptions);
    players.push(player);
    return player;
  }

Step 4

So replace Object.assign with using combinePlayerOptions, and use let instead of const on playerDefaults so that the init function can update that variable.

In here I am changing:

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

const playerOptions to let playerDefaults?

This?
https://jsfiddle.net/289somkh/2/

    const defaultVars = defaultOptions.playerVars;
    const playerVars = settings;
    let playerDefaults = combinePlayerOptions(defaultOptions, settings);
    playerDefaults.playerVars = combinePlayerOptions(defaultVars, playerVars);
    const player = new YT.Player(video, playerDefaults);
    players.push(player);
    return player;
  }

What do I next?
I think I did everything you asked me to do.

https://jsfiddle.net/289somkh/2/

[Paul_Wilkins]

@asasass

What does this line become?
https://jsfiddle.net/qamgtopL/

const defaultOptions = defaults.playerOptions;

defaultOptions is already available so that line gets deleted.

@asasass

What do I next?
I think I did everything you asked me to do.

https://jsfiddle.net/289somkh/2/

We might need to do this a different way, so that you have easier to see landmarks to guide you. I’ll see what I can come up with.

[asasass]

I’ll wait for further guidance.

[Paul_Wilkins]

At the end of the init function you can add a console.log statement showing defaultOptions.
That way, it’s easy to see that after object.assign has occurred, the defaultOptions is not a valid combination of the defaults and the specified playerOptions.

What has gone wrong is that Object.assign has destroyed the previous playerVars that was there. That’s why combinePlayerOptions need to be used instead of Object.assign.

If we assign that combinePlayerOptions to a separate variable, such as preferred and use console.log on that preferred variable, it should be easy to see that playerVars is now a correct combination of options.

We now need to have that managePlayer code remember those preferred options for later on. It’s not suitable to update the defaultOptions as that destroys any opportunity to init the code again with different options.

We do have two different sets of options to keep track of, one being default and the other being preferred. That means that it’s reasonable to have an object called options, which has one object called default, and another empty object called preferred.

That way we can update options.preferred with those settings, so that other code inside of managePlayer can access that options.preferred information.

That way, options.defaults is only accessed (but not changed) only by the init function, and the combination of those defaults and init options are kept as the options.preferred object.

[asasass]

I’m confused.

I don’t understand what I am changing in here.
https://jsfiddle.net/dp8we3b6/

    const defaultVars = defaultOptions.playerVars;
    const playerVars = settings;
    let playerDefaults = combinePlayerOptions(defaultOptions, settings);
    playerDefaults.playerVars = combinePlayerOptions( defaultVars, playerVars);
    const player = new YT.Player(video, playerDefaults);
    players.push(player);
    return player;
  }

[Paul_Wilkins]

All of the things related to playerVars get deleted in there, because combinePlayerOptions properly does those things instead.

[asasass]

This is too complicated to figure out.

I am done working on this for now.

There are other things I want to work on that I haven’t been able to work on.

[asasass]

I think I was able to figure this out.

This gets changed to:
https://jsfiddle.net/mcy16bLw/

     function createPlayer(videoWrapper, playerOptions = {}, videoIds = "") {
        const video = videoWrapper.querySelector(".video");
        if (!videoIds) {
            videoIds = video.dataset.id;
        }
        playerOptions = combinePlayerOptions(defaults, playerOptions);
        return videoPlayer.addPlayer(video, playerOptions, videoIds);
    }

This:
https://jsfiddle.net/z4gvwdoy/

I added this:

     const fixedOptions = {
            playerVars: playerOptions
        };

To the code:

     function createPlayer(videoWrapper, playerOptions = {}, videoIds = "") {
        const video = videoWrapper.querySelector(".video");
        if (!videoIds) {
            videoIds = video.dataset.id;
        }
        const fixedOptions = {
            playerVars: playerOptions
        };
        playerOptions = combinePlayerOptions(defaults, fixedOptions);
        return videoPlayer.addPlayer(video, playerOptions, videoIds);
    }