+THRUBITS flag

[Major Cooke]

Pull Request

When working on adding collision groups to my minecraft monsters against blocks, I discovered a problem. ThruBits is always active which means monsters could all go through each other and the player could too. This isn't good. Now, it's opt-in. Only one actor needs to have this flag in order to pass through the other of the same group now.

I discovered this issue when trying to make shotgun shells pass through all but the minecraft blocks - which worked, but monsters could also unintentionally pass through other monsters/players/projectiles because it's always on.

[Rachael]

Shouldn't Actor->ThruBits == 0 be sufficient to cover this use case? If it isn't, then this is a bug, not something that should be solved with an additional flag.

[Major Cooke]

That works fine. My minecraft blocks have 0 for thrubits, blocking everything.

Here's the issue I face. Monsters/players need a ThruBit that's not 0, in order to allow for things like shotgun shells casings to pass through them, otherwise they collide needlessly. But since there's only 31 groups, you can see how this quickly becomes a problem. There's hundreds of monsters at a time. None of them are meant to pass through each other or the player. They must collide.

Furthermore I have an event handler set up to assign monsters/players/projectiles with THRUACTORS to group 1, but only projectiles with +THRUACTORS have the flag replaced with +THRUBITS and also assigned to the entire spectrum. This ensures monsters/players still collide with each other, but the projectiles can go through them without hindrance, as intended.

That's currently impossible with how it is now, so it must be opt-in.

[Apeirogon]

ThruBits is always active which means monsters could all go through each other and the player could too.

This was one of the reason why I was suggesting to do it in other direction, instead of thrubits -> collidebits.
Collision is much more often/expected behavior for physic simulation than...eeeeeee thruing...case when one solid object should pass through another solid object volume without any interaction.

[Rachael]

Even if this isn't a bog on the engine (and it might not be, but I think Graf knows more than me on that), this is very confusing and unintuitive. I think the name at the very least should change to something that more clearly explains the flag's purpose.

At any rate, I am hesitant to accept this - I really would like some insight from Graf about what he thinks. If he's cool with it then I am too.

[Major Cooke]

It's not a bug. I can assure you. It's more along the lines of 'missing a component'.

The main problem is, it's an all-or-nothing system without this flag. I could change the name of the flag easily if desired to ALLOWTHRUBITS.

If you want I can upload a demo to show what the problem is without this flag.

[Rachael]

I get what you are saying, don't worry. It's just that the collision code is so performance sensitive that you really have to be careful touching it. One or two changes might not be a big deal but - ok - let's say, what happens in a map with 10k monsters? Is there going to be a major performance difference between GZDoom after this pull request is accepted, and 4.4.2 (if say, hypothetically, its memory leaks had been fixed)?

Adding to this code is not a trivial task and should never be treated as such, and this pull request begins just *exactly* what I was worried would happen with it, opening Pandora's box with the other...

[Apeirogon]

I think the name at the very least should change to something that more clearly explains the flag's purpose.

In Godot, for example, this thing are named "collision_mask", which is self explanatory IMO.

[Graf Zahl]

So, the feature isn't even official yet and here we already start with the overrides. Yuck!

[Major Cooke]

I see your concern, Rachael. What I've done is flip the order of checking so that the bits go first and then the flag, as it was previously. Makes sense as most actors will generally have 0 for it, so that should lessen the impact.

Graf: I simply thought this would be beneficial to have it start disabled off the bat than enabled by default. If you want to close it, that's fine. I can try and work with thinkers that basically interpret movement for some actors while disabling their collisions themselves entirely in the mean time.