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

Add C# resource export. #63126

Closed

Conversation

willnationsdev
Copy link
Contributor

@willnationsdev willnationsdev commented Jul 17, 2022

Adds support for C# to be able to define global types using a [Global] attribute and export global script class resource types using an [Export] attribute. The [Export] attribute has an optional single-string constructor which allows for you to specify non-C# types to be exported (they must extend Resource), but if you omit it, then it will simply use whatever data type the C# property type is (so the C# property must be a Resource-extending C# class with the [Global] attribute attached to it).

Related Issues:

Related PRs:

Sibling PRs:

Considerations:

  • Everything appears to be working properly.

@willnationsdev
Copy link
Contributor Author

FYI: it has been brought to my attention that there are plans for Godot 4 to rely on .NET 6 and source generators that interact with the new GDExtension system rather than having C# types appear as scripts. If that comes to pass, then merging this PR will be unnecessary as C# types would no longer appear as scripted types, but rather as built-in object types (i.e. GDScript should automatically define native references to their classes just as it does for things like Node or Resource).

@willnationsdev willnationsdev force-pushed the gdres-csharp branch 2 times, most recently from bd2248e to f2704f4 Compare July 30, 2022 20:15
@geowarin
Copy link
Contributor

geowarin commented Jul 30, 2022

I notice that your commit a3cefd8 fixed the mono build for me.

It was probably broken after #63603 and #63595.

Generating the mono glue is however still broken, probably after #63219.

I know the focus of godot 4 is not on C# right now and I fully understand and support this decision.
However, before this, I was able to build the mono version locally so I'm a bit sad it is no longer possible.

Would it be a good idea to keep track of those failures and fixes in a PR or should I give up on mono altogether and wait for the .net 6 work to be merged?

Edit: #63656 and #63661 fixed the mono build and #63537 will fix the mono glue.

@geowarin
Copy link
Contributor

@willnationsdev Mono build is functional again.
Of course it's not for me to tell if you should continue this PR.

But I'm really excited for this, hope this gets merged 😄

@willnationsdev
Copy link
Contributor Author

@geowarin is this really even relevant anymore? I suppose it could be relevant for future 3.x builds, but it wouldn't be for 4.0 even if using Mono, right? I assume the 4.0 Mono would likewise just register types with GDExtensions like the .NET 6 version would.

@geowarin
Copy link
Contributor

geowarin commented Aug 1, 2022

Not sure, maybe @akien-mga or @neikeq can chime in and tell us more.

What I understand is .net 6 is still very much in the works, and won't be available in the next months (not before 4.1).
So in the meantime, mono is still (unofficially) supported for those willing to build the engine themselves and deal with the occasional breakage.

I can see that this PR would turn into throwaway code once .net 6 gets merged. I imagine the whole csharp_script class will.

So it would be used by a very limited set of persons until 4.1 gets there.
On the other hand, it's only a dozen of lines, it's easy to throw away, and some people still use the mono build and would be delighted to have this (wink wink).

Wow this is a very long way of saying "I don't really know", isn't it? 😄

Do you have concerns about this code outside of "this will get deleted soon (tm)"?

@geowarin
Copy link
Contributor

geowarin commented Aug 1, 2022

At the very least I would keep this PR separate from your main PR.

@willnationsdev
Copy link
Contributor Author

@geowarin FYI, I just rebased this PR, so if there is a need to merge it, then it should be available to those that want it.

@dotlogix
Copy link
Contributor

dotlogix commented Oct 7, 2022

@willnationsdev any updates on this? Would be nice to merge it at some point.

@willnationsdev
Copy link
Contributor Author

willnationsdev commented Oct 7, 2022

@dotlogix After the .NET 6 overhaul, everything in this PR doesn't really apply anymore for the most part. I'd have to start from scratch in analyzing/understanding the new codebase. On the plus side, implementing support should be fairly easy / straightforward since it should just involve adding an updated Export attribute constructor (to my inexperienced pov anyway). Since C# is in ClassDB now rather than being a script, it largely should work out of the box.

