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 specifying C# class name instead of source file path as node's script #15661

Open
mysticfall opened this issue Jan 13, 2018 · 51 comments
Open

Comments

@mysticfall
Copy link
Contributor

mysticfall commented Jan 13, 2018

Godot version:
master / 30d7943

OS/device including version:
Manjaro Linux 17.1-rc2

Issue description:
Currently, C# is treated in the same manner as GDScript, in that it is assumed the code would be reused in its source code form. However, it is not ideal for C# which is a compiled language and often used to create binary assemblies that can be reused in other programs.

With Godot, it is not possible to attach a C# class to a node directly (by specifying its fully qualified class name), but must be done by providing a path to the source file where the class is declared.

This is problematic, since C# allows declaring a class that has a different name from its source file, or even declaring multiple classes in a single source file.

More importantly, it makes creating a library or a framework that can be used in Godot very difficult, since it is impossible to reuse such a module in its binary form, as it is generally done by a normal C# project (by using NuGet, and etc.).

This limitation also prevents one from writing editor plugins, as described in #15237.

Ideally, users should be able to reference a C# type by its fully qualified name (including the namespace) and attach it to a node, as long as the assembly that contains the class is referenced by the main C# project which is associated with the current Godot project.

@Zylann
Copy link
Contributor

Zylann commented Jan 13, 2018

Same as #7402

@mysticfall
Copy link
Contributor Author

@Zylann Urg... you are right, it seems I somehow missed that one when I searched for a duplicate issue.

Sorry, and you can safely close this issue.

@mysticfall
Copy link
Contributor Author

@Noshyaar Can we reopen this again please? From how the discussion went in #7402, it seems like we need to keep another issue open to discuss the matter specifically from the C#'s side.

@ghost ghost reopened this Mar 10, 2018
@mysticfall
Copy link
Contributor Author

Thanks :)

@reduz
Copy link
Member

reduz commented Mar 10, 2018

Reopened since #7402 diverged into many different issues.

@reduz
Copy link
Member

reduz commented Mar 10, 2018

Regarding assigning classes instead of files:

The rationale of what was discussed is that, in my opinion, assigning classes rather than files as the common case makes usability worse. Most of the time when developing a game, you just create new files together with the class name, and only rarely you assign a script to a node (where it would make the most sense to assign classes to nodes/resources). For this reason, I am against a change like this.

Regarding declaring a class that has a different name than the source file.

I still don't understand why this is a limitation, would like @neikeq to explain better the rationaly for this.

Referencing C# type from fully quaified name

Does not C# have import ? I am not sure if I understand the limitations in regards of this.

@mysticfall
Copy link
Contributor Author

@reduz Thanks for continuing the discussion here.

Unfortunately, I just stumbled upon a much more fundamental problem with the current way of resolving C# classes in Godot.

It seems that Godot does not use qualified names for classes at all, completely ignoring any namespace information.

I was trying to explain the problem of parsing qualified type names out of arbitrary file paths, especially when it involves referencing classes from 3rd party addons. But if namespace is not supported at all, I don't think there's any point in discussing it further before we can decide how to implement namespace support (i.e. just use qualified names in the first place / parse .cs file / parse .csproj files /etc).

@neikeq
Copy link
Contributor

neikeq commented Mar 10, 2018

This issue is not very clear since you seem to include the problem which #17409 is supposed to cover.
I commented there regarding the fully qualified name issue.

I'm gonna comment here regarding attaching a script with a class (that does not necessarily match the file name) to a node. I would like you to clarify first what exactly you need to be able to do though. Is this something required for libraries/frameworks? For what? Or is this just to allow specifying a different class in the file when attaching a script to a node in the editor?

I wanna mention that we already support programmatically creating a CSharpScript with custom class via Ref<CSharpScript> CSharpScript::create_for_managed_type(GDMonoClass *).
This is needing to allow constructing nodes from managed code, e.g.: new Node2D(), new MyCustomNode2D().

@neikeq neikeq added this to the 3.1 milestone Mar 10, 2018
@mysticfall
Copy link
Contributor Author

Yeah, I admit I've caused some confusion by mentioning several different issues here.

The main reason why I think it is better to use a fully qualified name to reference classes is it's more idiosyncratic C#'s way of managing dependencies.

