Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding back ZoneComponent #5123

Draft
wants to merge 58 commits into
base: main
Choose a base branch
from

Conversation

MushAsterion
Copy link
Collaborator

@MushAsterion MushAsterion commented Mar 4, 2023

Adding back the zone component. It supports detection against entities position as well as using colliders. Dectection from colliders can be disabled from ZoneComponent#useCollisions. Zones an entity is member of can be accessed through Entity#zones property, it is a getter to avoid circular references as ZoneComponent contain entities in entities.

Here are the full API changes:

Entity

  • Properties:
    • zone: Gets the ZoneComponent attached to this entity.
    • zones: List of all zones this entity is currently within.
  • Events:
    • zoneEnter: Fired when the entity enters a zone.
    • zoneLeave: Fired when the entity leaves a zone.

ZoneComponent

  • Properties:
    • shape: The shape of this zone. Can be a box or a sphere.
    • halfExtents: The half-extents of the box-shaped zone in the x, y and z axes. Defaults to [0.5, 0.5, 0.5].
    • radius: The radius of the sphere-shaped zone. Defaults to 0.5.
    • useColliders: Whether the zone should look for colliders collision if entity is outside of it. Defaults to false.
    • entities: The list of entities currently inside this zone.
  • Methods:
    • isPointInZone(point): Check if a point is within the zone.
  • Events:
    • entityEnter: Fired when an entity enters the zone.
    • entityLeave: Fired when an entity leaves the zone.

ZoneComponentSystem:

  • Properties:
    • zones: Holds all the active zone components.

ZoneComponentData:

  • Properties:
    • shape: The shape of this zone. Can be a box or a sphere. Defaults to box.
    • halfExtents: The half-extents of the box-shaped zone in the x, y and z axes. Defaults to [0.5, 0.5, 0.5].
    • radius: The radius of the sphere-shaped zone. Defaults to 0.5.
    • useColliders: Whether the zone should look for colliders collision if entity is outside of it. Defaults to false.

CollisionComponent:

  • Properties:
    • zoneCheck: Whether this collider should be used to detect if entity is within zone. Defaults to false.

CollisionComponentData:

  • Properties:
    • zoneCheck: Whether this collider should be used to detect if entity is within zone. Defaults to false.

I also think that it would be nice to have an effects property on ZoneComponent with some predefined effects such as Audio Reverb or Force like some other game engines but I think the component itself is a first thing to get approved before going further.

CollisionComponent#zoneCheck and ZoneComponent#useColliders are set to false by default to optimize. Users must only enable when needed (character for example) to reduce calculations. May you want to play a bit with it just fork https://playcanvas.com/project/1045217 and add whatever script you might need to test it out.

Note: This PR is ready for review. Draft status is only because it requires another PR before being added.

I confirm I have read the contributing guidelines and signed the Contributor License Agreement.

@MushAsterion MushAsterion marked this pull request as draft March 4, 2023 16:53
Copy link
Contributor

@MAG-AdrianMeredith MAG-AdrianMeredith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't speak for the playcanvas maintainers but from my perspective it meets my needs quite nicely and I've built a reflection probe volume implementation using this where mutliple reflection probes can be switched into as objects move into its zone. This includes zones within zones also

@MAG-AdrianMeredith
Copy link
Contributor

An early build but I've got authorization to share it
https://github.com/playcanvas/engine/assets/100127317/429019b1-6f5a-460c-9f21-5b4c9b370683
Thanks everyone

@MAG-AdrianMeredith
Copy link
Contributor

Hey Mush, got some more feedback for you. I've recently enhanced my reflection probe work to now implement support for box projected cubemaps using the zones underneath (got a separate bug coming for those, rotation doesn't work ;) ). However I've found two issues

  1. MeshInstances can be a long way away from the position of the entity. In my Level mesh (exported from unreal) I noted that the entities seemed to be at 0,0,0 with the aabb of the mesh instances containing the real position of the visible mesh. So I added a isPointInZone check to succeed if at least one meshInstance was in the zone. This allows for more accurate bounds check (haven't yet noted any obvious performance impact).
  2. in src/frameworks/components/zone/component.js: 296
            // Magnopus Patched, add support for zones over mesh instances
            if (entity.render) {
                const inZone = entity.render.meshInstances.some((mi) => {
                    return this._isPointInZone(mi.aabb.center, position, rotation, _matrix);
                });
                if (inZone) {
                    if (index === -1) {
                        this.entities.push(entity);
                        results.add(entity);
                    }
                } else if (index !== -1) {
                    this.entities.splice(index, 1);
                    resultsToRemove.add(entity);
                }
            } else if (this._isPointInZone(entity.getPosition(), position, rotation, _matrix)) {```

3. It currently fires the "zoneEnter" etc events inside of the loop in check entities. however calling isPointInZone is a destructive operation and so due to some extra zone checks I was doing I was calling it inside of the event handler.  This then overwrote the **_matrix** value that the top loop was depending on causing it to start failing (I was seeing unchanged entities suddenly being added/removed from zones incorrectly).

You can see above, rather than fire the events immediately, I'm adding them to a Set and firing them at the end when the loop has finished.
    for (const entity of results) {
        entity.fire('zoneEnter', this);
        this.fire('entityEnter', entity);
    }
    for (const entity of resultsToRemove) {
        entity.fire('zoneLeave', this);
        this.fire('entityleave', entity);
    }

I'm running this inside my own fork so feel free to disagree or suggest and alternative theres no immediate rush.

Thanks again, I'm very happy with the results so far

@MushAsterion
Copy link
Collaborator Author

Hey,

Thank you for your feedback!

It was intentional to restrict the point check for the position of the entity and may you want to have a mesh away from the entity my suggestion would be to have the entity you look for at the position you want it to be.

Having the possibility to work based on the mesh would however be interesting and it might be relevant to do the same as I did for collisions support, if that something you are interested into having a look please go ahead and contribute to the PR.

Rationale behind the position being a point by default is that if you have a character at the surface of the water, half in water and half out you only want to be in the "underwater" zone if the point is underwater and not any point of the mesh and I think this precision is important to keep.

Finally regarding the time to trigger events was initially set in the loop before the entity was added/removed which I changed few commits ago based on your feedback. I completely agree on this change and just pushed a new commit to reflect it!

@MushAsterion MushAsterion changed the title [BREAKING] Adding back ZoneComponent Adding back ZoneComponent Oct 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request open source contribution Issues related to contributions from people outside the PlayCanvas team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants