When using padding: bodies are sometimes not deleted with system.delete(body)

[mdealiaga]

Bodies in my project are sometimes (seemingly randomly) not getting removed when calling system.remove(body). It seems to happen more often when many objects are bunched together

This issue fixes itself when I set padding = 0 when creating that body.

I have only tested with Circle objects

When the body remains, the object looks like this (note the system: undefined):

Circle {
  pos: Vector { x: 7251, y: 8597 },
  r: 6,
  offset: Vector { x: 0, y: 0 },
  padding: 18,
  type: 'Circle',
  minX: 7227,
  minY: 8573,
  maxX: 7275,
  maxY: 8621,
  system: undefined,

This is happening since I upgraded from the previous Sinova version

if I set a deleteProcessed=true flag to deleted bodies, and then add to my game loop:

  bodies.forEach(body => {
        if (body.deleteProcessed) { 
             body.systemAllDeleteProcessed = true
             system.remove(body)
            console.log('attempting to remove bullet body', body)
         }

This will delete many of the hanging bodies. However, some still seem to remain forever and ignore this loop.

I tried looking into the codebase to fix this but I'm not too familiar with how the core collision detection works. It seems to be mainly this function that uses it

export function updateAABB(body: Body, bounds: BBox): void {
  body.minX = bounds.minX - body.padding;
  body.minY = bounds.minY - body.padding;
  body.maxX = bounds.maxX + body.padding;
  body.maxY = bounds.maxY + body.padding;
}

Perhaps the core libraries work in a way that is causing issues when these properties are modified with padding?

As an additional note, padding did not only cause issues during the removal phase in my project. It also sometimes resulted in some strange behaviour where the entity was (seemingly randomly) not updated properly, making collision detection temporarily act strangely.

[Prozi]

hello thank you for opening an issue :)

When the body remains, the object looks like this (note the system: undefined):

this is like a normal circle would look after system remove actually

~/projects/detect-collisions$ node
Welcome to Node.js v18.7.0.
Type ".help" for more information.
> const { System } = require(".")
undefined
> const system = new System
undefined
> const circle = system.createCircle({}, 10)
undefined
> system
System {
  _maxEntries: 9,
  _minEntries: 4,
  data: {
    children: [ [Circle] ],
    height: 1,
    leaf: true,
    minX: -10,
    minY: -10,
    maxX: 10,
    maxY: 10
  },
  response: Response {
    a: null,
    b: null,
    overlapN: Vector { x: 0, y: 0 },
    overlapV: Vector { x: 0, y: 0 },
    aInB: true,
    bInA: true,
    overlap: 1.7976931348623157e+308
  }
}
> system.remove(circle)
System {
  _maxEntries: 9,
  _minEntries: 4,
  data: {
    children: [],
    height: 1,
    leaf: true,
    minX: Infinity,
    minY: Infinity,
    maxX: -Infinity,
    maxY: -Infinity
  },
  response: Response {
    a: null,
    b: null,
    overlapN: Vector { x: 0, y: 0 },
    overlapV: Vector { x: 0, y: 0 },
    aInB: true,
    bInA: true,
    overlap: 1.7976931348623157e+308
  }
}
> circle
Circle {
  pos: Vector { x: 0, y: 0 },
  r: 10,
  offset: Vector { x: 0, y: 0 },
  padding: 0,
  angle: 0,
  isConvex: true,
  isCentered: true,
  type: 'Circle',
  offsetWithAngle: { x: 0, y: 0 },
  isStatic: false,
  isTrigger: false,
  radiusBackup: 10,
  minX: -10,
  minY: -10,
  maxX: 10,
  maxY: 10,
  system: undefined
}
> 

what makes you believe that body is still part of the collision system?

[mdealiaga]

Hi, thanks for your quick reply.

I made several tests to confirm:

• I am drawing all collisions in my game and can see hanging bodies remain there visually (see https://imgur.com/a/FPh5zze).
• When the player walks to it, the collision is registered.
• Furthermore, if I loop via

  system.all().forEach(body => {
  if (!body.system) {
   console.log('body still here', body)
  }
  }

  I can still see these bodies.

body still here Circle {
  pos: Vector { x: 8675, y: 8595 },
  r: 6,
  offset: Vector { x: 0, y: 0 },
  padding: 18,
  angle: 0,
  isConvex: true,
  isCentered: true,
  type: 'Circle',
  isStatic: false,
  isTrigger: false,
  radiusBackup: 6,
  minX: 8651,
  minY: 8571,
  maxX: 8699,
  maxY: 8619,
  system: undefined,
  active: false,
  ownerType: 'enemy',
  ownerID: 4,
  deleteProcessed: true,
  wasBullet: true
}

[Prozi]

hello again @mdealiaga

I've looked into detect-collisions codebase and I thought that the case you're describing might be because of wrongly using updateAABB (it should've been only used internally by system) or some bug during updateBody / create body / insert body action

So I've decided since 6.5.0 to replace totally remove updateAABB from body and utils and integrate what updateBody does into insert(body)

please update to 6.5.0, change the updateBody to insert and then see if it works correctly now please

[Prozi]

or actually you can update to 6.5.1 then you dont have to refactor so much propably

but I advise to use system.insert() instead of system.updateBody() it does the same thing now anyway

[mdealiaga]

Hi @Prozi, thank you for the update. I was using only body.updateAABB() (no physics.update()) and have refactored all of these to physics.insert(body). I re-enabled padding and bodies are now getting properly deleted, thanks for the fix!

Some minor notes:

• I'm not sure the deprecation worked properly, my typescript stopped compiling when I tried to update and use my old code with message Property 'updateAABB' does not exist on type 'Point'. It might have been my typescript wrapper breaking though. Since I updated to the new code, this is not affecting me but thought I would note.
• physics.insert() has a confusing naming since from a user perspective, they are updating an existing body, not inserting a new one. I know the AABB is technically re-inserting the body, but it's not how the user of the library sees it.physics.updateBody(body) seems more intuitive.

[Prozi]

Hi @Prozi, thank you for the update. I was using only body.updateAABB() (no physics.update()) and have refactored all of these to physics.insert(body). I re-enabled padding and bodies are now getting properly deleted, thanks for the fix!

that's great!

• I'm not sure the deprecation worked properly, my typescript stopped compiling when I tried to update and use my old code with message Property 'updateAABB' does not exist on type 'Point'. It might have been my typescript wrapper breaking though. Since I updated to the new code, this is not affecting me but thought I would note.

that is fine, because updateAABB was never supposed to be used anywhere other than system that one is actually not deprecated its removed as it was harmful to your code for example

calling updateAABB outside system resulted in updating bounding box without removing old one from tree which broke for example deletion in your case

• physics.insert() has a confusing naming since from a user perspective, they are updating an existing body, not inserting a new one. I know the AABB is technically re-inserting the body, but it's not how the user of the library sees it.physics.updateBody(body) seems more intuitive.

you have a point, I removed the deprecation warning from updateBody and updated readme to use updateBody as preferred update method as it surely makes more sense, even if underneath it just does system.insert(body)

thank you for the remarks - please check v6.5.6 it has the changes I mention above

[mdealiaga]

Thank you!

I did just read the code for updateBody

    updateBody(body) {
        this.insert(body);
    }

And had not realised I was suggesting to do the extra function call. I'm curious since I don't know fully how the JS compiler works, does this have any performance cost or does it gets optimised automatically? It probably is a negligible cost anyways.

[Prozi]

Thank you!

I did just read the code for updateBody

    updateBody(body) {
        this.insert(body);
    }

And had not realised I was suggesting to do the extra function call. I'm curious since I don't know fully how the JS compiler works, does this have any performance cost or does it gets optimised automatically? It probably is a negligible cost anyways.

yes, the argument of ease of use of updateBody vs insert when updating a body still stands, and I believe that updateBody has greater readability even if it adds another function call [which as you wrote has negligible cost anyways]

inside system and bodies I use system.insert to avoid the call.

you are free to use any of the two inside your projects

anyway, may I close the issue with the padding as resolved?