@dotlogix
Copy link
Contributor

dotlogix commented Oct 7, 2022

It would be very nice to add support for C# because atm it is not really usable out of the box.

For GDScript we now have the ScriptClasses available for selection in the "Add Node" Section automatically for C# this is not possible (I assume) because EditorPlugins don't work anymore if I remember correctly.

Same applies for Resources with GDScript this is very easy and you get a nice filtered view with your exported properties, in C# not possible.

Theming also depends on these types to be available to automatically show up in the Theme editor if you registered them manually in ThemeDB.

@willnationsdev
Copy link
Contributor Author

willnationsdev commented Oct 8, 2022

@dotlogix

It would be very nice to add support for C# because atm it is not really usable out of the box.

For GDScript we now have the ScriptClasses available for selection in the "Add Node" Section automatically for C# this is not possible (I assume) because EditorPlugins don't work anymore if I remember correctly.

I may be wrong since I haven't actually had time to try building Godot 4 at all after .NET 6 support was merged, but if I understand correctly and the .NET 6 support is implemented using source generators to generate binding code that registers C# classes as "extension classes" to the ClassDB using the new GDExtensions API, then all of the C# classes you define that extend a Godot type should automatically show up as built-in classes in the engine, just like Node, Resource, and all of the others. They won't even appear as scripts. And likewise, they shouldn't require any sort of EditorPlugin to call add_custom_type (in fact, I think they should probably deprecate and remove all things related to the legacy "CustomType" API since it's nothing but an editor-only pseudo-shortcut under-the-hood that makes the object and attaches the script to update the scene dock).

Same applies for Resources with GDScript this is very easy and you get a nice filtered view with your exported properties, in C# not possible.

If C# classes are now built-in classes as described, then the only thing you should need to do is define a C# class that extends the Resource class. That alone should make the GDScript parser consider the C# class's name to be a valid identifier for type hints, thereby allowing you to export a property of that type on a node with the GDScript attached to it. Viewing that node in the Inspector should then allow you to create inline instances of the C# resource just as you would any other in-engine resource.

On the C# side of things, I took a peek at the ExportAttribute, and it seems to have a constructor that allows for specifying the property hint and hint string manually, for example:

[Export(PropertyHint.ResourceType, "MyGDScriptResource")] public Resource Data { get; set; }

The only thing I might add is another constructor that assumes PropertyHint.ResourceType if all you do is provide a string, that way you can make it even more concise when exporting data where the actual resource data is based on a script from another a scripting language.

So, in short...

# Define a GDScript Resource type.
class_name GDResource
extends Resource
// Define a C# Resource type.
public CSResource : Godot.Resource {}

// Export properties of either type in a C# node.
public CSNode : Godot.Node
{
    // Should work out-of-the-box, ideally.
    [Godot.Export] public CSResource? CSResource { get; set; }
    
    // Not yet supported, but I might add it.
    [Godot.Export("GDResource")] Godot.Resource? GDResource { get; set; }

    // Should be available as a backup option for now.
    [Godot.Export(Godot.PropertyHint.ResourceType, "GDResource")] Godot.Resource? GDResource2 { get; set; }
}
# Export properties of either type in a GDScript node.
class_name GDNode
extends Node

@export var gd_data: GDResource = null # should definitely work
@export var cs_data: CSResource = null # will PROBABLY work, given my recent updates to GDScript that were merged.

Theming also depends on these types to be available to automatically show up in the Theme editor if you registered them manually in ThemeDB.

Idk anything about the ThemeDB, but I would hazard a guess that if it's defined in the ClassDB, then whatever it is should show up here too.