The confusion has arisen partly because I thought the reason Godot's trying to do it in its own way was simply an oversight or a side effect of trying to model C# support after that of GDScript.

In that case, if we 'fix' this issue then it'd either make other issues mentioned here to be irrelevant or much easier to fix. I guess that was how the discussion became such a mess.

Anyway, as it turns out that it was an intended design choice and the core developers seem to have a strong opinion (probably with some good technical reasons that I'm not aware of) for keeping it that way, I won't push it too far, as long as other issues can be fixed without changing it.

I'll just reiterate my points of supporting this change here:

  • C# already has a well established way of managing dependencies and distributing a module (i.e. using NuGet), which is incompatible with the current method used by Godot.
  • Practically every other C# platform or frameworks (including Unity) rely on FQN rather than requiring the whole source bundle when referencing types defined in a module.
  • From the usability point of view, it's also better to use FQN, since C# projects tend to have deeper hierarchies due to the use of a namespace. Even it's not as common to use many complex addons as it is in Unity, we will eventually get there and the usability will become an issue.
  • It's not compatible with any project that relies on source code generation or assembly manipulation (i.e. Cecil / Fody). It's a corner case but still a possibility.
  • It does not support such a case where a source file contains mutiple class definitions. Again, it's a corner case, but I believe it's better not to introduce arbitrary limitations to what is supported by the language itself, if not absolutely necessary.

@neikeq
Copy link
Contributor

neikeq commented Mar 10, 2018

  • C# already has a well established way of managing dependencies and distributing a module (i.e. using NuGet), which is incompatible with the current method used by Godot.

I'm not sure what you mean. What is not compatible with NuGet?

  • Practically every other C# platform or frameworks (including Unity) rely on FQN rather than requiring the whole source bundle when referencing types defined in a module.

Last time I checked, Unity was still using files instead of fully qualified names to attach a MonoBehaviour to a GameObject. This was some time ago, so maybe things changed?

  • It's not compatible with any project that relies on source code generation or assembly manipulation (i.e. Cecil / Fody). It's a corner case but still a possibility.

How exactly is it not compatible?

  • From the usability point of view, it's also better to use FQN, since C# projects tend to have deeper hierarchies due to the use of a namespace. Even it's not as common to use many complex addons as it is in Unity, we will eventually get there and the usability will become an issue.
    <...>
  • It does not support such a case where a source file contains mutiple class definitions. Again, it's a corner case, but I believe it's better not to introduce arbitrary limitations to what is supported by the language itself, if not absolutely necessary.

Like I said previously, technically this is already possible. The problem is how to let the user select a script this way without affecting the usability for selecting scripts from other languages.

I've been thinking about the following solution. What if we make the script editor dialog allow to optionally specifying a fully qualified name rather than a file (if the selected language allows it)? It would be a built-in script, like built-in GDScripts, and it would simply contain the FQN. Would this be enough?

I would like to know @reduz's opinion on this idea.

@mysticfall
Copy link
Contributor Author

mysticfall commented Mar 11, 2018

I'm not sure what you mean. What is not compatible with NuGet?
As you know, managing dependencies in C# involves downloading and adding binary assemblies to your project, which process NuGet automates. Sometimes, source bundle is also provided but it's purely for referencing and debugging purposes, not because it has any runtime impact.

In Godot, we are free to distribute our libraries as NuGet packages like any other C# projects, but as soon as we need to include Node derivatives which users assign to nodes in their project, it's no longer possible to rely on such a method as we'd also need actual source files for them.

To me, having two separate ways of managing dependencies seems to be problematic, especially when the one we are using is not a common practice among other C# projects.

There's an already a well established method of adding assembly references in C# projects, which is covered by many documentations, tutorials, and supported by IDEs and tools like NuGet.

So, an exception like "you can do it like as any other C# projects do, except when you want to include Node type classes, in which case you need to distribute the whole project source bundle instead" feels quite adhoc and confusing to me.

Last time I checked, Unity was still using files instead of fully qualified names to attach a MonoBehaviour to a GameObject. This was some time ago, so maybe things changed?

I believe Unity just uses hashes for script assets internally, but at least on the surface it shows a UI that lists classes with FQNs in a hierarchical manner for users to choose a class to attach.

How exactly is it not compatible?

On a second thought, it seems that Cecil/Fody might not be a problem, as modified methods will be detected by Godot if it uses reflection to query the metadata. Source generation is a different matter but I agree it's not that significant a matter since users can just run the build process once and attach the generated sources to nodes.

I've been thinking about the following solution. What if we make the script editor dialog allow to optionally specifying a fully qualified name rather than a file (if the selected language allows it)? It would be a built-in script, like built-in GDScripts, and it would simply contain the FQN. Would this be enough?

My main concern was to provide a unified way of distributing and managing dependencies, but even if we decide to leave it as it is, I believe it'd be better to provide such a way to reference classes by FQNs.

Unlike what @reduz mentioned in another thread about users very rarely need to assign a class to an existing node, my own experience with Unity differs on that aspect.

There are a lot of 3rd party assets available in Unity, which work by letting users to select and attach components provided the asset to their own game objects.

Even if we don't have that many addons right now, I can easily see how we'll see more framework or template type addons appear for Godot which will provide commonly used scripts for their uses to choose and assign to their own nodes, as we become more popular.

And if we consider scripts contained in a Unity asset usually reside in such a path like Assets/Plugins/Zenject/Source/Install/MonoInstaller.cs, it's not difficult to see why it's much easier to find something by typing a few characters or navigating to Zenject/MonoInstaller in a FQN tree rather than to the actual physical location of the source.

So I believe what @neikeq suggested could be a good way to improve usability for our users in the long run, regardless of whether or not we are going to change the way we assign classes to nodes.

@Zylann
Copy link
Contributor

Zylann commented Mar 11, 2018

FYI, Unity still doesn't see your script if its filename doesn't match class name ;) But it can differentiate two classes of the same name by the file hash (still all file based then).

