Enhanced CollisionHash/CollisionFilter.

[CookStar]

This makes it possible to manipulate collisions in both directions.
If you have a better function name, please suggest it.

[jordanbriere]

Thanks, but please leave the current implementation as is and inherit from ICollisionHash if you want to implement a different behaviour (just like we discussed on the forums). I prefer multiple classes that are simple and behave distinctly than a large one that tries to be them all. Moreover, it doesn't seem to currently work properly (e.g. has_pair always seems to return True for some reason).

EDIT: Just to add more details;

• The engine provides tools used for that very purpose; no need to reinvent the wheel.
• If we ever want to support physics collisions; it is already capable of.
• Your proposal is not even a "hash" anymore.

So, I would rather have the following 3 classes with explicit distinct behaviours:

h = CollisionHash()

# entity and other never collide with each other
h.add_pair(entity, other)



m = CollisionMap()

# entity never collide with other, but other collides with entity
m[entity][other] = True



s = CollisionSet()

# entity never collide with anything
s.add(entity)

I would also prefer them to be kept simple (e.g. no need for the from south, from north, to east, etc. methods; they are just duplicates of the indexing operator and are redundant).

EDIT: Ended up implementing it myself the way I wanted just so I can call it done with.

[CookStar]

• The engine provides tools used for that very purpose; no need to reinvent the wheel.

I may have said this before, but I don't trust Source Engine, and neither should you, and we shouldn't rely on something we don't know how it is implemented.

CCollisionHash/ObjectPairHash has a serious flaw, which is that it cannot add a sufficient number of pairs. In order to properly process entity pairs, we need to be able to store 2,096,128 different pairs, but we are not even 10% of that number.

Code:

from entities.collisions import CollisionHash
from entities.entity import Entity

collisions = CollisionHash()

for i in range(1000):
    index = Entity.create('env_beam').index

start_index = index-999
end_index = index+1
index1 = start_index
index2 = start_index
for i in range(int((1000*999)/2)):
    index2 += 1
    if index2 == end_index:
        index1 += 1
        index2 = index1
        index2 += 1
    print(f"Pair {i}: {index1}, {index2}")
    collisions.add_pair(Entity(index1), Entity(index2))

Output

Pair 32760: 102, 424
Pair 32761: 102, 425
Pair 32762: 102, 426
Pair 32763: 102, 427
Pair 32764: 102, 428
Pair 32765: 102, 429
Pair 32766: 102, 430
Pair 32767: 102, 431
CUtlMultiList overflow! (exhausted index range)
CUtlMultiList overflow! (exhausted index range)

CUtlMultiList overflow! (exhausted index range)


/home/steam/run_csgo.sh: line 16:  2466 Segmentation fault

ObjectPairHash should not be used for this purpose.

I prefer multiple classes that are simple and behave distinctly than a large one that tries to be them all.

I completely disagree. What we need to provide is a solid Collision Filter and various functions to access it. Splitting them up will only make it more difficult to manage them. For example, what if a plugin that was using CollisionHash wanted to use a directional collision filter, the plugin would have to start by verifying the correctness of both CollisionHash and CollisionMap.

And, I added Disable(CBaseEntity *pEntity), because in the use case that @Velocity-plus was targeting, it is more efficient to disable collision with all entities once and re-enable collision with required entities, rather than iterate over all entities and add them to a pair. Features such as CollisionSet do not match that.

I would also prefer them to be kept simple (e.g. no need for the from south, from north, to east, etc. methods; they are just duplicates of the indexing operator and are redundant).

EDIT: Ended up implementing it myself the way I wanted just so I can call it done with.

If you have a specific feature that you think is unnecessary, that's fine, just tell me, but it's not healthy to implement such a vague feature with a small number of people (especially just two!) with limited use cases. We should develop APIs for a wide range of use cases, involving other people.

[CookStar]

• If we ever want to support physics collisions; it is already capable of.

I chose bitvec because memory usage was a concern.
This is fine when there are only a few entities to filter, but when it comes to passing all entities in the server, boost::unordered_map<void *, boost::unordered_set<void *>> will quickly become a memory hungry mess. However, if we want to include physics collisions, our options are limited indeed.

[jordanbriere]

CCollisionHash/ObjectPairHash has a serious flaw, which is that it cannot add a sufficient number of pairs. In order to properly process entity pairs, we need to be able to store 2,096,128 different pairs, but we are not even 10% of that number.

