C#: Implement tooltips for Signals and Properties in the inspector. #83505

magian1127 · 2023-10-17T15:01:02Z

    /// <summary>
    /// tip text
    /// </summary>
    [Export]
    string s1 = "AAAA";

resolve godotengine/godot-proposals#8269

geekley · 2023-12-30T23:13:06Z

Would it be possible to reuse this implementation for documentation of GDShader uniform fields? They are shown in the inspector, and it would be really useful for sharing shaders; e.g. on sites like https://godotshaders.com

van800 · 2024-03-30T10:52:04Z

If this gets approved, it looks like worth setting
GenerateDocumentationFile in the Godot net sdk.

mhilbrunner · 2024-04-06T16:47:41Z

Removing the documentation label, as while this PR deals with documentation generation/display, this is more suited for the dotnet team to handle :)

magian1127 · 2024-05-15T10:35:22Z

Remove the need for GenerateDocumentationFile=true.

magian1127 · 2024-09-27T17:25:30Z

Hi @anvilfolk !
Sorry, I don't speak English very well, and real-time communication is difficult for me.
Currently, I am creating the PR based on the current GODOT code. After your PR is merged, I will make further modifications or submit a new PR.

anvilfolk · 2024-09-27T17:52:30Z

No worries. I can also speak portuguese, french and some spanish if that is better. I am making progress on the other PR!

Hope to see C# documentation working soon! <3

modules/mono/csharp_script.cpp

modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators/ScriptDocsGenerator.cs

raulsntos

Thanks for your patience and continued work on this feature. This is already looking pretty good, so I'm moving it to the 4.4 milestone because it seems likely to be finished on time and we want to include it in the next stable release.

However, keep in mind this is not a guarantee that it will be in the 4.4 release, it may still be pushed to a future release if we run into issues.

Also, would you be able to add tests for the new generator in Godot.SourceGenerators.Tests? This will help ensure that future changes to the generator don't change the generated code in unexpected ways.

modules/mono/glue/GodotSharp/GodotSharp/Core/Bridge/ScriptManagerBridge.cs

modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators/ScriptDocsGenerator.cs

modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators/ExtensionMethods.cs

raulsntos · 2024-09-28T18:25:06Z

editor/editor_file_system.cpp

-				bool should_reload_script = _should_reload_script(path);
 				Ref<Script> scr = ResourceLoader::load(path);
 				if (scr.is_null()) {
 					continue;
 				}
