-
-
Notifications
You must be signed in to change notification settings - Fork 25
Proposed GodotJS 1.1 / Godot 4.5 inclusions #114
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
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
2772df1
to
4e146db
Compare
4e146db
to
daec040
Compare
…ngesets being merged.
This enables bi-directional transfer of all Godot types between workers and the host environment. The structured clone algorithm uses referential equality of Variant rather than structural equality. This both gives us a performance boost and also ensures that if the same Variant is referred to in nested structures, upon deserialization, these relationships will remain intact. JSWorkerParent.transfer() is now deprecated since postMessage can achieve the same and more. As before, this is still a v8 only feature. I would say this makes workers somewhat LESS experimental, but not yet stable.
Annotating a field with @signal/@ExportSignal leads to a getter of the same name being added to the prototype. This doesn't work as users may expect, since the field initialization then blocks access to the Signal. The recommended approach is therefore to add the annotation on a `declare readonly` field, which does not lead to a field blocking access to the prototype getter. However, it's very easy to forget! Additionally, we're currently using TypeScript's experimentalDecorators functionality, which is no longer on the standards track. Instead we can offer a more reliable API with TC39 Stage 3 decorators, which have been supported since TypeScript 5.0. Consequently, all existing decorators have deprecated (but remain for backwards compabitility). In their place a new API createClassBinder has been introduced. It's slightly more verbose due to limitations of the newer Stage 3 decorators specification. We're required to annotate classes in addition to fields. However, this may actually be beneficial since we can conceivably do away with our regex based exported class detection. Our default tsconfig.json has been updated to turn off legacy decorators, unlocking access to the newer syntax. You cannot use both at once. Both sets of decorators contain warnings if used with in incompatible tsconfig. The new syntax looks something like: ``` const bind = createClassBinder(); @Bind() @bind.tool() export default class Player extends CharacterBody2D { @bind.export.cache() @bind.export.object(SceneSynchronizer) accessor synchronizer!: SceneSynchronizer; @bind.export.cache() @bind.export(Variant.Type.TypeInt) accessor walkSpeed: number = 350; @bind.export.cache() @bind.export(Variant.Type.TypeInt) accessor dashSpeed: number = 1000; @bind.export(Variant.Type.TypeInt) accessor dashCooldownMs: number = 500; @bind.export.enum(Direction) accessor facing = Direction.Down; @bind.export(Variant.Type.TypeInt) accessor useCooldownMs: number = 500; } ``` A few key points: 1. You must create a ClassBinder using createClassBinder(). This is a function and contains properties/APIs on it. You can use any variable name, but the convention I'll be using going forward is to use a variable named `bind`. 2. The decorator APIs are all functions that return a decorator i.e. It's `@bind()` and `@bind.export.cache()` not `@bind` or `@bind.export.cache`. 3. We're using JavaScript's new/upcoming auto-accessor syntax. This is not a requirement for all decorators, but not all decorators (e.g., the new cache() decorator) are supported on fields. 4. The new cache decorator enables caching of variants on the Godot side of the JS <-> Godot bridge. The decorator generates a `set` accessor that updates the cache automatically whenever a value is assigned to the JS property. The purpose of the cache is that is provides a fairly sizeable performance improvement when using Godot's general purpose .get("property_name") method. This is particularly useful if you want to expose data to performance sensitive GDExtensions. 5. The order of decorators matters! Decorators are evaluated "inside->out" and class decorators are evaluated after all property decorators. `bind()` MUST be executed AFTER all other decorators. The `@bind.export.cache()` decorator MUST be evaluated AFTER the property export. Although you can't necessarily tell from this example, these new decorators are strongly typed. If for example you were to apply the root `bind()` decorator to a property, or a cache decorator to a field (rather than a setter/auto-accessor), TypeScript will catch these as type errors.
Much like JS Array tuples, there are a plethora of APIs that are able to mutate the GArray tuple to violate the tuple typing. The intent is that only basic set/get functions (which are type safe for tuples) ought to be used. For now I've opted not to add support for tuples to GArray's proxy() types. Simply because the types involved are already very complicated/slow. Perhaps this will be revisited in future.
a2f60c5
to
bf501c3
Compare
Instead we log an error and prevent the script from being instantiated. We probably shouldn't outright crash due to user error in general. However, beyond that, this is actually an expected state when using the editor. If you wanted to change node type, then without this fix you have a chicken and egg problem between the script and the scene.
Previously, when a JS class was being instantiated for an existing native Godot object (e.g. ResourceLoader.load('...')) the JS instance wasn't actually being bound to the native Godot object until AFTER the JS instance was constructed/initialized. This created a problem in which field/property initializers and the constructor were unable to call any native functions, but could call JS functions. Now, the Godot object is always bound to our instantiated object as part of our Godot native class constructor. Thus, we can now safely call methods during initialization. Importantly, this also fixes TC39 Stage 3 decorator @bind.signal() support for fields.
This doesn't change anything about how strings are used within JavaScript i.e. we're still using JS native string type, not Godot's String. However, Godot's String class has many utility functions, some of them static and some of them as instance methods. These are now all available for consumption in JavaScript. Godot String instance methods are mapped to static methods that take a `target: string` as their first parameter. In general, if there's an equivalent native JS string method, you should always use it instead since it will be much more performant.
Typing these methods was not nearly as straight-forward as I would have liked. There's also a small usage gotcha due to method parameters being bivariant in TypeScript. Even with strict function types enabled, contravariant parameter enforcement only applies when comparing the variance of non-method functions. This might be a bit theoretical, so I'll demonstrate with an example. ```ts class Sound { play() { console.log('Ba-ding!'); } } class Moo extends Sound { moo() { console.log('Moo!'); } } class Animal { vocalize(sound: Sound) { sound.play(); } } class Cow extends Animal { override vocalize(moo: Moo) { moo.moo(); } } function vocalize(animal: Animal, sound: Sound) { animal.vocalize(sound); } vocalize(new Cow(), new Sound()); ``` The above is perfectly valid in TypeScript, no type errors, but will crash at runtime. The issue is TypeScript allows us to override vocalize() and take a covariant (subtype) parameter. So, we proceed to call moo() on what the implementation believes is a Moo, but we end up receiving a Sound instead. This obviously isn't ideal, and the TypeScript language developers are well aware of the situation, but at present this behavior is required to support structural type checking on generics, and to handle some DOM type weirdness. Now if we add: ```ts function callLater<T, S extends keyof T>(delayMs: number, target: T, methodName: S, ...args: T[S] extends (...args: any[]) => any ? Parameters<T[S]> : never) { setTimeout(() => (target[methodName] as (...args: any[]) => any)(...args), delayMs); } class Animal { vocalize(sound: Sound) { sound.play(); } vocalizeLater(sound: Sound) { callLater(1000, this, 'vocalize', sound); // Error on this line } } ``` This gives the error: > Argument of type '[Sound]' is not assignable to parameter of type 'this["vocalize"] extends (...args: any[]) => any ? Parameters<this["vocalize"]> : never'.(2345) Basically, `this` is NOT the same as `Animal`, it's a polymorphic type. Due to the use of `this` the type checker is unable to validate that `[Sound]` is the correct parameter types tuple. This occurs BECAUSE the parameter types are bivariant, the parameter types can be (and in this example are) more restrictive than those declared in the `Animal` type. Now, this poses an interesting problem for Godot's call_deferred (and similar) APIs. Because it's quite common to want to do: ```ts this.callDeferred('remove_child', someChild); ``` Which leads to a similar error with `removeChild` parameters not being known for the `this` type. The solution is to introduce a cast to the same type (or a parent type): ```ts (this as Node).callDeferred('remove_child', someChild); ``` This works around the issue. Of course, this *technically* isn't type-safe if a sub-class was to override `removeChild` similarly to our `vocalize` example above. The cast is basically telling the typechecker, "Go away. I know what I'm doing. Probably." Now that I've explained the usage gotcha, I'll touch on some technical details of the implementation. Using our example above, if you were to try pull `callLater` into the `Animal` class, drop the `T` generic parameter and replace its usages with the type `this`. When you try use the method you'll run into the dreaded: > Type instantiation is excessively deep and possibly infinite. This occurs because `callLater` attempts to handle parameters for all functions on the `Animal` class. But one of those functions is `callLater`. So the parameters for a call to callLater are potentially the parameters for another callLater... you see where this is going. The solution is basically to explicitly prevent recursion through `callLater`. Easy enough for this one example. But Godot has several dynamic dispatch methods, they all need to be excluded, not just the function itself, because you could chain calls back and forth between them. So, this is where the new 'godot' module interface comes in: ```ts /** * Godot has many APIs that are a form of dynamic dispatch, i.e., they take the name of a function or property and * then operate on the value matching the name. TypeScript is powerful enough to allow us to type these APIs. * However, since these APIs can be used to call each other, the type checker can get hung up trying to infinitely * recurse on these types. What follows is an interface with the built-in dynamic dispatch names. GodotJS' types * will not recurse through methods matching these names. If you want to build your own dynamic dispatch APIs, you * can use interface merging to insert additional method names. */ interface GodotDynamicDispatchNames { call: 'call'; callv: 'callv'; call_deferred: 'call_deferred'; add_do_method: 'add_do_method'; add_undo_method: 'add_undo_method'; } ``` An interface isn't actually the most obvious way to define the exclusions, a union would be simpler. However, if you were to add your own dynamic dispatch type method in a sub-class, GodotJS types will need to avoid recursing through it too. So you can use interface merging to add to the set. There's actually a bit more complexity than just calls to those methods. Because TS type checking is structural, without the above you hit up against infinite recursion simply by virtue of the methods existing on the type, even if you're not calling them.
Additionally, fixed some class name mishandling, when the type name mishandling which cause improved codegen for enums when camel-case bindings are enabled.
bf501c3
to
28929a8
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a mammoth
(DRAFT ONLY)PR that includes everything in my personal fork of GodotJS.How much effort I'm going to invest into splitting this up into smaller PRs will depend on whether there's anyone who wants to put up their hand to properly review lots of smaller changesets. Otherwise, I'd appreciate it if people could at least try out a build (grab from Github Actions on commits below) with their project and flag any concerns.
At the very least, none of this will be merged until I've written feature / release notes.Done.