@mysticfall
Copy link
Contributor Author

@Zylann Yeah, it's quite messy with many exception cases there. Unity even fails to recognize a script when it contains a static import statement, for instance :)

@neikeq
Copy link
Contributor

neikeq commented Mar 11, 2018

I believe Unity just uses hashes for script assets internally, but at least on the surface it shows a UI that lists classes with FQNs in a hierarchical manner for users to choose a class to attach.

The way Unity does it is very similar to how we do it in Godot, unless something changed. They assing a MonoBehaviour via files. I guess they then parse the file to find the FQN, which is what we are missing.

My main concern was to provide a unified way of distributing and managing dependencies

I don't understand what you mean by this now.

@mysticfall
Copy link
Contributor Author

mysticfall commented Mar 11, 2018

I'm not sure if it was my lack of experience in C# or of my writing skills that makes it difficult for me to explain the problem clearly. But either way, please let me know if I'm wrong in any specific points.

As you know, assemblies are the basic building blocks for all .NET applications and components, so managing dependencies in a typical C# project is normally done at the assembly(.dll) level, not by manually downloading the whole project and referencing individual source files.

If I want to write an open source framework or a library in C#, I'd simply set up my CI to build and publish a NuGet package, which contains all the managed assembly dlls.

If anyone wants to use my library, one would simply need to add the assembly name as a reference to one's .csproj file and everything will be ready.

So, if I want to make a Godot addon, I also want to follow such a standard workflow to publish it, and expect users to do the same to use it.

But currently, such a workflow is only partially supported because if I want to let users to reference any node types defined in my addon within the editor, just providing a dll assembly wouldn't be enough.

In that case, I need to tell them to download the whole project tree and extract it where they can navigate easily inside the editor, and instruct them to go to such and such directory to fine 'script' files.

I don't have to clone the whole repository and deal with individual source files to use JSON.NET or Microsoft.Extensions.Logging, for examlple, and I don't have to manually track down what other components such libraries might depend on and do the same things for each of them, since the standard dependency management mechanism is well defined and supported by popular development tools.

If I want to use .NET's configuration API in my project, all I need to do is add a reference to Microsoft.Extensions.Configuration.FileExtensions to the .csproj and rebuild it.

I don't have to search for the website, and download the project bundle, and figure out it also depends on 2.0+ version of Microsoft.Extensions.FileProviders which I also need to download and extract.

Also, I don't need to know where exactly is the source file for FileConfigurationProvider.cs located, since I normally learn how to use such a library by looking at its API reference, not how its project source is structured.

I can understand if there are some good technical reasons why we have such a requirement (other than it works for GDScript) in place, or it might not cause too much trouble for average users.

