Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Double checking the gamepad docs are correct #14874

Open
greggman opened this issue Apr 11, 2022 · 2 comments
Open

Double checking the gamepad docs are correct #14874

greggman opened this issue Apr 11, 2022 · 2 comments
Labels
Content:WebAPI Web API docs effort: medium This task is a medium effort. help wanted If you know something about this topic, we would love your help!

Comments

@greggman
Copy link
Contributor

greggman commented Apr 11, 2022

MDN URL

/en-US/docs/Web/API/Gamepad_API/Using_the_Gamepad_API

What specific section or headline is this issue about?

Multiple sections

What information was incorrect, unhelpful, or incomplete?

I could be reading the article wrong but ... stuff like this

Note: The Gamepad object is available on the gamepadconnected event rather than the Window object itself, for security reasons. Once we have a reference to it, we can query its properties for information about the current state of the gamepad. Behind the scenes, this object will be updated every time the gamepad's state changes.

What did you expect to see?

AFAIK This wrong.

#1. While it's true a gamepad object will be provided on gamepadconnected, it's also true that gamepad objects will be provided by navigator.getGamepads

#2. The description above and elsewhere suggests holding on to a reference to the gamepad object and querying it for values. But this fails. In fact, the example code in the article does not do this. It updates the gamepad objects near the end

       controllers[gamepads[i].index] = gamepads[i];

In other words, it's updating the controllers array with a new gamepad object for the same gamepad.

If instead you did this

   // no need to use a new gamepad object if we already have a gamepad object
   // for this controller from the gamepadconnected event
   if (!controllers[gamepads[i].index]) {
      controllers[gamepads[i].index] = gamepads[i];
   }

You'd see the example would fail. At least in Chrome I get a few readings and then gamepad object I was holding a reference too goes dead. Where as if I made sure to get new gamepad objects by calling navigator.getGamepads then the code keeps running.

Let me add, holding on to a reference used to work. I was grabbing some old code that held references and I know worked great when I wrote it 7 yrs ago but suddenly it was not working. I tracked the issue down to this. You can't keep a reference to the gamepad object. You need to take new ones by calling getGamepads

Do you have any supporting links, references, or citations?

No response

Do you have anything more you want to share?

No response

MDN metadata

Page report details
@github-actions github-actions bot added the needs triage Triage needed by staff and/or partners. Automatically applied when an issue is opened. label Apr 11, 2022
@greggman
Copy link
Contributor Author

Also, I'd be nice if the code was modern using let and const where appropriate and not using the arguably bad var. I get that's a personal opinion but it would be nice to steer new devs into better, safer, practices.

@sideshowbarker sideshowbarker added help wanted If you know something about this topic, we would love your help! Content:WebAPI Web API docs effort: medium This task is a medium effort. and removed needs triage Triage needed by staff and/or partners. Automatically applied when an issue is opened. labels Apr 11, 2022
@greggman
Copy link
Contributor Author

Here's the code cleaned up.

  • var removed.
  • Using a Map since things are indexed by number not by string.
  • Use the more compact string templates instead of concatenating strings.
  • Use elem.classList.toggle instead of manually manipulating the className
  • use elem.textContent (safe) instead of elem.innerHTML (not safe)
  • use descriptive variable names
  • fix the bug that adds a new rAF loop for every controller and never removes them even if the controller is removed.
  • use querySelector instead of getElementById, getElementsByClassName, etc...
  • Don't query for an arrays of elements, just query the parent and then index the children (arguably you shouldn't query at all but that would require a bigger refactor)
<style>
.axes {
  padding: 1em;
}

.buttons {
  margin-left: 1em;
}

.axis {
  min-width: 200px;
  margin: 1em;
}

.button {
  display: inline-block;
  width: 1em;
  text-align: center;
  padding: 1em;
  border-radius: 20px;
  border: 1px solid black;
  background-image: url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAIAAACQd1PeAAAAAXNSR0IArs4c6QAAAAxJREFUCNdjYPjPAAACAgEAqiqeJwAAAABJRU5ErkJggg==);
  background-size: 0% 0%;
  background-position: 50% 50%;
  background-repeat: no-repeat;
}