-				if (should_reload_script) {
-					// Reloading the script from disk. Otherwise, the ResourceLoader::load will
-					// return the last loaded version of the script (without the modifications).
-					scr->reload_from_file();
+				if (lang->get_name() != "C#") {
+					bool should_reload_script = _should_reload_script(path);
+					if (should_reload_script) {
+						// Reloading the script from disk. Otherwise, the ResourceLoader::load will
+						// return the last loaded version of the script (without the modifications).
+						scr->reload_from_file();
+					}


These changes don't seem to be needed, so I would revert them.

Modify the code in VS.

Rebuild in Godot.

This will execute reload_from_file.

Then, GodotSharp will throw an error about _scriptTypeBiMap being added multiple times.

Because of lang->supports_documentation(), C# will never execute here.
However, this PR adds supports_documentation().
If we don't include it, we'll need to modify _should_reload_script.

@magian1127 This still needs to be addressed. In my testing it didn't seem to be needed, but I could be missing something, so if you still think it's needed could you explain why? Thank you.

Yeah, I'd hate to see code like this start getting added, especially if folks end up adding more scripting languages in the future.

We should eventually all get together and figure out what a good doc-generation architecture might be. It probably wants to live inside Script, ScriptLanguage and ScriptServer, which would abstract it away nicely and not require special cases!

@magian1127 Thanks, I'm able to reproduce the exception. I understand why you made this change now.

Reloading a C# script checks if the script is already registered before trying to register it again. So if it still throws after that, it must mean the script is different, and that's a bug. There must always be only one CSharpScript instance for each C# type, so if another one is being created somewhere we need to find where.

The potentially related PR #98269

raulsntos

The latest changes look good, although there's still an unresolved comment. I added a few more comments, but it should be the last batch and then we can move on to a code-style review.

modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators/ScriptDocsGenerator.cs

modules/mono/csharp_script.cpp

modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators.Tests/ScriptDocsGeneratorTests.cs

modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators.Tests/TestData/Sources/ClassDoc.cs

modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators/ScriptDocsGenerator.cs

magian1127 · 2024-10-05T03:25:46Z

@magian1127 This still needs to be addressed. In my testing it didn't seem to be needed, but I could be missing something, so if you still think it's needed could you explain why? Thank you.

@raulsntos
I have written the cause and steps to reproduce. After rebuilding the project, scr->reload_from_file() causes the script to be registered multiple times.

raulsntos · 2024-10-06T03:36:26Z

It seems you forgot to submit the message. Notice the 待定¹ next to your account name.

I think that's what it says, but I can't speak Chinese. I believe this is Pending in English and it appears on messages added in a review that hasn't been submitted yet. ↩

magian1127 · 2024-10-06T04:33:16Z

😓 Some comments have not been replied to because the 'Review changes' button in the upper right corner was not clicked.

magian1127 force-pushed the 4.0tip branch from aa555e2 to 81b7c8a Compare October 17, 2023 15:08

AThousandShips added enhancement documentation topic:dotnet labels Oct 17, 2023

AThousandShips added this to the 4.x milestone Oct 17, 2023

magian1127 force-pushed the 4.0tip branch 3 times, most recently from 3c6209c to b0a3350 Compare October 18, 2023 07:47

magian1127 marked this pull request as ready for review October 18, 2023 07:54

magian1127 requested a review from a team as a code owner October 18, 2023 07:54

magian1127 force-pushed the 4.0tip branch from b0a3350 to 9c21641 Compare October 25, 2023 11:09

AThousandShips added topic:dotnet and removed topic:dotnet labels Nov 10, 2023

31 mentioned this pull request Dec 30, 2023

Add support for generating class reference descriptions from C# documentation comments godotengine/godot-proposals#8269

Open

magian1127 force-pushed the 4.0tip branch 2 times, most recently from 9c21641 to 943ddbc Compare January 4, 2024 07:24

magian1127 marked this pull request as draft March 24, 2024 06:18

magian1127 force-pushed the 4.0tip branch 3 times, most recently from 601893f to 4f4ee11 Compare March 24, 2024 07:58

magian1127 marked this pull request as ready for review March 24, 2024 07:59

magian1127 force-pushed the 4.0tip branch from 4f4ee11 to 636ea5a Compare March 24, 2024 08:22

mhilbrunner removed the documentation label Apr 6, 2024

magian1127 force-pushed the 4.0tip branch 2 times, most recently from 02a0e02 to dd8605f Compare April 23, 2024 10:00

magian1127 force-pushed the 4.0tip branch from dd8605f to b9b2fc0 Compare May 15, 2024 10:27

magian1127 force-pushed the 4.0tip branch 2 times, most recently from f29bbca to db2f047 Compare September 27, 2024 17:20

magian1127 requested review from dalexeev and raulsntos September 27, 2024 17:31

raulsntos reviewed Sep 28, 2024

View reviewed changes

modules/mono/csharp_script.cpp Outdated Show resolved Hide resolved

modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators/ScriptDocsGenerator.cs Outdated Show resolved Hide resolved

modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators/ScriptDocsGenerator.cs Show resolved Hide resolved

magian1127 force-pushed the 4.0tip branch from db2f047 to f36d1b3 Compare September 28, 2024 08:21

magian1127 requested a review from raulsntos September 28, 2024 12:05

raulsntos reviewed Sep 28, 2024

View reviewed changes

raulsntos modified the milestones: 4.x, 4.4 Sep 28, 2024

magian1127 force-pushed the 4.0tip branch 3 times, most recently from 2cf3331 to b39f2b6 Compare September 29, 2024 08:34

magian1127 requested a review from raulsntos September 29, 2024 10:03

raulsntos reviewed Oct 1, 2024

View reviewed changes

magian1127 force-pushed the 4.0tip branch from b39f2b6 to 0ed3b47 Compare October 2, 2024 04:52

magian1127 requested a review from raulsntos October 2, 2024 05:52

raulsntos reviewed Oct 3, 2024

View reviewed changes

modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators.Tests/TestData/Sources/ClassDoc.cs Show resolved Hide resolved

modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators/ScriptDocsGenerator.cs Outdated Show resolved Hide resolved

magian1127 force-pushed the 4.0tip branch from f5391e2 to 86af914 Compare October 4, 2024 05:33

magian1127 requested a review from raulsntos October 4, 2024 09:37

ryevdokimov mentioned this pull request Oct 11, 2024

Description for properties of parts Open-Industry-Project/Open-Industry-Project#42

Open

C#: Implement tooltips for Signals and Properties in the inspector.

dbcb8a4

magian1127 force-pushed the 4.0tip branch from 86af914 to dbcb8a4 Compare October 22, 2024 13:19

magian1127 requested a review from a team as a code owner October 22, 2024 13:19

ZachAR3 mentioned this pull request Nov 4, 2024

Add descriptions for exported variables / methods. ZachAR3/Destructibles-CSharp#11

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

C#: Implement tooltips for Signals and Properties in the inspector. #83505

C#: Implement tooltips for Signals and Properties in the inspector. #83505

magian1127 commented Oct 17, 2023 •

edited

Loading

geekley commented Dec 30, 2023

van800 commented Mar 30, 2024

mhilbrunner commented Apr 6, 2024 •

edited

Loading

magian1127 commented May 15, 2024

magian1127 commented Sep 27, 2024

anvilfolk commented Sep 27, 2024

raulsntos left a comment

raulsntos Sep 28, 2024

magian1127 Sep 29, 2024

raulsntos Oct 4, 2024

anvilfolk Oct 4, 2024 •

edited

Loading

raulsntos Oct 6, 2024

magian1127 Oct 17, 2024

raulsntos left a comment

magian1127 commented Oct 5, 2024 •

edited

Loading

raulsntos commented Oct 6, 2024

magian1127 commented Oct 6, 2024

C#: Implement tooltips for Signals and Properties in the inspector. #83505

Are you sure you want to change the base?

C#: Implement tooltips for Signals and Properties in the inspector. #83505

Conversation

magian1127 commented Oct 17, 2023 • edited Loading

geekley commented Dec 30, 2023

van800 commented Mar 30, 2024

mhilbrunner commented Apr 6, 2024 • edited Loading

magian1127 commented May 15, 2024

magian1127 commented Sep 27, 2024

anvilfolk commented Sep 27, 2024

raulsntos left a comment

Choose a reason for hiding this comment

raulsntos Sep 28, 2024

Choose a reason for hiding this comment

magian1127 Sep 29, 2024

Choose a reason for hiding this comment

raulsntos Oct 4, 2024

Choose a reason for hiding this comment

anvilfolk Oct 4, 2024 • edited Loading

Choose a reason for hiding this comment

raulsntos Oct 6, 2024

Choose a reason for hiding this comment

magian1127 Oct 17, 2024

Choose a reason for hiding this comment

raulsntos left a comment

Choose a reason for hiding this comment

magian1127 commented Oct 5, 2024 • edited Loading

raulsntos commented Oct 6, 2024

Footnotes

magian1127 commented Oct 6, 2024

magian1127 commented Oct 17, 2023 •

edited

Loading

mhilbrunner commented Apr 6, 2024 •

edited

Loading

anvilfolk Oct 4, 2024 •

edited

Loading

magian1127 commented Oct 5, 2024 •

edited

Loading