Collision Script in javascript help needed

Question

Alright so I just tried to cut down on lines of code by changing manually writing out everything into an array.. My problem is that he now teleports and gravity doesnt work... first of all I have a grid of cell objects which basically are a 32x32 grid "640X480". These objects are all passed onto an array like so-

var gridcellarray = [750];
gridcellarray[0] = cell0;
gridcellarray[1] = cell1;
gridcellarray[2] = cell2;
and so on for 750 32x32 cells...

Now as for the collision script I have this...

function collisioncheck(obj) {
    obj = obj;
    for(var i = 0; i < gridcellarray.length; i++){
    //really long if statement// sorry...
    if ((gridcellarray[i].solid == true) && ((obj.PosY >= gridcellarray[i].y - obj.maskImg.height) && !(obj.PosY >= gridcellarray[i].y ) && !((obj.PosX > gridcellarray[i].x + solidOriginX + solidImg.width/2-5) || (obj.PosX < gridcellarray[i].x - solidOriginX - solidImg.width/2)))){
         if(obj.onground == 0){
             obj.PosY = gridcellarray[i].y - obj.maskImg.height;
             obj.VelY = 0;
             obj.onground = 1;
             obj.jump = 0;
         }
      }
     else if (obj.PosY >= canvas.height - obj.maskImg.height){
        if(obj.onground == 0){
            obj.VelY = 0;
            obj.onground = 1;
            obj.jump = 0;
            obj.PosY = canvas.height - obj.maskImg.height;
        }
    }
    else {
        obj.VelY += 1;
        obj.onground = 0;
    }
  }
 }

now this code worked just fine before If I had manually copied it 750 times for each cell and the problem is that Now that I have one iteration of it cycling through them as an array it gets confused and teleports me to the bottom and If I try to jump even when not below or on a block it teleports me back to the bottom.

//edit
I think all the variables should explain their purpose by name but if you have any questions please ask.

//edit
All Variables apply the object your checking collision for such as collisioncheck(player) here is a link to a running demo of the code... Zack Bloom's code is in it and it works great applied to the unposted horizontal collision scripts but vertically, it wont reset and acknowledge your standing on a block ie ongroud = true; Demo link

//edit
Ok now that Zack pointed out resetting the y to and x amount it helped alot but he is still not able to jump, as the onground variable doesnt want to reset when the collision is detected... oh and the teleporting is due to my upwards script -

  //Upwards Collision//     
  if ((cell.solid == true) && ((obj.PosY >= cell.y - 32) && !(obj.PosY > cell.y+32) && !((obj.PosX > cell.x + solidOriginX + solidImg.width/2-5) || (obj.PosX < cell.x - solidOriginX - solidImg.width/2)))){
         if (obj.onground == 0){
             obj.VelY += 1;
             obj.onground = 0;
             obj.PosY = cell.y + obj.maskImg.height-13; 
         }
     }

Any Ideas on how to fix THIS mess above? to stop him from teleporting? It is only meant to check if the top of the collision mask(red rectangle) is touching the block as if trying to jump through it, but it is meant to stop that from happening so you hit your head and fall back down. Thanks in Advance!

Alright so now everything is good besides the onground variable never resetting to 1 and the new code I posted for upwards collision causing him to teleport. — Oni Enzeru
– Oni Enzeru, Commented Nov 2, 2011 at 23:30

Zack Bloom · Accepted Answer · 2011-11-02 22:53:51Z

3

The else if / else really don't belong in the loop at all, they don't evaluate the cell being looped over, but will be triggered many times each time collisioncheck is called.

function collisioncheck(obj) {
    for(var i = 0; i < gridcellarray.length; i++){
        var cell = gridcellarray[i];

        if (cell.solid && ((obj.PosY >= cell.y - obj.maskImg.height) && !(obj.PosY >= cell.y ) && !((obj.PosX > cell.x + solidOriginX + solidImg.width/2-5) || (obj.PosX < cell.x - solidOriginX - solidImg.width/2)))){
             if(!obj.onground){
                 obj.PosY = cell.x - obj.maskImg.height;
                 obj.VelY = 0;
                 obj.onground = 1;
                 obj.jump = 0;

                 break;
             }
         }
     }
     if (obj.PosY >= canvas.height - obj.maskImg.height){
        if(!obj.onground){
            obj.VelY = 0;
            obj.onground = 1;
            obj.jump = 0;
            obj.PosY = canvas.height - obj.maskImg.height;
        }
     } else {
        obj.VelY += 1;
        obj.onground = 0;
     }
}

But an even better way of doing it would be to store each gridcell based on where it was, so you just have to look up the gridcells near the ship.

// You only have to do this once to build the structure, don't do it every time you
// need to check a collision.
var gridcells = {};
for (var i=0; i < gridcellarray.length; i++){
    var cell = gridcellarray[i];

    var row_num = Math.floor(cell.PosX / 32);
    var col_num = Math.floor(cell.PosY / 32);

    if (!gridcells[row_num])
        gridcells[row_num] = {};

    gridcells[row_num][col_num] = cell;
}

