0

I'm trying to create a function that check if mooving bullet is hitting players.

The code simplified - shoot() is moving the bullet, and check_if_bullet_hit_player(bulletRef) gets a bullet ref, and checks if the bullet is hitting any player.

This is my code:

function shoot(){
  // ... moving bullet code
  const bulletRef = firebase.database().ref(`bullets/${bulletKey}`);
  if(check_if_bullet_hit_player(bulletRef)){
      alert("player got hit");
      bulletRef.remove();
  }
}
function check_if_bullet_hit_player(bulletRef){
  bulletRef.once("value").then(function(snapshot) {
    //for simplicity I just return true - player got hit
    return true;
  }
}

The code is not triggering the alert(). After reading this question I think the problem with the asynchrony, but how do I pass parameters to the function, and get the function's return value.

EDIT:

This is the full check_if_bullet_hit_player() function, which I loop over the players, and checks if the bullet is <0.5 to a player (if so - its count as a hit):

function check_if_bullet_hit_player(bulletRef){
  //bulletRef.once("value").then(function(snapshot) {
  return bulletRef.get().then(snapshot => {
    let bulletX = snapshot.val().x;
    let bulletY = snapshot.val().y;
    let shooterId = snapshot.val().shooterId;

    //loop over players
    const allPlayersRef = firebase.database().ref(`players`);
    allPlayersRef.once("value", (snapshot) => {      
      players = snapshot.val() || {};

      Object.keys(players).forEach((key) => {
          if(getDistance(players[key].x, players[key].y, bulletX, bulletY) < 0.5){

            if(players[key].id != shooterId){ //not the shooter
              return true;
            }
  // ...
Yagel
  • 1,184
  • 2
  • 18
  • 41

1 Answers1

1

Indeed the problem comes from the fact the once() method is asynchronous. But you also forget to return the promises chain in the check_if_bullet_hit_player() function.

The following, using then() as you do in your question, should do the trick:

function shoot() {
    // ... moving bullet code
    const bulletRef = firebase.database().ref(`bullets/${bulletKey}`);
    check_if_bullet_hit_player(bulletRef)
        .then(result => {
            if (result)
                bulletRef.remove();
        })
}

function check_if_bullet_hit_player(bulletRef) { 
    return bulletRef.get().then(snapshot => {  // See return at the beginning, and also that we use get()
       return snapshot.exists();  // I think you should check if the snapshot exists
    });
}

Note that the shoot() function is also asynchronous. So if you chain it with other function call, you need to use then() (and return the promises chain) or async/await.


Edit following your comment and your question update:

Again you are not returning the promise chain. You can modify it as follows, but I would recommend you use the every() method which "executes the provided callback function once for each element present in the array until it finds the one where callback returns a falsy value"

function check_if_bullet_hit_player(bulletRef) {
    
    let bulletX;
    let bulletY;
    let shooterId;    

    return bulletRef.get().then(snapshot => {
        bulletX = snapshot.val().x;
        bulletY = snapshot.val().y;
        shooterId = snapshot.val().shooterId;

        //loop over players
        const allPlayersRef = firebase.database().ref(`players`);
        return allPlayersRef.get();  // HERE we chain the promise
    })
        .then(allPlayersSnap => {
            players = allPlayersSnap.val() || {};
            let result = false;

            Object.keys(players).forEach((key) => {
                if (getDistance(players[key].x, players[key].y, bulletX, bulletY) < 0.5 && players[key].id != shooterId) {
                    result = true;
                }
            });
            return result;
        });

}

Extra note

It seems these functions are executed in the front end (you use firebase.database()...) You should probably do the calculations in the back-end, e.g. via a Cloud Function, to avoid possibilities for a malicious user to fake the results.

Renaud Tarnec
  • 79,263
  • 10
  • 95
  • 121
  • Thanks! I think I shorten the check_if_bullet_hit_player() a little too much. I edited the question, with the full code for the check_if_bullet_hit_player function. I basically check each movement of the bullet, how far the bullet from each player. Now, If I return true (player is close to the bullet & its not the shooter), its still doesn't work. So, Is my approach is correct, and why it doesn't work? Thanks again! – Yagel Mar 13 '22 at 14:52
  • 1
    See the update. – Renaud Tarnec Mar 13 '22 at 15:11
  • Thanks! I will read about cloud-functions - I knew about the fake results risk, but didn't know how to fix it. Also, I tried the edited function, and It seems that inside of *".then(allPlayersSnap = > {"*, it doest have access to the snapshot and the variables of its parent (e.g. bulletX, bulletY). any idea why? I also tried to put the bulletX, bulletY inside the *".then(allPlayersSnap = > {"*, but it didn't work – Yagel Mar 13 '22 at 15:32
  • 1
    "Any idea why?" => Yes, because these values were declared in the scope of the first `then()`. See the update. – Renaud Tarnec Mar 13 '22 at 15:40