Skip to content
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

Allow exporting custom resources from/to GDScript, VisualScript, C#, and PluginScript. #48201

Closed

Conversation

willnationsdev
Copy link
Contributor

@willnationsdev willnationsdev commented Apr 26, 2021

Old Content:

This is a partial complete 4.0 port of #44879. It does not include the GDScript changes from that PR. (Edit: it now exports GDScript resources in the new parser, and as far as I can tell, NativeScript and PluginScript have been rendered moot by GDNative Extensions)

The changes have been split up into digestible chunks as separate commits. That should hopefully make it easier to review.

While I've tested it somewhat in 3.2.x, I have not yet had a chance to test this 4.0 port thoroughly.

I believe the most controversial set of changes for which I have received limited feedback is "Make script class editor UX feel like C++ types." I recall I wrote it in to help address related proposals, but I can't quite recall now which ones they were. Stuff about making named scripted types having a more authentic feel. Anyway, whether that commit is included really needs more community feedback.

When a final approved version of the PR is ready, I can squash the commits and amend the message.

Also related is #48056 which has duplicate/similar code changes for some parts of the C# support section from this PR.

Edit: Sorry about the 3D nodes reviewers. Accidentally included a previous commit in one of my rebased pushes. Please ignore > that.

I have saved the old history of changes under a new branch called gdres-all-4.0-v1. From this point forward, since this is the PR most people are already "tracking" for solving this issue, I'm going to use it as a staging ground for merging together any relevant PRs that are separately submitted so that others can more easily grab everything significant with which to test out the functionality.

Child PRs:

Demo Project (Relies on all of the child PRs being present, which they are in this branch's history):
Demo_InspectorResources.zip

Demo Project for C#/Mono (also relies on all child PRs being present):
Demo_InspectorResources_Mono.zip

Bugsquad edit:

@willnationsdev willnationsdev requested review from a team as code owners April 26, 2021 02:06
@willnationsdev willnationsdev changed the title Allow exporting custom resources from/to any scripting language (GDScript, VisualScript, C#, NativeScript, PluginScript) [4.0]. Allow exporting custom resources from/to VisualScript, C#, NativeScript, and PluginScript. Apr 26, 2021
@willnationsdev willnationsdev requested a review from a team as a code owner April 26, 2021 02:15
@willnationsdev willnationsdev force-pushed the gdres-all-4.0 branch 4 times, most recently from c7c8429 to bf1021f Compare April 26, 2021 23:19
@Calinou Calinou added this to the 4.0 milestone Apr 26, 2021
@willnationsdev willnationsdev requested a review from a team as a code owner May 24, 2021 06:44
@willnationsdev
Copy link
Contributor Author

I have now rebased this to adapt to the introduction of EditorResourcePicker and EditorScriptPicker, though the Inspector functionality now needs testing again (I didn't have time to test). I also added the documentation changes that I forward-ported from the 3.x version of this PR.

@willnationsdev willnationsdev force-pushed the gdres-all-4.0 branch 3 times, most recently from 0947701 to 4a1d873 Compare May 24, 2021 19:21
@willnationsdev
Copy link
Contributor Author

Made several changes to the ScriptServer API and the VisualScript script class changes so that StringNames were used in place of String for class name member variables and method parameters. Since these all just get translated into String types in 3.x, it isn't strictly necessary to backport them, though I may do so anyway before the 3.x version is reviewed, just to keep the code more in sync. It's particularly important to get it set up correctly in this PR since String and StringName are different types in the 4.0 scripting API. Naturally, this also updates the XML doc files, so those were affected too.

@RedwanFox
Copy link
Contributor

Will this PR make its way before feature freeze?

@akien-mga
Copy link
Member

akien-mga commented Jul 28, 2022

Yes, there won't be 4.0 beta without this implemented.

Edit: And well now that 4.0 beta 1 has been released some users are prompt at pointing out the failed promise. This feature is still planned for 4.0 and we'll get it implemented one way or another as time and resources permit. Some core changes are needed to facilitate implementing this properly and @reduz will have to work on it when he finds time.

@willnationsdev willnationsdev force-pushed the gdres-all-4.0 branch 3 times, most recently from 68fdd12 to 30274fd Compare July 30, 2022 20:48
@willnationsdev
Copy link
Contributor Author

FYI, C# is temporarily removed from this "all together" build because of various inconsistent compiler errors I was getting that seem to be tied to the Mono code not being up-to-date(?). Just going to stop maintaining the C# support (#63126) until advised otherwise since the .NET 6 version will just convert them into native engine types using GDExtension integrations (i.e. they won't even need "global script class" support).

@willnationsdev
Copy link
Contributor Author

I've just rebased all of the child PRs and re-created this PR, now including the Mono C# support again, so that folks can fully test everything out. Please let me know if you experience any issues.

@antonWetzel
Copy link
Contributor

I would like to test the feature but I get non-trivial merge conflicts, especially with csharp after the merge of dotnet 6. Is there any date this might be possible to merge again?

@CsloudX
Copy link

CsloudX commented Sep 13, 2022

Love this feature so much. @willnationsdev

@willnationsdev
Copy link
Contributor Author

willnationsdev commented Sep 16, 2022

@antonWetzel I have just now rebased this PR. I had to remove the C# functionality once again, cause I'm pretty sure MOST of what I wrote is completely unnecessary now. I would need to actually find time to get working C# code in the latest beta to figure out where things stand, what's missing, and how best to fill in those missing pieces. In the meantime, C# should at least be accessible from GDScript with these changes since C# is now set up as in-engine classes (to my understanding). I just don't know yet if C# already has proper Export attributes to handle exporting user-defined script resources (or lists thereof - I kinda doubt it has support for either).

From akien's comment above though, it sounds as though these child PRs will need another rebasing that take into account whatever changes reduz is gonna make later down the line. In the meantime, I hope people can "test" it adequately in its current state.

@Calinou
Copy link
Member

Calinou commented Oct 2, 2022

@willnationsdev Is this PR superseded by #62413 and others?

@akien-mga
Copy link
Member

@willnationsdev Is this PR superseded by #62413 and others?

Yes it should be.

@Clonkex
Copy link

Clonkex commented Nov 30, 2022

Apologies if this should be obvious, but should I be able to create a custom resource in C# in beta 6? I've gone through all the relevant issues and pull requests I can find but there's too many for me to follow what the current status of this is. I tried extending Resource but the new resource type didn't show up in the editor. Also, when I export a field requiring the custom resource (like [Export] public Shape shape where Shape : Resource) the picker in the editor just shows a giant list of every possible resource instead of only the Shape resource or its subclasses as I would expect.

So, in Godot 4.0 beta 6, is there a way to create a custom resource in C# that will show up in the editor properly?

@raulsntos
Copy link
Member

@Clonkex It's not in C# yet, the relevant PR is #63126

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add first-class custom resource support