But still, I would want to keep the development workflow in Godot as closely as the one acknowledged to be the standard or best practice for each binding languages if possible, rather than forcing every languages to conform to the Godot/GDScript way of doing things.

@reduz
Copy link
Member

reduz commented Mar 11, 2018 via email

@mysticfall
Copy link
Contributor Author

mysticfall commented Mar 11, 2018

Aside from solving this by way of custom types, I'm a bit confused as to the purpose of .csc resources.

If what they do is providing an information about what classes an assembly provides or what other assemblies (dlls) it might depend on, don't we already have everything we need?

If I'm not mistaken, we already use MonoImage / MonoAssembly to resolve class names to their actual class instances for the main project assembly. They contain all the information regarding what types are declared in the assembly, or what external assemblies it might depend on.

In other words, we have most the things needed to make binary C# addons to work in Godot already, except that we just don't know what class name to pass to mono_class_from without parsing either the file path or source itself because we don't use FQN.

Actually, it was one of the reason why I initially thought it wouldn't be much of an issue to change it.

@reduz
Copy link
Member

reduz commented Mar 11, 2018 via email

@Zylann
Copy link
Contributor

Zylann commented Mar 11, 2018

I was about to post a huge brick of text examinating the ability to index resources by human-writable identifier instead of file path... but I'll write the conclusion I ended up with, for the C# case:

