BabylonJS / havok

The Havok Physics plugin runtime files (wasm and js)
16 stars 4 forks source link

Add runtime implementation for havok enums #15

Closed koteelok closed 7 months ago

koteelok commented 7 months ago

Fixes https://github.com/BabylonJS/havok/issues/12#issue-1995846366

eoineoineoin commented 7 months ago

I have a concern that these changes aren't very robust - we use emscripten to compile the C++ code to WASM, and these files are emitted by the linker, so the changes are liable to get lost every time we build the plugin. Maybe there's a way to get emscripten to generate these, but I'm not seeing anything in the docs.

koteelok commented 7 months ago

If you don't want to add them manually, you can write a script that parses d.ts files for enums and adds them to js files. I checked the emscripten docs and they don't have a solution for this, at least it's not mentioned in the docs.

RaananW commented 7 months ago

Before we do anything here, I commented in the issue - https://github.com/BabylonJS/havok/issues/12#issuecomment-2017891225

Trying to understand what exactly the issue is. I see that the enum definition are missing on the Module level, but is it there when the module was initialized? Is it typing or js issue?

koteelok commented 7 months ago

Before we do anything here, I commented in the issue - #12 (comment)

Trying to understand what exactly the issue is. I see that the enum definition are missing on the Module level, but is it there when the module was initialized? Is it typing or js issue?

The problem is that enums are in definition files (.d.ts), which means they are not compiled into js on the build step. This means that there are no enums at runtime.

So when you try to import enums, you won't get an error from the TypeScript LSP, you'll get a runtime error because you're addressing stuff that isn't there.

RaananW commented 7 months ago

@eoineoineoin let's discuss this offline and see if it can be exported directly

koteelok commented 7 months ago

@eoineoineoin let's discuss this offline and see if it can be exported directly

Ok, I found another problem with my pr, the thing is that Typescript enums usually start with 0, but Havok enums (it seems) start with 1.

Another option is not to export enums globally (remove export keywords from enum definitions) and just add definitions for enum getters in the HavokPhysicsWithBindings object.

koteelok commented 7 months ago
export interface HavokPhysicsWithBindings extends EmscriptenModule {
    Result: typeof Result;
    ShapeType: typeof ShapeType;
    MotionType: typeof MotionType;
    EventType: typeof EventType;
    ConstraintMotorType: typeof ConstraintMotorType;
    ConstraintAxisLimitMode: typeof ConstraintAxisLimitMode;
    ConstraintAxis: typeof ConstraintAxis;
    MaterialCombine: typeof MaterialCombine;
    ActivationState: typeof ActivationState;
    ActivationControl: typeof ActivationControl;
}

Something like this.

koteelok commented 7 months ago

guys? 🥺

RaananW commented 7 months ago

guys? 🥺

Hey, I understand you want a solution, and I can say - we are on it. We want to find the cause, not patch something that will break next time we build. The second solution is probably the right way to go, but I will first have to confirm with @eoineoineoin that these enums are exported (js-level).

RaananW commented 7 months ago

As the enums exist on a constructed module, the types need to move in the declaration file (and not exported from the base level). This would be the right solution for the issue. Roughly your last suggestion, @koteelok . I'll wait for @eoineoineoin to validate that.

koteelok commented 7 months ago

guys? 🥺

Hey, I understand you want a solution, and I can say - we are on it. We want to find the cause, not patch something that will break next time we build. The second solution is probably the right way to go, but I will first have to confirm with @eoineoineoin that these enums are exported (js-level).

As far as I can tell, these enums are exported because they are used in babylonjs/core. More to the point, you can see these enums in the chrome devtools when logging havok bindings.

koteelok commented 7 months ago

As the enums exist on a constructed module, the types need to move in the declaration file (and not exported from the base level). This would be the right solution for the issue. Roughly your last suggestion, @koteelok . I'll wait for @eoineoineoin to validate that.

No need to hurry, I'm using pnpm patch functionality to fix that on my side.

RaananW commented 7 months ago

Fixed in #18

RaananW commented 7 months ago

Solved in #18