Yes, I'm aware of this limitation. And just like everything else; this is not an issue until it becomes one. I doubt anyone will ever need that much space, and if they do, I will address it then.

Features such as CollisionSet do not match that.

CollisionSet is more adapted for this, because it affects all current and future entities. It is a global collision state tied to the entity itself, rather than having to constantly loop through all others. And such design is redundant anyways; just remove the object itself if you want to disable them all. No need for duplicated features.

We should develop APIs for a wide range of use cases

That's the opposite. It's always better to start with a simple and minimalist API that covers the current demand, and extend it with future requests. Trying to guess and cover unlimited of use cases is a good way to waste time developing features nobody will ever use and make the whole thing unnecessary complicated, Don't be a feature creep.

In any cases, my current branch works exactly how I wanted it to, and I won't debate about it. If there are any issues being reported, or extra features being requested, I will address them in due time. But for now, I'm very satisfied with its simplicity, usability, and flexibility.

Thanks.

[CookStar]

Yes, I'm aware of this limitation. And just like everything else; this is not an issue until it becomes one. I doubt anyone will ever need that much space, and if they do, I will address it then.

How can you tolerate a crash at less than 300 entities? It is not hard to imagine that this could easily exceed 300 entities. This is completely unacceptable.

CollisionSet is more adapted for this, because it affects all current and future entities. It is a global collision state tied to the entity itself, rather than having to constantly loop through all others. And such design is redundant anyways; just remove the object itself if you want to disable them all. No need for duplicated features.

How exactly do you remove a specific player or projectile from a CollisionSet for other player?
I would love to know how to achieve this other than iterating over all entities.

That's the opposite. It's always better to start with a simple and minimalist API that covers the current demand, and extend it with future requests. Trying to guess and cover unlimited of use cases is a good way to waste time developing features nobody will ever use and make the whole thing unnecessary complicated, Don't be a feature creep.

It is exactly you who is making it a feature creep, and it is obvious that CollisionHash/CollisionSet is unnecessary in the first place.

In any cases, my current branch works exactly how I wanted it to, and I won't debate about it. If there are any issues being reported, or extra features being requested, I will address them in due time. But for now, I'm very satisfied with its simplicity, usability, and flexibility.

Thanks.

One last thing, is Source-Python-Dev-Team/Source.Python your personal project? If you don't accept my opinion or refuse to discuss it, that's your right, but it's not fair to unilaterally close the discussion on an issue that affects Source.Python as a whole.

[jordanbriere]

How can you tolerate a crash at less than 300 entities? It is not hard to imagine that this could easily exceed 300 entities. This is completely unacceptable.

Well, that's not "300 entities". That's "300 entities paired with each others"; which is quite different. In a real scenario, not one that purposely overflow it, this limit should not really be an issue because in the end, that's quite a lot of pairs to override the collisions of. Though, I will push a fix later to address it. I haven't poked around it yet but it should be very easy to implement with a set of pairs and a custom hasher (so that they match in both directions). Which should also improve get_pairs anyways (the stack allocation can be quite inefficient).

How exactly do you remove a specific player or projectile from a CollisionSet for other player?
I would love to know how to achieve this other than iterating over all entities.

Entities that are contained into a CollisionSet have their collision disabled with everything (current entities, future entities, etc.) as long as they are still into it. This is an isolated global state that don't discriminate with what the entities are about to collide with. Which is exactly what the code you linked seems to do at first glance.

The only reason why you need to loop through all entities in your implementation, is because you keep a Boolean state for every entities for every others and that at all time. Which, in my opinion, is unnecessary. The lack of presence is enough to tell you whether you should allow their collisions or not.

It is exactly you who is making it a feature creep, and it is obvious that CollisionHash/CollisionSet is unnecessary in the first place.

They have specific behaviours, for different needs.

• CollisionHash; both-ways collisions.
• CollisionMap; one-way collisions.
• CollisionSet; global collisions.

Explicit purposes allowing scripters to pick the one that fit their needs. Which to me can be described with one word; simplicity. A huge class that tries to be them all is much less straight-forward than simple ones that have very specific and targeted usages.

One last thing, is Source-Python-Dev-Team/Source.Python your personal project? If you don't accept my opinion or refuse to discuss it, that's your right, but it's not fair to unilaterally close the discussion on an issue that affects Source.Python as a whole.

The thing is that you didn't create an issue. You made a PR to request changes to be merged into my branch. Changes that goes directly against the way I told you beforehand I wanted it to be implemented so I'm really not sure what you expected.