To be able to assign by FQN without relying on .csc, you need to load ALL scripts of the project first. Walk the whole filesystem and index all enclosing resources (here DLLs). That means the engine needs a special case for resource containers supporting FQN (which GDScripts don't need since they rely on the filesystem, unlike C#, though they have a related problem with inner classes), which brings us to a problem with referencing assets themselves. You would need to append the FQN to the path of the DLL, otherwise scenes and GDScript won't be able to load C# scripts.

@mysticfall
Copy link
Contributor Author

mysticfall commented Mar 11, 2018

@reduz I assume it's a way to register custom node types defined in a managed assembly, then?

@Zylann But doesn't MonoImage/MonoAssembly already contain all the type information defined in a project, which can be easily accessed (i.e. mono_image_get_table_rows)?

Anyway, it seems like I'm the minority in thinking it'd be better to change this behavior, so I won't push my points further.

I'm not still convinced about the reason why we shouldn't change it though, but I believe it'd be better to work on more important things if not enough number of people think this to be an issue.

Before I go, I just like to say that I'm glad I have at least a chance to persuade those who have actual power to change the things in such a prolonged discussion here - it was not something I used to see when I used another commercial product.

Thanks!

@akien-mga akien-mga added this to the 3.2 milestone Jan 9, 2019
@JeroMiya
Copy link

JeroMiya commented Mar 5, 2019

Just going to chime in here to add some perspective from a C# developer of over 15 years (outside of game engines at least).

If I were to write C# "scripting" support into a game engine now, I would do it this way (I've written something similar for a non game engine):

  • All behaviors/effects/script extensibility points are attached to the scene or nodes or whatever else they can be attached to - based on references to TYPES (including function types), not FILES.
  • All scripts are discoverable via .net reflection. A script list/search UI tool window lists all available script types in a searchable list automatically filtered based on the context and the metadata on the script types/functions themselves, with filtering options based on namespace and/or assembly.
  • Support script "bookmarks" and "Commonly Used in Project" lists in the search window.
  • Loose .cs files in the project structure are automatically assembled into a ".Net Standard 2.0" assembly. This csproj is generated and built using the standard dotnet CLI toolchain, not custom project generation or a custom build system. Thus, less code I have to write, and stays up to date.
  • Native support for NuGet dependencies and direct csproj references. That's: common dependencies (all platforms), and platform specific dependencies. Both are optional.
  • There is no difference as far as the engine is concerned between scripts in the project itself or from dependencies. They're handled the same way.
  • For those that like the old file-based way, you can still support pretty much the same UI. If you drag a .cs file onto a Node, and that .cs file has only one script type defined in it, the UI would just fill in the type reference for you.

This is just the right way to do it, in my opinion. Simple. Very little custom project generation code to maintain. 100% full native .NET support without artificial limitations imposed by file based scripting. Full access to the entire .Net ecosystem. Third party developers can distribute Godot script libraries on NuGet. No copying .cs files from a zip you downloaded on github - if the developer posts an update to the library you can update that dependency's version and get it, super easy.

In summary:

  • No need to write engine code for things that are already implemented and supported by the .Net Foundation (dependency management, build system, project generation, and so on).
  • Full .Net ecosystem support - all of NuGet dependencies, user projects. No artificial limitations.
  • Still super easy for users to just get started and go for basic scripting. The steps to attach scripts in the project to nodes can be identical to file based scripting, in most cases. Just drag .cs files over nodes like you used to. Only difference is the engine fills in a type reference instead of a file reference. Existing projects can be converted to use type references fairly easily.

@shartte
Copy link
Contributor

shartte commented Mar 15, 2019

Would this be solvable with custom scheme support when loading resources?
We have res:// user:// local:// right now, if a resource loader could handle custom schemes (i.e. mono://, gdnative://), and a MonoScriptLoader would handle mono://<assembly>/<class> URLs, wouldn't this make it easier to use for both native/mono and any other languages that are not really file-based?

Edit: As JeroMiya says below, <class> needs to be fully qualified. If <assembly> is just the project assembly, It'd be possible to omit <assembly> since class names cannot contain slashes in .NET.

@JeroMiya
Copy link

I would maybe use "clr" or "dotnetclr" instead of "mono" for the custom scheme name but otherwise this seems like a plausible. would be the fully qualified class name with namespace.

@neikeq
Copy link
Contributor

neikeq commented Mar 15, 2019

A ResourceFormatLoader for paths that begin with something like clr:// or mono:// sounds like a great idea.
However, I don't know if it could cause problems if the path is to be used for other things. For example, when running the editor or editor player, ResourceLoader::load will try to get the file modified time after loading the resource. This will definitely result in an error being printed. There may be other similar cases.
res://, user:// and local:// schemes are hard-coded. There is no way of adding our own for these things unless we also hard-coded it everywhere. Perhaps there is a way to remap or redirect it to something else.

@neikeq
Copy link
Contributor

neikeq commented Mar 15, 2019

Forget what I just said. ResourceFormatLoader can only be made to handle file extensions. So that won't work.

@Zylann
Copy link
Contributor

Zylann commented Mar 15, 2019

I thought about alternate resource path formats too when thinking about GUID identification for resources, and I thought this could just be a different way of specifying a resource locator. Each GUID would be associated with a path, which might vary if the file is moved, but that mapping allows to still load resources in the end because they are files. For C# classes, that means there would be a similar translation from class name to file.
And in fact, that already happens for the purpose of remapping import paths.

@shartte
Copy link
Contributor

shartte commented Mar 15, 2019

Forget what I just said. ResourceFormatLoader can only be made to handle file extensions. So that won't work.

It can if you override recognize_path. I have a working prototype that executes scripts on the command line using -s mono://MyToolScript. I'll polish it further.
The changes to ResourceLoader::load are minimal and hopefully acceptable to reduz. They relate to not trying to simplify/localize the path if it starts with a custom scheme, and not trying to set the timestamp :)

edit: And yeah, I also think the editor will be the most interesting part. I'll try how it behaves with such a custom scheme today.

@JeroMiya
Copy link

Could we make the class reference a separate field from the current file path? Given that the primary benefit of this proposal is from adding the ability to reference scripts defined in prebuilt assemblies (perhaps from NuGet, perhaps with their own dependencies), maybe we just add the ability to do that and keep the current behavior the same for loose C# scripts in the project?

@shartte
Copy link
Contributor

shartte commented Mar 16, 2019

Hrmpf, while using mono:// works fine at runtime, the editor doesn't work nicely with it when trying to edit such a script. To have editing capability for scripts addressed as mono://... the editor would probably need bigger changes. :-|

p.s.: The bare minimum to get mono:// working is here:
master...shartte:mono-protocol

@neikeq
Copy link
Contributor

neikeq commented Mar 17, 2019

What do you mean? What exactly doesn't work well?

@shartte
Copy link
Contributor

shartte commented Mar 17, 2019

