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

ResourceSaver serializes resources defined in C# as CSharpScript instead of the specific resource type #38191

Closed
wscalf opened this issue Apr 25, 2020 · 17 comments

Comments

@wscalf
Copy link
Contributor

wscalf commented Apr 25, 2020

Godot version:
3.2 (Mono) initially, but it repro'd in 3.2.1 (Mono) as well
OS/device including version:
Pop! OS 19.10 (kernel 5.3.0-22-generic)
Issue description:
UPDATE: upon closer inspection, it looks like ResourceSaver works with resources defined in gdscript (which serialize as expected) but not C#/Mono (which serialize as a CSharpScript resource rather than whatever they are)

UPDATE 2: I now have a much simpler repro project that doesn't mess with any plugins or anything. It just tries to save two equivalent models, one made with GDScript and one made with C#, and gets very the same different results as the more complex project. It's available here: https://gitlab.com/wscalf/evensimplerresourcesaverrepro

I'm working on an asset pipeline to take in source files from a 3rd party editor, build them, store them as imported assets, and then use them as resources at runtime. Every part of the process worked surprisingly well (props on both being a masterclass in modular design and having great guides and documentation) except, for reasons I don't understand, the ResourceSaver isn't saving my resource correctly from my EditorImportPlugin. interestingly, creating and saving it through the inspector does, so I don't think it's a problem with the resource itself, but I'm not sure what else I could do with the ResourceSaver to get the right behavior. For comparison, the inspector produces this:

[gd_resource type="Resource" load_steps=2 format=2]

[ext_resource path="res://ReverseText.cs" type="Script" id=1]

[resource]
script = ExtResource( 1 )
Text = "Text entered through inspector"

..which clearly defines the resource, sets a property, and associates it with the resource script needed to load it into the game. But, when I create an object of the same type in code and pass it to the ResourceSaver, I get this:

[gd_resource type="Resource" load_steps=2 format=2]

[sub_resource type="CSharpScript" id=1]

[resource]
script = SubResource( 1 )

..which I think says that Text.txt is a C# script, which simply isn't so.

I have a workaround - currently, I'm taking the good tres file from above and using it as a template to generate a valid one from my EditorImportPlugin instead of going through ResourceSaver, and that has the whole pipeline working, but I'd really like to be able to use the ResourceSaver instead (for future-proofing, to be able to use compressed binary files, etc.)

Also possibly related to #20496
Steps to reproduce:

  • Create a custom resource type
  • Create an editor import plugin for said resource type that uses ResourceSaver to store the processed files
  • Try to use an imported resource

Minimal reproduction project:
I made the simplest one I could using a custom resource that represents a reversed string and an editor import plugin that reads and reverses text files. There are two labels on the screen, one that's using a ReverseText resource I manually created through the inspector (which, ironically, means it's not reversed) which saves and loads correctly, and one created by the import plugin which fails to load with the error:

E 0:00:00.700   can_instance: Cannot instance script because the class 'Text' could not be found. Script: 'res://Text.txt::1'.
  <C++ Error>   Method failed. Returning: __null
  <C++ Source>  modules/mono/csharp_script.cpp:2915 @ can_instance()

The whole project can be found here: https://gitlab.com/wscalf/resourcesaver-repro

@wscalf wscalf changed the title Custom resource saves correctly from inspector but not from ResourceSaver [3.2.1] Custom resource saves correctly from inspector but not from ResourceSaver Apr 25, 2020
@RabbitB
Copy link

RabbitB commented Apr 25, 2020

I'm not sure if this is entirely your issue or if there's more to it than this, but at immediate glance, you're going to have an issue because .cs is the file extensions for a C# file, and ResourceFormatSaver which is used to recognize the type of resource when saving it with ResourceSaver is going to take priority for C# with a .cs file, unless you've registered your own with a higher priority for that extension.

@wscalf
Copy link
Contributor Author

wscalf commented Apr 25, 2020

Which file is it looking at to decide that? In my repro project, there file being imported is named Text.txt - all my Godot scripts are C#, but I'm extending it to work with a domain specific language for branching dialogue. The file extensions on the resources can be whatever - they're .txt here and currently .convo on my main project. I might change them to .dialogue or something soon. In any case, it shouldn't be an extension that's already in use, and the files don't show up in the file browser on the left unless my plugin is loaded.

Interestingly, for the same imported file, I can new up and pass in a different Resource object to the saver (like, say, a SpatialMaterial), and that serializes exactly as I'd expect, which suggests to me that it's an issue more with my resource class (whether because it was made with C# instead of gdscript or because I missed some step or didn't register something somewhere) than the file in importing ... except that it serializes correctly from the inspector.

I haven't tried making doing the same steps in gdscript to see if this is a C#/mono issue. It doesn't help me if it is because I have some .NET libraries I depend on in my actual project (versus the repro), but it might help with diagnostics and triage. I'll aim to do that today and report back.

@wscalf
Copy link
Contributor Author

wscalf commented Apr 25, 2020

Update! It does appear to be a C#/Mono issue! Specifically, it looks like it depends on the type being serialized - if it's a C# class that inherits from Resource, it gets serialized as a CScriptScript resource, but if it's a GDScript class that inherits from Resource, it gets serialized correctly.

I updated my repro project to give you a checkbox for which type of model to use, and otherwise it does everything the same (plus some interop stuff on the code that uses the resource to check which kind it has and all that.) It repros with a C# model but not with a GDScript model.

So..that gives me a better workaround than I had and may be a clue as to why it's happening. I poked around in the code some to see if I could find anything that would cause that, but I'm not super familiar with Godot's code base. I did see that the ResourceFormatSaverCSharpScript class (which I think is what you mentioned above) is will return true for any resource where Object::cast_to<CSharpScript> returns true, and I don't have a good way to know if I'm hitting that, but I at least tried calling .IsClass("CSharpScript") on one of my resource objects, and it returned false. That's probably about as deep as I can go at the moment. :/

@RabbitB
Copy link

RabbitB commented Apr 25, 2020

The only thing that noticeably stands out to me in your example code, is this line in your importer.

public override string GetResourceType() => nameof(Resource);

This is supposed to tell ResourceSaver what class of resource you're attempting to have it save, and you're telling it a generic base Resource. You should be giving it your uppercase_text class. To be honest, I don't know the exact value you should return, considering you're using a resource class declared in gdscript. If you used class_name, I would assume that. But without it, possibly the script's filename?

@wscalf wscalf changed the title [3.2.1] Custom resource saves correctly from inspector but not from ResourceSaver [3.2.1] ResourceSaver serializes resources defined in C# as CSharpScript instead of the specific resource type Apr 25, 2020
@wscalf
Copy link
Contributor Author

wscalf commented Apr 25, 2020

I did that to work around a different problem. If I give a more specific name there, it doesn't work at all, and my logs fill up with "cannot find class name" and whatever I put there. I was planning to open a different issue for that, but since it didn't seem to be impacting anything, it was lower on my list. Besides, that doesn't get passed to the ResourceSaver, which is what seems to be going off the rails. It looks like it just doesn't work with resource models defined in C#.

As a side note, how do you add a ResourceFormatSaver? ResourceSaver's add_resource_format_saver() function doesn't appear to be exposed to script code, though the ResourceFormatSaver class is. Is there something somewhere else you're supposed to call? I think adding a custom one may be a better solution to my immediate problem, but I also think there's a larger issue here.

@wscalf
Copy link
Contributor Author

wscalf commented Apr 25, 2020

Update: while looking at resource saver and loader, there's a bunch of documentation saying that custom resources aren't visible to the ClassDB and to use Resource as your resource name instead. I didn't see that until just now, but there it is: https://docs.godotengine.org/en/stable/classes/class_resourceformatloader.html#class-resourceformatloader-method-handles-type

@RabbitB
Copy link

RabbitB commented Apr 25, 2020

Besides, that doesn't get passed to the ResourceSaver

ResourceSaver relies on your import plugin to know what to do; the list of registered plugins is what ResourceSaver is polling when you save something. If you pass it the wrong type as your intended type, it has no idea of knowing otherwise and will simply fail to function properly.

The errors you were receiving were explicitly telling you that the type you wanted couldn't be found; I wish I was more help there, as I don't know what it expects when not talking about built-in types.

As a side note, how do you add a ResourceFormatSaver?

In the docs:

By default, Godot saves resources as .tres (text-based), .res (binary) or another built-in format, but you can choose to create your own format by extending this class. Be sure to respect the documented return types and values. You should give it a global class name with class_name for it to be registered. Like built-in ResourceFormatSavers, it will be called automatically when saving resources of its recognized type(s). You may also implement a ResourceFormatLoader.

@RabbitB
Copy link

RabbitB commented Apr 25, 2020

Update: while looking at resource saver and loader, there's a bunch of documentation saying that custom resources aren't visible to the ClassDB and to use Resource as your resource name instead. I didn't see that until just now, but there it is:

That's for ResourceFormatLoader, which is entirely different from your import plugin. ResourceFormatLoader handles custom file loading, where your import plugin is what allows Godot to recognize your custom resource. Custom ResourceFormatSaver and ResourceFormatLoader generally don't need to be created, unless you want to save and load a specific file format that's not normally supported. If you don't care about a generic file format, Godot handles it automatically.

@wscalf
Copy link
Contributor Author

wscalf commented Apr 25, 2020

All right, I think we're talking about two very different things at the same time.

  1. If you define a resource in C#, Godot will not save it (edit: via ResourceSaver, it does save via the inspector as previously mentioned). I just made a new project that did nothing but define two resource types and attempt to save them. The GDScript one saved, the C# one did not. I have a few paths forward for my project, but it's a larger problem than that, and I'm looking to help gather what information I can, though at the moment I don't expect I know the codebase well enough to offer a fix. Repro project here: https://gitlab.com/wscalf/evensimplerresourcesaverrepro
    In short:
public class Model : Resource
{
	[Export]
	public string Value {get; set;}
}

public void Run()
{
	ResourceSaver.Save("res://cs_output.tres", new Model() { Value = "test"});
		
	var gd_resource = (Resource)GD.Load("res://model.gd").Call("new");
	gd_resource.Set("Value", "test");
	ResourceSaver.Save("res://gd_output.tres", gd_resource);
}

The GDScript model serializes correctly, whereas the C# model does not.
2. Speaking of paths forward, I may be in business! I read the passage you referenced above about savers and loaders being called automatically, but I assumed that meant after they were registered. I didn't realize GDScript classes that inherited from ResourceFormatSaver(/Loader) would be registered automatically, but it seems they are. I'll keep going down this path and see if I can get it working, and if not, I'll fall back to making the resource model in GDScript, since that currently saves and loads correctly with the builtins.

@swift502
Copy link

swift502 commented Apr 25, 2021

Would be great to update the documentation to reflect this. Currently it suggests C# can save resources just like GDScript, but I understand it's not true. https://docs.godotengine.org/en/stable/getting_started/step_by_step/resources.html

(Unless a fix is around the corner of course)

@cgbeutler
Copy link

cgbeutler commented Aug 5, 2021

Issue #48828 may be a duplicate.
It has a few details that may help track down/solve the issue.
For example, when a resource is created on the c# side, the script object returned from GetScript() is not the cached ResourceLoader one. That means when serializing the value, it gets treated as a sub_resource instead of an ext_resource. It also means that the script object could mismatch when it comes to meta data stored on it via something like GetScript().SetMeta("Foo", "Bar")
If you create a resource the looong way via

GD.Load<CSharpScript>("res://CustomNode.cs").New() as CustomNode;

then everything works fine. You can save and load the resource all you want. (as also pointed out by WSCalf)

Sadly, that method won't work for some things, like classes with generics. For those cases, I'm pretty sure you're hozed.

@cgbeutler
Copy link

cgbeutler commented Aug 6, 2021

Workaround

I created an attribute and static class to help until this is fixed.
https://gist.github.com/cgbeutler/c4f00b98d744ac438b84e8840bbe1740

To use it, declare a class with the attribute. (This basically attaches a script path to the class.)

  [CSharpScript]
  public class CustomResource : Resource
  { ... }

Later, create new resources with

  var foo = CSharpScript<CustomResource>.New();

I also threw on there a few other functions from Script, since it's hard to get from a class to a script object.
NOTE: This will not work on generic classes.

@akien-mga akien-mga changed the title [3.2.1] ResourceSaver serializes resources defined in C# as CSharpScript instead of the specific resource type ResourceSaver serializes resources defined in C# as CSharpScript instead of the specific resource type Oct 26, 2021
@akien-mga akien-mga added confirmed and removed cherrypick:3.x Considered for cherry-picking into a future 3.x release labels Oct 26, 2021
@cgbeutler
Copy link

cgbeutler commented Mar 21, 2022

Been using my workaround of loading the script the long way for a while now. It has worked great for most all cases.

The few caveats I have found:
Loading the long way doesn't work within the script file you are trying to load.
Loading the long way doesn't work is in EditorImportPlugins. It seems that loading any Script (C# or GDScript) causes a circular ref error.

@cgbeutler
Copy link

cgbeutler commented Mar 21, 2022

For whoever fixes this, may be worth double checking EditorImportPlugin works. It seems like those are currently completely broken and unwritable for C# Resources at the moment.

@Ktar5
Copy link

Ktar5 commented Jul 2, 2022

Spent hours trying to fix this only to arrive at this issue
Going to try the workaround

@cgbeutler
Copy link

cgbeutler commented Jul 13, 2022

I just spent a half-hour of debugging to discover that this issue exists for nodes created/saved from editor plugins as well. If you create a node, set its owner, and then save the scene, the script is lost somewhere along the way and you either get a random script attached that was already in the scene or the same issue that resources have.

This is kinda a major issue. Not sure why there's not more attention on it, (unless their just hoping the net6/godot4 push will make this easier to fix...)

Anyway, if your nodes are saving wrong and losing their scripts, the same workaround listed above seems to fix it.

@raulsntos
Copy link
Member

This seems fixed in 4.0-beta.17 but keep in mind that C# scripts must be classes defined in their own file with a name that matches the name of the class, and they can't be nested inside other classes.

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

8 participants