If any of the explanations or examples above do not match up with the behavior you currently experience, then please make sure another issue is opened for it. As far as I'm concerned, if the above assertions hold true, then this pull request is moot and can be rejected/abandoned. If it doesn't work, then the problem shouldn't be that C# "resources" don't work so much as there is some sort of fundamental problem with the C# classes not showing up in the ClassDB at all, period. Unless, for whatever reason, things like ClassDB::get_class_list() or ClassDB::class_exists() don't evaluate extension classes for some reason (though, that seems like it would defeat the whole point of putting that data into the ClassDB in the first place). Regardless, it should be related to some other actual bug rather than needing to do anything specific to add "resource" support for the C# language.

@raulsntos
Copy link
Member

What @willnationsdev describes is the plan but note that the move to GDExtension hasn't actually happened yet, C# still uses the ScriptLanguage API.

For now, my recommendation would be to use the EditorPlugin.AddCustomType API like in 3.x.

@willnationsdev
Copy link
Contributor Author

@raulsntos Is Godot 4 planning to launch its stable release with C# distributed as a ScriptLanguage? If that's the case, and people still have issues using resources from C#, then I would prefer to try and find a solution for stable that removes any need for CustomTypes, even if I have to find time to do it myself. It was only b/c I thought 4.0 C# Resource support would be a reality that I stepped away from my Godot work to focus on other priorities. I don't want people's first Godot 4.0 C# experience to be tarnished with bad UX.

@dotlogix
Copy link
Contributor

dotlogix commented Oct 8, 2022

@willnationsdev please let me know if there is anything I can do to support you on that topic. Atm I came up with an addon script which does basically what @raulsntos said and register all C# types as CustomTypes automatically.
I don't know much of C++ unfortunately for anything else I would be open to support

@willnationsdev
Copy link
Contributor Author

No worries. I appreciate your support. The things one has to do are...

  1. Add a GlobalAttribute.
  2. Add logic to CSharpScriptLanguage::get_global_class_name(...) that extracts the attribute and, from it, the script class name / icon path.
  3. Ensure that ScriptServer/EditorFileSystem re-scans for global script classes any time the dependent assemblies are rebuilt.

So most of what I'll be doing is trying to figure out how the GDExtension-based ScriptLanguage registration works and where the logic for detecting changes to the .NET assemblies is, that way I can migrate the logic from this PR to the updated .NET implementation.

@raulsntos
Copy link
Member

Is Godot 4 planning to launch its stable release with C# distributed as a ScriptLanguage?

No, the plan is for .NET to move to GDExtension before the stable release.

So most of what I'll be doing is trying to figure out how the GDExtension-based ScriptLanguage registration works.

.NET support is still implemented as a module, not GDExtension, and should be very similar to how it was before .NET 6.0 but some logic that used to be in C++ was moved to C#.

@neikeq
Copy link
Contributor

neikeq commented Oct 9, 2022

C# will support GDExtension in Godot 4.0, but I'm not sure yet whether it will replace scripting or just be another option. There are going to be many of what I consider UX regressions with GDExtensions. Some are fixable, but that may take time. Others are unlikely to change (like lack of method overloads, of which we support to some extent in C# scripts).

If it was my decision, scripting support would stay, but there's a push to leave scripting as a GDScript-only interface and move all the other language bindings to GDExtension, so we will see.

If scripting stays, I will gladly salvage this PR when I have some time.

@willnationsdev
Copy link
Contributor Author

willnationsdev commented Oct 9, 2022

@neikeq Gotcha. So it's really just an "up in the air" uncertainty at this point, to some degree. If that's the case, I'll try to find time to get this PR salvaged in order to save you from having to do it since I know you're busy with a load of other .NET junk. Really appreciate all the work you've done.