What do you mean? What exactly doesn't work well?

  1. Opening Scripts from the File System
    Double-clicking .cs files in the file system will try to edit a resource with a res:// URL. To get any form of syntax highlighting (which seems very limited anyway), it'll need to be associated with the ScriptingLanguage. This makes it seemingly necessary to maintain the CSharpScript loader for res:// URLs, but that means you'll have CSharpScripts that came from .cs files as well as from mono:// URLs.

  2. Script/Node Assignment
    Assigning Scripts loaded from mono:// URLs to Nodes seems to work without changes. Since the Resource Path will be saved to the scene, this works exactly as it should, since it'll save the fully qualified class-name to the scene (mono://<classname>). But as long as you have CSharpScript resources that are loaded both from filesystem files as well as classes, you'll run into the same CSharpScript essentially being loaded from two different locations at the same time.

  3. Editing Scripts
    When clicking edit script on a node, it'll try to edit the attached script resource directly. It the script uses a mono:// URL, the mono module could still load the associated source code (which kinda works if it's in the project managed by Godot) and it should be shown in the editor. Saving it however will not work, since the script editor has a check that will display a "Save as" dialog in case the script's path is not a local path.

  4. Creating new Scripts / Assigning Scripts
    Since the dialog is also class-based, and assumes the path of the created source file equals the path of the script resource, this'll also not work out of the box.

All in all the assumed equivalence of Script == Resource == Source Code File makes this hard to get right.

@JeroMiya
Copy link

Based on the issues brought up so far, it seems a hybrid solution might be the most effective. So, given these two use cases:

  1. C# scripts as loose C# files in the project itself (as they exist today).
  2. C# scripts from pre-built .NET assemblies (not possible today).

I feel use case 1 is already well covered by the current file based system, i.e. with res: paths. Can we support use case 2 without breaking use case 1 or refactoring a lot of its related code in the engine?

I think so. First, let's just keep use case 1 the same as it is today. That is, for loose C# scripts in the project, continue referring to them by file paths and not class names. Node/script assignment should remain unchanged.

For scenario 2, add support for dotnet://<Namespace.ClassName> (if assembly dependencies are handled at the project level, else include a path to the assembly too). For dotnet:// resources, just disable the edit script button. Presume that the script is part of a library (perhaps from a third party) for which the source is not available to edit. Instead of dragging a script file on to the node, have a search UI that lists any script classes from assemblies referenced at the project level. Picking a script from this list fills in the dotnet:// reference on the node.

@shartte
Copy link
Contributor

shartte commented Mar 17, 2019

Going down that route will essentially create two entirely separate mono integrations in Godot, however. And you'd still need Editor enhancements for the class-assignment you mentioned.
The other option is to remove the assumption that Script == Source Code and actually let the Editor delegate finding the source code for a script (and vice-versa) to the Scripting Language, as well as possibly introducing a "Script Source Code" resource type for scripting languages that actually use that distinction (GDscript doesn't, for example).
It's a bigger change, and most likely something for 4.0 if it is even something that's desired.
There's ofcourse the nuclear option of not having special .cs file editing support in the Godot editor, and only better supporting external editors.

@Zylann
Copy link
Contributor

Zylann commented Mar 17, 2019

It seems globally named classes added in 3.1 could be an alternative starting point. The difference is, they would work for every kind of script, not just C#.

If not, it would be nice for modules to be able to register custom resource addressing systems, so resource locators (i.e "mono://xxx" or "whatever://thing") can be mapped to files without having to modify the core for each particular type of resource.

@shartte
Copy link
Contributor

shartte commented Mar 17, 2019

I looked into the global classes, but the problem - again - is that they are file based :-)
You'll find code like this:
ResourceLoader::load(ScriptServer::get_global_class_path(p_class), "Script");
But even worse, the EditorFileSystem is responsible for finding global classes and it does so by scanning the file system. See EditorFileSystem::_scan_script_classes.

@Zylann
Copy link
Contributor

Zylann commented Mar 17, 2019

@shartte unfortunately the resource==file mapping is assumed deep into Godot, even GDNative has to have pseudo-script files to map to the actual classes defined in libraries. But until that changes, that should not prevent class-based assignment as long as a file exists to match this design (could even be generated, in .import folder maybe?), which then makes it easier to implement custom locators managed by their own module.
Note: here, by module I mean C++ Godot module, not dll or whatever asset ;)

@JeroMiya
Copy link

