C# class validation fails with filenames that include a period

[ggppjj]

Tested versions

Reproducible in v4.3.beta2.mono.official [b75f0485b]

System information

Windows 11 - v4.3.beta2.mono.official [b75f0485b]

Issue description

I like to use web filenesting in Visual Studio's editor to keep what is likely an abomination of a file structure managed by the IDE like so:

Project.cs Project.BaseClass1.cs Project.BaseClass1.SubClass1.cs Project.BaseClass1.SubClass2.cs

Godot's file manger on creation of a C# file doesn't like making a file this way, with any additional period in the filename causing it to report a bad extension.

I suspect the part of Godot's file manager that reads the extension is reading the extension left-to-right from the first period in the filename, instead of reading from the end of the string up to the first period right-to-left.

Steps to reproduce

Attempt to create a new C# script with the name TestName.Test.cs

Minimal reproduction project (MRP)

N/A. Note for C# users understood, N/A.

[ggppjj]

I've updated the title to reflect what's actually happening, and will be opening a PR shortly with a change that should fix it. I'm building now to verify before submitting.

[raulsntos]

For C# scripts the filename must match the name of the C# class, and C# class names can't have periods.

That said, if the dialog is complaining about the extension then that's a bit unexpected. I would expect to see Class name must be a valid identifier.

[ggppjj]

I do see the class name error you named, apologies for the confusion.

I use partial classes a lot in library code so something like a SaveFile class would look like:

namespace Test;

public partial class BaseGame
{
    public partial class System
    {
        public class SaveFile
        {
            public int BlahBlah { get; }

            public SaveFile() => BlahBlah = 1;
        }
    }
}

... and would get the filename BaseGame.System.SaveFile.cs, where there exists already a BaseGame.System.cs and a BaseGame.cs.

I am doing this already in VS2022 with library code, but I don't know if adapting the existing class check to separate the string using the period into multiple strings and iterating over them each as class names is useful or not.

As a side-note, the function that does the check is currently only used for this dialog with C#.

[paulloz]

IMHO, the file manager in Godot should only handle Godot objects. And thus refuse file names that don't make sense in this context (even when they might be valid outside of this context).

[ggppjj]

I am very much just starting out with Godot, but I do plan on using partial classes until I'm dead in the ground where it makes sense, and C# allows me to do this without too much fuss just with a partial keyword.

A workflow that implements this would look like:

Godot object is created in the editor -> Script is worked on -> Script gets too big -> Parts of the script can be broken out into a partial class file, say a single really big method gets its own file.

So I may start with Player.cs that maps to a Player node in the editor, but decide later to add a Player.DoComplexThing() method, or Player.FamilyTree subclass. Partial classes make organizing these easier and allow for larger codebases to be split in a way that's easier to handle (to my mind). The fact that my IDE, VS2022, includes a "web" filenesting mode that keeps things tidy specifically when using this file convention, is a massive benefit to me.

As a side-note, GDscript currently allows for this naming convention, "Testing.Testname.gd" is totally valid in the newest beta. My change brings closer parity to the way that GDscript is currently handled.

[raulsntos]

As a side-note, GDscript currently allows for this naming convention, "Testing.Testname.gd" is totally valid in the newest beta. My change brings closer parity to the way that GDscript is currently handled.

GDScript and C# are different languages that have different rules. For example, a GDScript doesn't even need to have a class name, but C# scripts are C# classes so they all have names. As a result, the generated C# script from the template would be invalid if the provided filename is not also a valid C# identifier.

This check was added in https://github.com/godotengine/godot/pull/74330 to prevent generating invalid C# scripts.

For your use case it sounds like using the IDE to create these partial files would solve it. Your IDE probably also has refactoring tools that allow you to move these big methods to a newly created file in one step.

[ggppjj]

This check was added in https://github.com/godotengine/godot/pull/74330 to prevent generating invalid C# scripts.

I could only find a call to this check in the file creation dialog, but I think my main sticking point is that "test.class.cs" is a valid name for a C# script and Godot currently doesn't allow for it.

For your use case it sounds like using the IDE to create these partial files would solve it. Your IDE probably also has refactoring tools that allow you to move these big methods to a newly created file in one step.

