vrld / HC

General purpose collision detection library for the use with LÖVE.
http://hc.readthedocs.org/
402 stars 48 forks source link

Consider replacing collision ids with 2d-colliding table #14

Closed kikito closed 12 years ago

kikito commented 12 years ago

It seems to me that collision Ids are only used to discard the pair "a,b" when the pair "b,a" has already been tested (by looking on the tested table inside HC:update:

if not tested[id] then ...

As well as to index the colliding && self._colliding_last_frame variables with collision info.

If that's the case, then I suspect that a rearranging colliding a bit, so that it stores the collision information in a 2-dimensional way, could save some cycles:

-- check for collisions
function HC:update(dt)
    local colliding = {} -- tested is gone
    for shape in self:activeShapes() do
        for other in shape:neighbors() do
            if not(colliding[other] and colliding[other][shape]) -- collision already tested
            and not (self._ghost_shapes[other] or self:share_group(shape, other)) then
                local collide, sx,sy = shape:collidesWith(other)
                if collide then
                    colliding[shape] = colliding[shape] or {}
                    colliding[shape][other] = {x=sx, y=sy}
                end
            end
        end
    end

    -- call colliding callbacks on colliding shapes
    for shape,others in pairs(colliding) do
        for other, mtv in pairs(others) do
            if self._colliding_last_frame[shape] then self._colliding_last_frame[shape][other] = nil end
            self.on_collide( dt, self, other, mtv.x, mtv.y )
        end
    end

    -- call stop callback on shapes that do not collide anymore
    for shape,others in pairs(self._colliding_last_frame) do
        for other, mtv in pairs(others) do
            self.on_stop( dt, self, other, mtv.x, mtv.y )
        end
    end

    self._colliding_last_frame = colliding
end

I warn you that I have not made performance tests at all; Intuitively I think it should work better than the previous implementation. It should invest a shorter time handling "non-colliding objects" in exchange of 1 extra function call to pairs and some assignments on each collision, so in the regular case (where most objects are not colliding) it should go faster. But please do test the performance.

kikito commented 12 years ago

I retract myself on a little part here. After implementing this on my own lib, I've realized that tested is still needed. This is because colliding[item][neighbor] will not be set if item and neighbor are indeed neighbors but they didn't collide. So we need a different table to avoid testing the opposite case. This said, I still don't think ids are needed - 2d arrays will do just fine:

-- check for collisions
function HC:update(dt)
    local colliding, tested = {}, {}
    for shape in self:activeShapes() do
        tested[shape] = {}
        for other in shape:neighbors() do
            if not(tested[other] and tested[other][shape]) -- collision already tested
            and not (self._ghost_shapes[other] or self:share_group(shape, other)) then
                local collide, sx,sy = shape:collidesWith(other)
                if collide then
                    colliding[shape] = colliding[shape] or {}
                    colliding[shape][other] = {x=sx, y=sy}
                end
                tested[shape][other] = true
            end
        end
    end

    -- call colliding callbacks on colliding shapes
    for shape,others in pairs(colliding) do
        for other, mtv in pairs(others) do
            if self._colliding_last_frame[shape] then self._colliding_last_frame[shape][other] = nil end
            self.on_collide( dt, self, other, mtv.x, mtv.y )
        end
    end

    -- call stop callback on shapes that do not collide anymore
    for shape,others in pairs(self._colliding_last_frame) do
        for other, mtv in pairs(others) do
            self.on_stop( dt, self, other, mtv.x, mtv.y )
        end
    end

    self._colliding_last_frame = colliding
end
vrld commented 12 years ago

I think the performance gain will be negligible, but the implementation seems cleaner than using a collision id (implicit mentioning of the involved shapes is sexy ;)). Will update as soon.