I'm not familiar with the details of extensibility functionality in various .NET IDEs (VS, VSCode, VSMac, Ryder) to know if they'd support "GoToDefinition(fullClassName)" from an external application. Plus, for third party dependencies, e.g. from NuGet, there is no source to open. That's why I suggested just disabling "Edit Script" functionality for any scripts that come from a "module", for whatever a module means in a given scripting system (.DLL assembly for .NET, something else for other languages).

I suggested handling it as a separate .NET integration only because I thought it would be less disruptive than replacing the current integration with something that handled scripts from C# files and scripts from .NET assemblies in the same way. If that's not the case, then, by all means replace away.

@shartte
Copy link
Contributor

shartte commented Mar 17, 2019

@Zylann It would certainly be possible to do "Assembly scanning" and generate such files in the .import folder. However, to enable things like new MyCustomNode from within .NET to correctly associate a script with the native Godot object that is being created behind the scenes, the mapping between a .NET class and such a mapping-file needs to be 1:1 and navigable in both directions. (Class -> File, File -> Class). And that makes it, well, complicated :-)

@bfoo75
Copy link

bfoo75 commented Feb 2, 2020

Thought I would add some feedback - Sorry for the seemingly newby github account (It's my personal account) - But I've worked a number of mid to large size game studios in the last 15 years of my career. I've got about 6 years of development experience with Unity at Black Bird Interactive (we put out Homeworld: Deserts of Kharak and another internal project developed with Unity). I can say that Unity in fact DOES NOT use filenames to attach classes to game objects. They use a GUID keyed to the assembly qualified name. As far as I understand it, loose scripts are either compiled into a temporary dll or dynamically compiled during runtime... but they are compiled before use and the script/game object hooks are indeed the logical class, not the source.

This means that if you define a unity game object in an external assembly, then drop that dll anywhere under the Assets/Code folder, it'll automatically load the dll and it's classes as available game object scripts. We in fact had paid for development support from Unity themselves and they confirmed that this method of importing assemblies was the preferred workflow. It worked well for build systems & various source control models.

I'm currently doing some consultation work and I can say that this alone makes me want to move back to unity, which is a shame because a lot of the other features are very attractive in comparison.

I think the issue is that using a file path to indicate a class declaration places a dependency on the projects code organization. By using filepaths, I no longer have the freedom to define classes or nodes under a companies guidelines and managing large scale projects where all the source is contained in 1 project is not a particularly scaleable solution. If I break the projects up, I may end up with extremely long path names and a lot of noise in terms of files that are likely not important to my task.

As an alternative - using either roslyn or reflection to get nodes defined in all loaded assemblies and then presenting the available classes through the same window as the 'Inherits' selection would be ideal. As a constraint, you could also only show extended classes that are derived from existing Godot base types. I believe the reflection method would provide everything you need without adding anything to the existing project - but that is under the assumption that you have a c# runtime somewhere between the population of the window and the loading of the external assemblies.

I hope this adds some perspective wrt knowing what could be influencing your users' decisions.

@cgbeutler
Copy link

cgbeutler commented Jul 6, 2022

Maybe this is the wrong proposal, but I think this proposal would also allow for generic nodes and resources, which are currently kinda broken.
The only way I've found to make them work is to explicitly declare each derived type of a generic class, which is tedious and needlessly verbose.

Example:

Generic base class

// Generic display
public class ItemDisplay<TItem> : Control
  where TITem : Resource, IItem
{ ... }

PrimaryDisplayItem.cs

// In a separate file, you have to declare each derived type explicitly, or exports of it will break.
public class PrimaryItemDisplay : ItemDisplay<PrimaryItem> {}

... Etc. for each item type resulting in a bunch of 1-liner files.

This is particularly annoying when working with Resources, as generics could really simplify certain data structures or layouts.

I support the core dev's view that TRES files should be human-readable. I LOVE that you can currently write one by hand with very little issue. That said, it would be nice if certain cases could use a fully-qualified name, when a path will not do.

I also get that generics don't make sense from the editor side. You can't drag in a generic script, because there's no way to know what types to specify. This makes generics nodes and resources only usable from code. A big limitation, but it would still be nice to have as an option.

Note: Another solution to generics could be godotengine/godot-proposals#307 (so upvote that one if you like that solution more.)

@Pedro-0550
Copy link

Any updates on this?

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

No branches or pull requests