search

Delsin-Yu / CSharp-Wrapper-Generator-for-GDExtension

This editor plugin generates C# wrappers for classes from the GDExtension plugins.
MIT License
16 stars 2 forks source link

Thread issue: conflict with the UI Thread? #25

Closed GeorgeS2019 closed 5 months ago

GeorgeS2019 commented 5 months ago

"can only be accessed from either the main thread or a thread group. Use call_deferred() instead."
when setting a Label.Text e.g. lbl_blendshapes

var lbl_blendshapes: Label = $VBoxContainer/Image/Blendshapes lbl_blendshapes.text += "%s: %.2f\n" % [category.category_name, category.score]

https://github.com/j20001970/GDMP-demo/blob/3474994c3a844d935551c9793c8b642868105d2e/project/vision/face_landmarker/FaceLandmarker.gd#L58

The errors raised per loop significantly slowed down the performance.

No such issue when running the GDscript version.

=> It is unclear where and what are the issues. How to separate the wrapper binding threads, It will be interesting to see if the new implementation using Concurrent Dictionary would help

https://github.com/Delsin-Yu/CSharp-Wrapper-Generator-for-GDExtension/blob/6f9e6a3209953368c92df16971f172c6a08a6bc5/addons/cs_wrapper_generator_for_gde/CodeGenerator.cs#L123

Delsin-Yu commented 5 months ago

cc @ZerxZ

ZerxZ commented 5 months ago

Can you provide Minimal reproduction project? So that it can be tested otherwise I don't know what the problem is.

Delsin-Yu commented 5 months ago

If you are having trouble understanding the definition of MRP, this is a quote from the GodotEngine repo.

A small Godot project which reproduces the issue, with no unnecessary files included. Be sure to not include the .godot folder in the archive (but keep project.godot). Having an MRP is very important for contributors to be able to reproduce the bug in the same way that you are experiencing it. When testing a potential fix for the issue, contributors will use the MRP to validate that the fix is working as intended. If the reproduction steps are not project dependent (e.g. the bug is visible in a brand new project), you can write "N/A" in the field. Drag and drop a ZIP archive to upload it (max 10 MB). Do not select another field until the project is done uploading.

GeorgeS2019 commented 5 months ago

@ZerxZ @Delsin-Yu

I am not trying to be difficult here. In fact, I thank both of you for advancing the project.

The only way this project will move faster if more users see the benefits of contributing.

There are now more than 10+ gdextensions now in the project.

It is great to see if the codes generated would have any error with these gdextensions.

However, based on my preliminary experimentation.

The next step is to scan these gdextensions using unit and even better integration tests.

This is, in my view, the fastest way to engage the community is adding tests to this project.

The typical way how Godot community works with Minimum zip project is very hard to track.

The better way is to set up unit tests framework so new users could quickly run these tests to convince ,just like myself, that this project is useful.

I have spoken with others recently. Many remain sceptical to contribute. A few told me it is not worth trying.

suggestion

Is there a way to automate the creation of basic Unit Tests?

Delsin-Yu commented 5 months ago

No, there is no way to automate unit tests. All the unit tests for each of these GDExtensions need to be line-by-line translated from GDScripts into C#. That's why we are not starting it now.

Delsin-Yu commented 5 months ago

Migrating unit tests from GDExtension repos will happen, it's just not now, and we understand the will for you to push the progress, but be patient or, engage the development by opening pull requests, after all, it's an open-sourced project.

Delsin-Yu commented 5 months ago

We are unable to reproduce your issue, and since you are unable to provide an MRP for this, I can only categorize this as a misuse of the wrapper.