ShouldCollide causes physics engine to mess up when certain things are done.

[crazycarpet]

This is just an example, I'm aware there is a setting for collision with teammates.
In the example:

function GM:ShouldCollide(enta, entb) if enta:Team() ~= entb:Team() then return false end end

if I were to change my team well standing in another player there is a high chance source engine's physics will totally fail and objects will start floating around.

I am not calling return true anywhere in ShouldCollide.

[Lexicality]

Source does not respond well to collision rules changing on an entity
mid-game. I seem to remember Garry was going to add logic for custom
collision groups to solve this issue. Does anyone know what happened to
that?

On 17 December 2013 06:58, crazycarpet notifications@github.com wrote:

This is just an example, I'm aware there is a setting for collision with
teammates.
In the example:

function GM:ShouldCollide(enta, entb)
if enta:Team() ~= entb:Team() then
return false
end
end

if I were to change my team well standing in another player there is a
high chance source engine's physics will totally fail and objects will
start floating around.

I am not calling return true anywhere in ShouldCollide.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/642
.

[robotboy655]

Probably the same thing that happened to everything else - Rust.

[crazycarpet]

Lol ^ so true, you can enable custom collision on the entity, but it doesn't help. I mean it seems to happen less often now.

Garry - Please fix this!

[xaviergmail]

Could we get an update on this? I have some collision checks that simply can't be done with the current collision groups and it gets really annoying. I also have the same code running on two different maps and it only happens on one. (rp_evocity2_v2p)

I haven't been able to figure out exactly what causes it, but I cache collisions every tick so it can't be returning a different value in the same tick for the two entities.

if I were to change my team well standing in another player there is a high chance source engine's physics will totally fail and objects will start floating around.

Hmm, I will try caching the first collision state until it stops colliding to see if that'll help with anything.

[xaviergmail]

Hmm, I will try caching the first collision state until it stops colliding to see if that'll help with anything.

A year later, this issue still persists and I can't figure out for the life of me a reliable way to make it crash (oy any reproducible test scenario at all), it only happens in production at the most unfortunate times

[thegrb93]

It's probably best to avoid it. Physical collisions aren't the only thing that call it e.g. traces (i think), so finding the problem is probably impossible.

[xaviergmail]

I'm not entirely sure how to avoid it seeing as I need dynamic collision rules for certain things in my gamemode and I can't resort to collision groups :S

[Mista-Tea]

@xaviergmail Have you given this thread a read? It contains some useful information that might help.
https://facepunch.com/showthread.php?t=1540390

[mcNuggets1]

Is there any error message?

[Pdzly]

Any updates here ?

[Kefta]

There will never be.

[Pdzly]

sadge

[31east]

when

[RaphaelIT7]

Why not store the return value of ShoudCollide for both entities and if it changes without CollisionRulesChanged getting called it will return the old value and throw a warning, or it calls CollisionRulesChanged in the next tick to update it.
It would at least stop the Physics engine from breaking

[WilliamVenner]

@RaphaelIT7 I believe it already does that, but it’s not the cause of this issue; even if you do it properly and call CollisionRulesChanged it still happens.

[RaphaelIT7]

@RaphaelIT7 I believe it already does that, but it’s not the cause of this issue; even if you do it properly and call CollisionRulesChanged it still happens.

Calling CollisionRulesChanged properly works fine for me. I have an entity that changes the collision rules with every player every 20 minutes, and it can also change randomly for one player. After I called CollisionRulesChanged for every possible change I never had this issue again. So I would think that everything works fine if you call it for all possible cases.

But to be sure, I tested a bit around with ShouldCollide, and it seems like CollisionRulesChanged really works completely fine.
It should also be noted that if you don't call CollisionRulesChanged it would only break the physics engine when something collides with the Entity/a Collision happens. It doesn't seem to break randomly.

This is btw how I tested it with CollisionRulesChanged. I'm randomly changing if it should collide each frame and I call CollisionRulesChanged on all Entities that have a PhysicsObject.
This works completely fine, but If I were to remove the CollisionRulesChanged call it can break Physics in a few seconds
(You would need to collide with the Entity or cause a collision with a prop to break the Physics).

local ret = math.random(0, 1) == 1
hook.Add("ShouldCollide", "Test", function(ent1, ent2)
	return ret
end)

hook.Add("Think", "Test", function()
	local last = ret
	ret = math.random(0, 1) == 1
	if last != ret then
		for _, ent in ipairs(ents.GetAll()) do
			if !IsValid(ent:GetPhysicsObject()) then continue end
			ent:CollisionRulesChanged()
		end
	end
end)

concommand.Add("enable_customcollison", function(ply) -- Spawn a prop, look at it and run this command.
	local ent = ply:GetEyeTrace().Entity
	if IsValid(ent) then
		ent:SetCustomCollisionCheck(true)
	end
end)

Would you maybe have a case where CollisionRulesChanged would fail?

[RaphaelIT7]

This issue is actually caused by not calling Entity:CollisionRulesChanged on the entity and Entity:CollisionRulesChanged being called.
In a debug build, IVP would crash but in a release build it just calls delete this; without any warning or error.
This seems to cause this undefined behavior.

I think the best solution would be to just cache the return value of ShouldCollide and return it.
This way we would reduce the amount of Lua calls, fix the issue (By requiring everyone to call CollisionRulesChanged) and probably improve performance.
The code is probably not the fastest, but it should work.

class CGarrysMod
{
public:
	[ ur other stuff ]
	bool ShouldCollide(CBaseEntity*, CBaseEntity*);
	void ClearCollideCache(CBaseEntity*);
};

std::unordered_map<CBaseEntity*, std::unordered_map<CBaseEntity*, bool>> cache;
bool CGarrysMod::ShouldCollide(CBaseEntity* ent1, CBaseEntity* ent2) // It seems like ent1 is always the Entity with CustomCollisions enabled.
{
	std::unordered_map<CBaseEntity*, bool> entires;
	if (cache.find(ent1) == cache.end()) {
		cache[ent1] = entires;
	} else {
		entires = cache[ent1];
	}

	if (entires.find(ent2) != entires.end()) {
		return entires[ent2];
	}

	// Do the Lua call

	bool val = true; /*LUA->GetBool(1)*/
	entires[ent2] = val;

	return val;
}

void CGarrysMod::ClearCollideCache(CBaseEntity* ent) // Call it from CBaseEntity::CollisionRulesChanged
{
	cache.erase(ent);
}

And at least a Warning should be readded to print that somehow it failed again, just to be sure (If it should happen)

Edit: Removing the delete this; also solves this bug, but it could cause other issues, but I couldn't find any.

Edit 2:
Replacing the delete this; with

for (int i = 0;i<2;i++){
	objects[i]->recheck_collision_filter();
}

Seems to fix this, and the IVP_Mindist will also be deleted without it breaking the physics engine. So this should actually be the real fix.
NOTE: You also need to move the if statement to the end of the function like here