vrld / HC

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

Polygons disappearing #44

Closed JomerDev closed 6 years ago

JomerDev commented 8 years ago

I'm having a bit of a problem with HC.

I have a map with many isometric tiles where I needed to check if the mouse is hovering over them, I use HC:polygon to add those tiles to HC and I also have a 'mouse' circle just like the tutorial

I now have two problems: 1: Only half of the tiles are actually in cells, as you can see in this image image

The collisions for tiles that are in the filled cells get detected correctly.

2: Sadly only for a few seconds. Because after that, most (if not all) of the filled cells get removed and only one or two tiles get detected correctly image

My code looks somewhat like this: ` -- in love.load -- This gets called in a loop for every tile map.collider:polygon(x-32,y,x,y-16,x+32,y,x,y+16)

map.mouse = map.collider:circle(400,300,1) map.mouse:moveTo(love.mouse.getPosition())

-- in love.draw local shape = map.collider:collisions(map.mouse) for i,v in pairs(shape) do local _x, _y = i:center() love.graphics.circle("fill",_x,_y,5) end

-- in love.update map.mouse:moveTo(love.mouse.getPosition()) `

vrld commented 8 years ago

Sorry for the late reply: Do you have a .love that reproduces the error? I have the suspicion that the error is in spatialhash.lua, but would like to take a closer look at the problem first.

JomerDev commented 8 years ago

Sadly I don't have the code anymore. I could try to add HC back in and see if I can reproduce it, but the codebase has changed a lot since then.

Although there isn't much more than I've writen in the Issue. What interesst you appart from that? Maybe I can show you what you need to see

sixFingers commented 8 years ago

@vrld I'm experiencing the same issue. It happens with both HC spatial hash (like, in the attached example) and spatial hashes created through HC.new(). The example is a .love, just rename extension. testcase.zip

Jellonator commented 8 years ago

I figured out why this is. SpatialHash stores shapes by weak reference, which means that if you don't keep a reference to your shape, it will be garbage collected.

There are two ways this can be resolved. You can modify line 56 of spatialhash.lua, going from cell = setmetatable({}, {__mode = "kv"}) to cell = {}, which will make sure that cells keep strong references instead of weak references. This will prevent shapes from being garbage collected.
Your second option is to always keep a reference to your shape somewhere, like in a table of tiles or something.

This is not really a bug, it's actually by design (I assume). I think the creator wanted to make sure that there aren't any 'dangling' shapes left laying around hogging up resources. I personally think it's kinda silly and non-deterministic, but that's just me.

vrld commented 8 years ago

The problem is in fact that the spatial hash keeps weak references. That is only half of the problem, though; the other half is that the shape is thrown away the shape after creation. This leaves the weak reference dangling until the garbage collector picks it up.

I think the creator wanted to make sure that there aren't any 'dangling' shapes left laying around hogging up resources.

That is true. HC's job is to find collisions between shapes, but it does not manage those shapes for you. If the spatial hash would keep strong references, there would have to be a HC:remove(shape) that removes the reference when you do not want it anymore. However, that means you have to have remove "active" shapes twice: once from HC and once from your game's bookkeeping (NB: previous versions of HC had this anti-feature).

I feel that the current solution is much cleaner, because it separates concerns: HC does collision detection; bookkeeping has to be done by someone else.

vrld commented 8 years ago

I just realized that there actually is a HC:remove(shape)... I'll have to think about this ;)

sixFingers commented 8 years ago

From my current experience with HC, while i like the concept of separation of concerns, during development I felt that it could be easier to just let SpatialHash keep strong references. So that HC thinks about "physicality" of shapes, and I as a developer think about "logicality" of shapes. It will be my work as a dev to keep references to those shapes for logic stuff, while it would be the lib work to keep references for the physical stuff. The turning point is then HC.resetHash. I have huge worlds, for whom i keep references to shapes since i append data to them; it makes sense to be able to remove them from HC, while still keeping them for state purposes.

Just to add problems to problems, i had a note about this in my code. Can't remember exactly the issue, but i couldn't fully reset HC SpatialHash Copy-pasting:

-- This should really be documented
-- to vrld, since i expected HC.resetHash()
-- to effectively reset the world ..
-- well use this before an HC.resetHash().

Helpers.clearHardon = function(world)
    local shapes = world.hash:shapes()
    for shape in pairs(shapes) do
        world:remove(shape)
    end
end

Hope this can help.

Oberon00 commented 7 years ago

As I understand it, the intention of using a weak-table for the shapes is to automatically remove them as soon as no external references are kept. However, as the OP demonstrates this does not really work: The shapes will be kept in the weak table until the GC runs. So we end up with the following cases:

In the end, I still need to explicitly call remove if I don't want to risk collisions with "ghost-objects". Would we use ordinary strong references, collision detection would consider the shape, which is clearly better than undefined behavior.

Also, I have a use case where I draw objects to a canvas without clearing it (i.e. I don't need a reference to keep the object visible) and the only time I would need the drawn object is when another object (for which I have an external reference) collides with it.

TL;DR: I see zero benefit in a weak table as it only introduces non-determinism.

buckle2000 commented 7 years ago

@vrld can you just add an argument to new() that specifies whether a collider should use weak table?