That is true, and as it is a use-case that the language supports I am of the opinion that Godot should similarly support it. Not the fancy one-click refactoring as a whole but just the general "allow you to make the file from within Godot" part.

[raulsntos]

I think my main sticking point is that "test.class.cs" is a valid name for a C# script and Godot currently doesn't allow for it.

Right, because of the limitations I mentioned of the C# scripting language implementation in Godot. I think it's preferable to disallow names that can result in confusing behavior.

In the context of creating scripts, it makes sense to disallow anything that can't be a valid script. A file with the name "test.class.cs" can't be a script, the C# class would have to be named test.class which is an invalid identifier. Disallowing this name avoids confusion when the user tries to attach it to a Node/Resource.

I am of the opinion that Godot should similarly support it. [...] just the general "allow you to make the file from within Godot" part.

While I think creating C# files from the IDE is better for your use-case. You can still create any file you want from within Godot if you use the Create New > TextFile... option:

This allows you to circumvent the script creation checks. But keep in mind that you may be shooting yourself in the foot, be aware of the limitations of using C# as a scripting language in Godot.

[ggppjj]

In the context of creating scripts, it makes sense to disallow anything that can't be a valid script. A file with the name "test.class.cs" can't be a script, the C# class would have to be named test.class which is an invalid identifier. Disallowing this name avoids confusion when the user tries to attach it to a Node/Resource.

I'm confused by this point, and I suspect I just don't know something about how Godot or C# works. Apologies if being self-taught has left me with a huge gaping hole in my knowledge that would make all of this click for me.

Is a nested class not valid? Here's a minimal script in C# for demonstration, fiddle here: https://dotnetfiddle.net/fjFbv3

using System;

TestClass.TestSubClass test1 = new();
Console.WriteLine($"{test1.TestInt}");

public class TestClass
{
    public class TestSubClass
    {
        public int TestInt { get; set; }
        public TestSubClass() => TestInt = 1;
    }

    public TestSubClass TestSubClassInTestClass { get; set; }
    public TestClass() => TestSubClassInTestClass = new();
}

I should be able to directly make an instance of TestClass.TestSubClass based on my understanding of C#. Is there something I'm missing with how this would interact technically with Godot that would make this wrong?

At this point, I only continue to ask about this because it feels like there's something I don't understand about Godot C# that I really should based on my preferred workflow with C#.

[raulsntos]

The class TestClass.TestSubClass is a perfectly valid C# class and you can use it like you are doing in that example, but you can't use it as a Godot script (as in attaching the file to a Node or Resource).

[ggppjj]

I apologize again if I'm being dumb here, unless there's another reason that it can't be used, PR #94134 exists to address the inability to attach it to a Node or Resource. Unfortunately, I haven't yet had the chance to test directly with creation of a new project with a new scene and a new script named "testscript.scripts.cs" or something silly to full build (dotnet dependencies and limited time between work responsibilities are stopping me from fully testing), but script creation seems OK and the Godot Editor doesn't seem to have a problem with it with the PR applied.

[ggppjj]

I think I do finally understand, the main concern is in the autogenerated C# that uses the class name from the filename to generate the class.

I still would like to make Godot's handling of C# a bit closer to how C# is able to be handled, though. Would a potential solution here that would allow for the creation of complex class names be to use the last section of a split filename as the name of the actual class? I'd be willing to resubmit the PR with this in mind if so.

[ggppjj]

So in my head, this would look like:

If a user wanted to create MainGame.Test.cs, Godot would only really care about "Test" as the actual class name.

User creates MainGame.Test.cs during the attachment to a Node2D, Godot generates:

using Godot;
using System;

public class Test : Node2D
{
    // Called when the node enters the scene tree for the first time.
    public override void _Ready()
    {
    }

    // Called every frame. 'delta' is the elapsed time since the previous frame.
    public override void _Process(double delta)
    {
    }
}

[ggppjj]

PR updated with this in mind.

[raulsntos]

Would a potential solution here that would allow for the creation of complex class names be to use the last section of a split filename as the name of the actual class?

That would still not allow you to attach this file to a Node or Resource, since the name of the .cs file does not match the name of the class.

As mentioned earlier, I think it'd be preferable to disallow creating invalid scripts (files that can't be attached) when creating them from Godot. And advanced use cases can use an external file manager or IDE already.