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

Combining trigger shapes needs more elaboration #19

Closed
aaronfranke opened this issue Jul 27, 2023 · 10 comments
Closed

Combining trigger shapes needs more elaboration #19

aaronfranke opened this issue Jul 27, 2023 · 10 comments

Comments

@aaronfranke
Copy link
Contributor

aaronfranke commented Jul 27, 2023

https:/eoineoineoin/glTF_Physics/tree/master/extensions/2.0/Khronos/KHR_rigid_bodies#triggers

The spec needs more elaboration on how multiple trigger shapes can be combined into one trigger.

With the solid colliders, the spec is pretty straightforward, any collider on a node with motion uses that motion, or uses an ancestor node's motion, or if there is no ancestor with motion then it will be a static collider.

Here @eoineoineoin gave an example of how to combine 2 triggers together, with a structure like this:

Trigger
    Trigger
[
	{
		"name": "HorizontalPartOfTheL",
		"children": [1],
		"extensions": {
			"MSFT_rigid_bodies": {
				"trigger": { "collider": 0 }
			}
		},
		"name": "Cube"
	},
	{
		"name": "VerticalPartOfTheL",
		"extensions": {
			"MSFT_rigid_bodies": {
				"trigger": { "collider": 0 }
			}
		},
		"rotation": [0, 0.7071067094802856, 0, 0.7071068286895752],
		"translation": [-1.3, 0, -0.7]
	}
]

When importing into Godot, we would have to generate a node structure like this:

Area
    Shape
    Shape

However, I am not sure how I would best export the above node structure out of Godot. Purely based on the example you've given so far, one option would be to make one trigger shape a child of the other. While I could move one of the shapes into the above node, that would be very messy for multiple reasons, since if the child had a transform I would have to alter the transform of the parent to match the child, then alter the other children to take that into account, and it would still alter the behavior of how the parent node moves and rotates since its pivot would change.

I don't see any explicit statement in the MSFT spec of how to combine multiple shapes into one trigger without having the parent be a shape, but from looking at the JSON schema, since MSFT_rigid_bodies.trigger.collider is not required, it seems like a structure like this is allowed, with an empty "trigger": {}:

[
	{
		"children": [1, 2],
		"extensions": {
			"MSFT_rigid_bodies": {
				"trigger": {}
			}
		}
	},
	{
		"extensions": {
			"MSFT_rigid_bodies": {
				"trigger": {
					"collider": 0
				}
			}
		},
		"translation": [1, 2, 3]
	},
	{
		"extensions": {
			"MSFT_rigid_bodies": {
				"trigger": {
					"collider": 0
				}
			}
		},
		"translation": [4, 5, 6]
	}
]

If so, this would be great since it gives me a clear way to export the node structure from Godot while preserving all of the original transforms and even round-trips correctly. It looks conceptually sensible, since it's saying that this node is used as a trigger, but does not have a shape defined on the same node, therefore the shapes used must come from descendant nodes. Please let me know if this is good.

Regardless, the ways in which triggers can be combined should be stated explicitly in the spec.

@eoineoineoin
Copy link
Owner

Looks like something has gone missing from that schema file; thanks. It's invalid and missing at least the extensions and extras, so will have to dig in to figure out what happened.

As for what the structure should look like when reading/writing a glTF file, I think this is open-ended and application-specific. From the perspective of a physics description, the only important thing is marking a particular collider as producing events (whatever what means) - different applications will want to do different things with those events. There's nothing in glTF at all which tells you what to do with such an event or what it does; a particular application is going to have to add some additional extension into glTF to encode the logic and state of the associated event handler - that can be leveraged to import the triggers with some explicit structure, if it matters in that application.

@aaronfranke
Copy link
Contributor Author

aaronfranke commented Aug 17, 2023

@eoineoineoin What you mention in your reply is not relevant to this issue, this isn't about defining application-specific behavior or events. This is about combining multiple trigger shapes into one "compound trigger" / "trigger body". It is imperative that we have a way to define this. For example, take a look at this configuration:

Screenshot 2023-08-17 at 5 13 47 PM

I asked you about how to define an L-shaped trigger volume before, which you replied here about having a trigger shape be a child of another trigger shape. That is fine but it does not solve the problem. In the case above, neither shape's position is aligned with the origin of the "compound trigger" / "trigger body".

One option would be to "bubble up" one of the shapes, but this would require altering the transforms of the nodes, thus altering the pivot and affecting behavior and animations. This is not acceptable, we need a way to specify a parent that has no shape itself but is a "compound trigger" / "trigger body" that uses child shapes.