.pressed {
  border: 1px solid red;
}

.touched::after {
  content: "touch";
  display: block;
  position: absolute;
  margin-top: -0.2em;
  margin-left: -0.5em;
  font-size: 0.8em;
  opacity: 0.7;
}
</style>
<body>
<h2 id="start">Press a button on your controller to start</h2>
</body>
<script>

const haveEvents = 'ongamepadconnected' in window;
const gamepadsByIndex = new Map();

function connecthandler(e) {
  addgamepad(e.gamepad);
}

function addgamepad(gamepad) {
  gamepadsByIndex.set(gamepad.index, gamepad);

  const indexElem = document.createElement("div");
  indexElem.setAttribute("id", `controller${gamepad.index}`);

  const idElem = document.createElement("h1");
  idElem.appendChild(document.createTextNode(`gamepad: ${gamepad.id}, mapping: ${gamepad.mapping}`));
  indexElem.appendChild(idElem);

  const buttonsElem = document.createElement("div");
  buttonsElem.className = "buttons";
  for (let i = 0; i < gamepad.buttons.length; i++) {
    const elem = document.createElement("span");
    elem.className = "button";
    elem.textContent = i;
    buttonsElem.appendChild(elem);
  }

  indexElem.appendChild(buttonsElem);

  const axesElem = document.createElement("div");
  axesElem.className = "axes";

  for (let i = 0; i < gamepad.axes.length; i++) {
    const progressElem = document.createElement("progress");
    progressElem.className = "axis";
    progressElem.setAttribute("max", "2");
    progressElem.setAttribute("value", "1");
    progressElem.textContent = i;
    axesElem.appendChild(progressElem);
  }

  indexElem.appendChild(axesElem);

  const startElem = document.querySelector("#start");
  startElem.style.display = "none";

  document.body.appendChild(indexElem);
}

function disconnecthandler(e) {
  removegamepad(e.gamepad);
}

function removegamepad(gamepad) {
  const d = document.querySelector(`#controller${gamepad.index}`);
  document.body.removeChild(d);
  gamepadsByIndex.delete(gamepad.index);
}

function updateStatus() {
  if (!haveEvents) {
    scangamepads();
  }

  for (const [ndx, gamepad] of gamepadsByIndex.entries()) {
    const controllerElem = document.querySelector(`#controller${ndx}`);
    const buttons = controllerElem.querySelector(".buttons");

    for (let i = 0; i < gamepad.buttons.length; i++) {
      const buttonElem = buttons.children[i];
      let value = gamepad.buttons[i];
      let pressed = value == 1;
      if (typeof value == "object") {
        pressed = value.pressed;
        value = value.value;
      }

      const pct = Math.round(value * 100);
      buttonElem.style.backgroundSize = `${pct}% ${pct}%`;

      buttonElem.classList.toggle("pressed", pressed);
    }

    const axesElem = controllerElem.querySelector(".axes");
    for (let i = 0; i < gamepad.axes.length; i++) {
      const axixElem = axesElem.children[i];
      const axis = gamepad.axes[i];
      axixElem.innerHTML = `${i}: ${axis.toFixed(4)}`;
      axixElem.setAttribute("value", axis + 1);
    }
  }

  requestAnimationFrame(updateStatus);
}

function scangamepads() {
  const gamepads = navigator.getGamepads()
  for (let i = 0; i < gamepads.length; i++) {
    if (gamepads[i]) {
      if (gamepadsByIndex.has(gamepads[i].index)) {
        gamepadsByIndex.set(gamepads[i].index, gamepads[i]);
      } else {
        addgamepad(gamepads[i]);
      }
    }
  }
}

requestAnimationFrame(updateStatus);
window.addEventListener("gamepadconnected", connecthandler);
window.addEventListener("gamepaddisconnected", disconnecthandler);

</script>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:WebAPI Web API docs effort: medium This task is a medium effort. help wanted If you know something about this topic, we would love your help!
2 participants