// Then to check a collision:
function collisioncheck(obj){
    // I'm not sure exactly what your variables mean, so confirm that this will equal
    // the width of the object:
    var obj_width = solidImg.width;
    var obj_height = obj.maskImg.height;

    var collided = false;

    var left_col = Math.floor(obj.PosX / 32),
        right_col = Math.floor((obj.PosX + obj_width) / 32),
        top_row = Math.floor(obj.PosY / 32),
        bottom_row = Math.floor((obj.PosY + obj_height) / 32);

    for (var row=top_row; row <= bottom_row; row++){ 
        for (var col=left_col; col <= right_col; col++){
            var cell = gridcells[row][col];

            if (cell.solid){
                collided = true;

                if (row == top_row){
                    // We collided into something above us

                    obj.VelY = 0;
                    obj.PosY = cell.PosY + 32;
                } else if (row == bottom_row && !obj.onground){
                    // We collided into the ground

                    obj.PosY = cell.x - obj_height;
                    obj.VelY = 0;
                    obj.onground = 1;
                    obj.jump = 0;
                }

                if (col == left_col){
                    // We collided left

                    obj.VelX = 0;
                    obj.PosX = cell.PosX + 32;
                } else if (col == right_col){
                    // We collided right

                    obj.VelX = 0;
                    obj.PosX = cell.PosX - obj_width;
                }
            }
        }
    }

    if (obj.PosY >= canvas.height - obj_height){
        if (!obj.onground){
            obj.VelY = 0;
            obj.onground = 1;
            obj.jump = 0;
            obj.PosY = canvas.height - obj_height;
        }
    }

    if (!collided){
        obj.VelY += 1;
        obj.onground = 0;
    }
}

Rather than looping through 720 cells each frame, we are only looking at the cells we know we are overlapping. This is much more efficient and easier to read than all the position calculations.

edited Nov 2, 2011 at 22:53

answered Nov 2, 2011 at 0:37

Zack Bloom

8,4372 gold badges23 silver badges27 bronze badges

Sign up to request clarification or add additional context in comments.

10 Comments

Oni Enzeru Over a year ago

Even with your code he still teleports around once you get below a set of blocks he will automatically teleport to the top of it..

Zack Bloom Over a year ago

That's due to your obj.PosY = cell.x - obj.maskImg.height; code, you are literally saying "When there is a collision, move the object to the top of the cell". Change that (possibly to consider the velocity), to do whatever you intend.

Zack Bloom Over a year ago

For example, if you want to be able to pass through things when going up, but land when falling, check if VelY is negative in the collision check.

Zack Bloom Over a year ago

@OniEnzeru Before I didn't actually touch the collision code, but now I've rewritten it to work in a way which should make more sense.

Oni Enzeru Over a year ago

Thanks Zack you've been a real help with this! please check out the demo link to see what im talking about and it should only let him land on the blocks I also have a script to check if he hits his head so to speak and if so to stop him...Demo

|

RobG · Accepted Answer · 2011-11-02 00:43:59Z

Some comments on your code:

var gridcellarray = [750];

That creates an array with a single member that has a value of 750. I think you are assuming that it is equivalent to:

var gridcellarray = new Array(750);

which creates an array with a length of 750. There is no need to set the size of the array, just initialise it as an empty array and assign values.

var gridcellarray = [];
gridcellarray[0] = cell0;

This replaces the value of the first member with whatever the value of cell0 is.

function collisioncheck(obj) {
    obj = obj;

There is no point in the second line, it just assigns the value of obj to obj (so it's redundant).

  for(var i = 0; i < gridcellarray.length; i++){

It is much more efficient in most browsers to save the value of gridcellarray.length, otherwise it must be looked up every time (the compiler may not be able to work whether it can cache it), so:

  for (var i = 0, iLen = gridcellarray.length; i < iLen; i++) {

It is more efficient to store a reference to gridcellarray[i] rather than look it up every time. Also, you can use a short name since it's only used within the loop so its purpose is easily found:

    var c = gridcellarray[i];

so not only will the code run faster, but the if statement has fewer characters (and you might prefer to format it differently):

if ((c.solid == true) &&
    ((obj.PosY >= c.y - obj.maskImg.height) && !(obj.PosY >= c.y ) &&
    !((obj.PosX > c.x + solidOriginX + solidImg.width/2-5) ||
      (obj.PosX < c.x - solidOriginX - solidImg.width/2)))) {

Wow, that really is some if statement. Can you break it down into simpler, logical steps?

Ah, Zack has posted an answer too so I'll leave it here.

Thanks RobG and yeah Im just learning javascript as it is and programming in general... and I was just going out of the book I have and it said to do that...kinda stupid a beginners would would include such redundant things... Thank you for explaining things!

Collectives™ on Stack Overflow

Collision Script in javascript help needed

2 Answers 2

10 Comments

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

10 Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related