On that note, has the documentation for compiling the latest master been updated (didn't appear to have any changes that I saw in the latest docs version)? If not, where can I find accurate engine compilation & project creation instructions for C#?

@dotlogix
Copy link
Contributor

dotlogix commented Oct 9, 2022

@willnationsdev I use this PowerShell Script to build the engine from master atm:

# Build temporary binary
scons -j16 platform=windows target=editor tools=yes module_mono_enabled=yes mono_glue=no

# Generate glue sources
.\bin\godot.windows.opt.tools.x86_64.mono.exe --generate-mono-glue .\modules\mono\glue

# Build C# solutions
python .\modules\mono\build_scripts\build_assemblies.py --godot-output-dir .\bin --push-nupkgs-local ".\nuget"

@neikeq
Copy link
Contributor

neikeq commented Oct 10, 2022

The docs haven't been updated yet. The only instructions are in the module readme.

@Jakob-PB
Copy link

C# will support GDExtension in Godot 4.0, but I'm not sure yet whether it will replace scripting or just be another option. There are going to be many of what I consider UX regressions with GDExtensions. Some are fixable, but that may take time. Others are unlikely to change (like lack of method overloads, of which we support to some extent in C# scripts).

If it was my decision, scripting support would stay, but there's a push to leave scripting as a GDScript-only interface and move all the other language bindings to GDExtension, so we will see.

If scripting stays, I will gladly salvage this PR when I have some time.

@neikeq Wow, this is massive and seemingly signals the end of C# as one of the primary supported scripting languages, and looks like a major blow to C# support in Godot in general. Where is this discussion taking place? Also could you expand upon the UX regressions the move to GDExtensions entails (is there more information about this anywhere)?

Apologies that this is slightly off topic for this PR but I'm not really sure where else to follow up on this.

@neikeq
Copy link
Contributor

neikeq commented Oct 30, 2022

@neikeq Wow, this is massive and seemingly signals the end of C# as one of the primary supported scripting languages, and looks like a major blow to C# support in Godot in general.

No, GDExtension is simply a different system for doing the same thing. Switching away from the scripting system doesn't change the support status of the language, and it will still be included by default.

Where is this discussion taking place? Also could you expand upon the UX regressions the move to GDExtensions entails (is there more information about this anywhere)?

Discussion is on our RocketChat, either in private or the scripting channel. UX regressions could be many things. I don't have a full list, as we will only know when we get user feedback. One example is no go-to-script button in the node tree since there's no file association. That one can likely be solved, although not likely in time for 4.0.

@Clonkex
Copy link

Clonkex commented Nov 30, 2022

C# will support GDExtension in Godot 4.0, but I'm not sure yet whether it will replace scripting or just be another option. There are going to be many of what I consider UX regressions with GDExtensions. Some are fixable, but that may take time. Others are unlikely to change (like lack of method overloads, of which we support to some extent in C# scripts).

Would someone mind explaining what this means exactly? Sorry if I've missed something obvious. I love C# and I'm worried it's being pushed to a second-class language, that's all. Thanks!

@Braveo
Copy link

Braveo commented Jan 2, 2023

Any updates on this? This is something I've been looking forward to since it'd make the workflow a lot more intuitive to work with in C#.

@YuriSizov
Copy link
Contributor

Superseded by #72619.

@tarragonfly
Copy link

tarragonfly commented Feb 8, 2023

C# will support GDExtension in Godot 4.0, but I'm not sure yet whether it will replace scripting or just be another option. There are going to be many of what I consider UX regressions with GDExtensions. Some are fixable, but that may take time. Others are unlikely to change (like lack of method overloads, of which we support to some extent in C# scripts).

If it was my decision, scripting support would stay, but there's a push to leave scripting as a GDScript-only interface and move all the other language bindings to GDExtension, so we will see.

If scripting stays, I will gladly salvage this PR when I have some time.

I hope C# will not be demoted to a second-class citizen. GDScript 2.0 is nice and all but I just can't stomach loss of QoL features I have in IDEs. I don't have the confidence I can scale a project with GDScript past a certain point, but I can with C#.

@Calinou
Copy link
Member

Calinou commented Feb 9, 2023

I hope C# will not be demoted to a second-class citizen.

It's not being demoted to a second-class citizen – quite the opposite, in fact. C# resource support is being worked on for a future 4.x release: #72619

@AThousandShips AThousandShips removed this from the 4.0 milestone Jan 26, 2024
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.

None yet