My proposal is this: Remove "required": ["collider"] from the trigger JSON spec here, allowing "trigger": {} to represent a trigger body. This says that a node is a trigger, but does not have a shape itself, only the children do. This is what I am describing in the OP - please let me know if there is still any confusion. (the schema did not require collider before I opened this issue, and I was asking for excluding it to be explicitly defined as allowed behavior, I did not want you to add it to the list of required properties).

@eoineoineoin
Copy link
Owner

Allowing a node to have a "trigger": {} doesn't actually do anything. A trigger property defines a collision volume, so it doesn't make sense to allow an empty trigger like that.

@aaronfranke
Copy link
Contributor Author

@eoineoineoin How do you suggest we allow defining a "trigger body" that is made of multiple shapes?

@eoineoineoin
Copy link
Owner

We've been through this. It's exactly the same as you make a non-trigger body that is composed of multiple shapes: KhronosGroup/glTF#2258 (comment). The fact that a body might be composed of triggers or "normal" colliders has no bearing on the actual behaviour of the physics.

@aaronfranke
Copy link
Contributor Author

aaronfranke commented Aug 28, 2023

@eoineoineoin We have been through this, but as I explained to you, this does affect the behavior of physics. Not movement of course, the simulation will still run the same. It affects anything that uses the triggers as a trigger. If a body has 2 trigger shapes, how do you determine if those shapes should detect things individually (like a set of signals for each of Shape1 and Shape2) or if those shapes should detect together (one set of signals for both shapes)? What if an object moves from Shape1 to Shape2 but is at no point in neither shape, do we expect a signal for exiting Shape1 and entering Shape2, or do we expect no signal because it's in the same "body"?

@eoineoineoin
Copy link
Owner

That depends on the use-case. Even within a single application, there will be many different use-cases, with different requirements for the signals you get. The physics extension doesn't know anything about those use-cases and the readme is very clear on this:

Describing the precise mechanism by which overlap events are generated and what occurs as a result is beyond the scope of this specification; simulation software will typically output overlap begin/end events as an output from the simulation step, which is hooked into application-specific business logic.

So, the behaviour will need to be determined by the requirements of the individual signal handlers.

@aaronfranke
Copy link
Contributor Author

aaronfranke commented Aug 30, 2023

So, the behaviour will need to be determined by the requirements of the individual signal handlers.

Behavior in terms of what the application does with those signals: Yes.

Behavior of what triggers exist that could emit signals: No, that should be specified in KHR_rigid_bodies.

As the text you've quoted describes, it is expected that triggers can be used for "overlap begin/end events". Without the ability to group two shapes as one trigger, those events would happen in the middle of the compound volume, and they would happen individually for each shape instead of for the whole trigger. Compounding these is in-scope of KHR_rigid_bodies. It's not a part of the details of how those events are "hooked into application-specific business logic".

@aaronfranke
Copy link
Contributor Author

aaronfranke commented Aug 31, 2023

@eoineoineoin: The fact that a body might be composed of triggers or "normal" colliders has no bearing on the actual behaviour of the physics.

To be clear, triggers in general do not directly affect the physics simulation. You can remove all of the trigger shapes from the file and it will have no bearing on the actual behaviour of the physics.

If KHR_rigid_bodies will not support compound triggers, then perhaps KHR_rigid_bodies should just not have triggers at all, and instead leave it to KHR_physics_trigger or similar. Because they don't affect the rigid body simulation.

@eoineoineoin
Copy link
Owner

So, the behaviour will need to be determined by the requirements of the individual signal handlers.

Behavior in terms of what the application does with those signals: Yes.

Behavior of what triggers exist that could emit signals: No, that should be specified in KHR_rigid_bodies.

The behaviour needs to be determined entirely by the signal handlers. It is a mistake to suggest that KHR_rigid_bodies should specify any particular behaviour concerning of merging or dispatching of events. Consider this setup:

image

It's a body with two trigger sensors for the headlights. There's three unique "listeners" attached to this object:

  1. Add a visual highlight to an object which enters either headlight
  2. Turn right if an object enters the left headlight and turn left if an object enters the right headlight
  3. Beep the horn if any object is in either headlight

Your suggestion to add a "grouping" of triggers only allows to distinguish between cases 1 and 2 - it doesn't do anything to address 3. However, now that you're mandating a particular behaviour in KHR_rigid_bodies, you have made it impossible to describe all of those simultaneously. On the other hand (as I'm suggesting,) if the individual "listeners" are allowed to determine what behaviour they require, you can describe this use-case.

If KHR_rigid_bodies will not support compound triggers, then perhaps KHR_rigid_bodies should just not have triggers at all, and instead leave it to KHR_physics_trigger or similar. Because they don't affect the rigid body simulation.

Splitting out this functionality into a separate extension does not solve the problem. It only adds additional complexity and we'll be having exactly the same discussion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants