jrouwe / JoltPhysics.js

Port of JoltPhysics to JavaScript using emscripten
MIT License
246 stars 19 forks source link

Typescript issue?: BodyCreationSettings alternate constructor using ShapeSettings not accepted #114

Closed DennisSmolek closed 6 months ago

DennisSmolek commented 6 months ago

Jolt.BodyCreationSettings(shapeOrSettings: Jolt.Shape | Jolt.ShapeSettings

In The Jolt Docs it lists both Shape and ShapeSettings as acceptable first arguments.

However Typescript currently only accepts Jolt.Shape as the first argument.

Argument of type 'ShapeSettings' is not assignable to parameter of type 'Shape'.
  Type 'ShapeSettings' is missing the following properties from type 'Shape': GetType, GetSubType, MustBeStatic, GetLocalBounds, and 14 more.ts(2345)
DennisSmolek commented 6 months ago

Oh it looks like this constructor isn't currently exposed: https://github.com/jrouwe/JoltPhysics.js/blob/4833c340e2f325131442e6445dc81723840a1991/JoltJS.idl#L2257

jrouwe commented 6 months ago

Unfortunately we can't have function overloads with the same number of parameters in JS, so you'll have to do:

shapeSettings.Create().Get()

to get a Shape to pass to that constructor

DennisSmolek commented 6 months ago

Unfortunately we can't have function overloads with the same number of parameters in JS,

Oh interesting! Is this a WASM limitation?

jrouwe commented 6 months ago

It is a limitation of the webidl binder, but that may in turn be based on some other limitation, I don't know

DennisSmolek commented 6 months ago

What about the default empty constructor? BodyCreationSettings ()=default

Right now you can't create an empty BodyCreationSettings

LeXXik commented 6 months ago

Can you describe your use case? I personally can't see one where the current initialization method would not suffice.

Edit: It might feel a bit different, if you are coming from Rapier, where you are creating shape and body descriptors separately, and then using those to create actual objects in the world, or creating a body using a shape only (e.g. statics), or creating a body without a shape. However, I don't think it is viable to try to fit Jolt into your current setup that is written for Rapier. You'd benefit more from writing a new one that is fit for Jolt instead, where you cannot create a body without a shape (similar to Ammo, for example).

jrouwe commented 6 months ago

Although I also don't see the direct use case for having this 2nd constructor, WebIDL supports it so it is trivial to add (which I've just done).

DennisSmolek commented 6 months ago

It might feel a bit different, if you are coming from Rapier, where you are creating shape and body descriptors separately, and then using those to create actual objects in the world, or creating a body using a shape only (e.g. statics), or creating a body without a shape. However, I don't think it is viable to try to fit Jolt into your current setup that is written for Rapier. You'd benefit more from writing a new one that is fit for Jolt instead, where you cannot create a body without a shape (similar to Ammo, for example).

I think this may be the core of my original thinking, however I also wanted the JavaScript side of the library to best match the capabilities written in the docs. I understand the IDL limitations you mentioned (and in my research it simply is because that's how the algorithm detects overloads) but the blank constructor is also in the docs.

My potential use case was closer to what you describe, where I have my own functions to create the various components in isolation. But once I got further in the flow of things my original idea wasn't viable with how Jolt operates.

In JavaScript it's more common to have .copy() or .clone() methods that allow me to more easily create objects that are very similar. For example if I create 4 different kinds of bodies I could create one with all the complex settings, then just clone it and put it onto the other 3 regardless of shape.

As I got further I realized Jolt settings classes however don't have this functionality so a simplified settings object isn't as valuable.

LeXXik commented 6 months ago

I think what you are describing is an ECS system. As such, it is still useful to have an object or component that describes a body. You can then .clone() it to quickly generate a new one. It is also useful for setting up defaults, so you don't supply everything every time you want to create a new body. Jolt would still receive all the properties whether you clone or create a new body, but if you are creating them from the component interface, most of the settings become hidden away. I guess what I am trying to say is that the idea is not to copy the actual Jolt body descriptor, but the JS side component that contains all the necessary properties for that descriptor.