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

[bug] sprite.collider = "none" should not remove overlap sensors #262

Closed
quinton-ashley opened this issue Sep 26, 2023 · 0 comments
Closed

Comments

@quinton-ashley
Copy link
Owner

quinton-ashley commented Sep 26, 2023

sprite.collider, is the getter/setter for a sprite's collider type. Behind the scenes in p5play, changing a sprite's collider type requires removing its entire planck.js physics body, sprite.body, and recreating it with the new body type.

sprite0.overlaps(sprite1, cb);
sprite0.collider = "none";

There's a problem with the current implementation. If a sprite's collider type is set to "none" after overlap detection was implemented, the sprite will not regain its overlap sensors. Hence the cb callback function will not run. But it should, since sprites can have sensors without having any colliders. Reverse the lines and it works, but it should work either way.

Solving this will be tricky because some games rely upon the current behavior.

Let's look at "Into the Mines" by Tezumi as an example. In this game breaking platforms get their collider type changed "none" (along with an image change to show the empty space) when they break, the platform's overlap sensor is also removed. This error prevents player.groundSensor.overlapping(breakingPlatforms) from returning true, which in this case is desirable.

When an overlapping check is performed, if a sprite being checked doesn't have overlap sensors, the sprite will automatically receive default sensors based on its collider shape. But if a group is being checked, p5play will only add default overlap sensors to a group if no sprites in the group have overlap sensors. So that's why the breakingPlatforms don't get their sensors re-added. But for consistency they should,

...because otherwise this would work if the sprite is in a group and the user is doing an overlap check on the group.

platform.collider = "none";
platform.removeSensors();

But that wouldn't work if the user checks for overlaps in a for loop instead.

for (let b of breakingPlatforms) {
  if (player.overlapping(b)) {
    // ...
  }
}

That could be solved by checking whether each platform b has a collider type of "none".

for (let b of breakingPlatforms) {
  if (b.collider != 'none' && player.overlapping(b)) {
    // ...
  }
}

That would also work in callback form.

function onGround(player, b) {
  if (b.collider != 'none') {
    // ...
  }
}

player.overlapping(breakingPlatforms, onGround);

The actual section of Tezumi's code where the overlap check occurs is in player.js.

if (
        player.groundSensor.overlapping(ground) ||
        player.groundSensor.overlapping(oneWayPlatforms) ||
        player.groundSensor.overlapping(movingPlatforms)
    ) {
    // ...
}

breakingPlatforms are added to a super group, called ground. I think the best solution is to just remove a broken platform from the ground group.

bp.collider = "none";
ground.remove(bp);

But the thing is the player.groundSensor.overlapping(ground) check still returns true. I think this is because removing the broken platform from the ground group doesn't cause an overlapped event between the player's groundSensor and the ground.

@quinton-ashley quinton-ashley added this to In Progress in p5play Project Planning Sep 26, 2023
@quinton-ashley quinton-ashley changed the title [bug] sprite.collider = "none" removes overlap sensors [bug] sprite.collider = "none" should not remove overlap sensors Oct 10, 2023
p5play Project Planning automation moved this from In Progress to Done! Oct 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

1 participant