goostengine / goost

A general-purpose, extensible and customizable C++ extension for Godot Engine.
https://goostengine.github.io/
MIT License
481 stars 18 forks source link

`Internal Script Error! - opcode #13 (report please).` when specifying variable type #130

Open opyate opened 3 years ago

opyate commented 3 years ago

Goost and Godot version:

Godot_v3.3.2-stable.goost_v1.0-stable_osx.universal

Issue description:

I'm typing var pb below with PolyBoolean2D, which causes an error. If I remove the type, all is fine. Am I doing something wrongly?

extends Node2D

# this works
var pb

# this causes the error
# var pb: PolyBoolean2D

# Godot built-in types work:
var t: Tween

func _ready():
    pb = PolyBoolean2D.new_instance()
    pb.parameters.strictly_simple = true
    t = Tween.new()

Running this with the offending line uncommented triggers the debugger on line 1.

Further output:

Screenshot 2021-09-20 at 21 04 37

Further errors:

Screenshot 2021-09-20 at 21 05 24

Steps to reproduce:

On Mac, have a simple node, with the above content in a script, then run.

Is this issue reproducible in official Godot builds?

No, as it uses the Goost API, and won't be available.

Minimal reproduction project:

godot-goost-bugreport.zip

Xrayez commented 3 years ago

Introduction: originally, classes like PolyBoolean2D in Goost were singletons only. The problem is that embedding parameters like strictly_simple to each and every method in PolyBoolean2D resulted in API bloat (each method would have to repeat the same 3-5 parameters). So I've decided to split the interface into classes like PolyBooleanParamters2D.

Now, the problem is that overriding those parameters in a singleton would lead to hard to debug errors due to the fact that the same parameters would be shared across project. Therefore, so I've made it possible to create a new local instance with new_instance() method from a "singleton", so that those parameters could be configured freely without affecting the singleton's global state.

The same would likely apply to Random, Random2D, PolyOffset2D, PolyDecom2D classes, because the same mechanism is used there.


According to GDScript source, opcode 13 means OPCODE_ASSIGN_TYPED_NATIVE. Apparently, GDScript fails to recognize that PolyBoolean2D is not just a class, but a global instance of the class. You can reproduce the same issue with this kind of snippet:

extends Node2D

var geometry: Geometry  # Geometry is a Godot class.

So, I'd say the issue is two-sided: it's probably something which can/should be fixed in Godot's GDScript implementation, or we could find a way to workaround this issue. But mostly, I haven't seen much limitations with current approach, and I'm really not sure if there's a better way to fix this other than removing all those classes to be registered as singletons, and that would be unfortunate because it would mean that classes like Random won't be able to be used globally, which would result in a major convenience/usability issue compared to what we have here.

I've seen several Godot proposals that suggest to add static methods support for classes, so perhaps that would be the way to fix this issue in Godot 4.x. Some of those classes shouldn't be "singletons", because it's possible to create several local instances of the same class. But there's just no other way to use those methods without singletons in Godot.

So I guess the best workaround is: don't annotate variables with types that are treated as global instances rather than classes.

If you print(PolyBoolean2D), you'll actually see that it's just an instance rather than a class. This is the reason why you cannot instantiate a new class with new(), but new_instance() which was added specifically to workaround this.

You could even nuke those instances with free() (if they inherit from Object directly):

image

So, this is just to demonstrate that Godot has quite a bunch of issues revolving around singleton usage already. Hopefully all of these issues could be fixed in Godot 4.x.

opyate commented 3 years ago

Thanks, @Xrayez . If this is a bug with Godot/GDScript and not Goost, then I'm happy for you to close it.

Xrayez commented 3 years ago

Thanks, @Xrayez . If this is a bug with Godot/GDScript and not Goost, then I'm happy for you to close it.

No need, lets keep this issue open so that Godot devs know there's a problem to resolve. 🙂

I reported an issue to Godot as well: godotengine/godot#52893.