godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
87.02k stars 19.53k forks source link

[C#] `GodotObject.Connect()` has too long description in Visual Studio tooltip #94122

Open ggppjj opened 2 weeks ago

ggppjj commented 2 weeks ago

Tested versions

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

System information

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

Issue description

The tooltip in Visual Studio for the Connect method of a Node is... way too much. It doesn't even fit on my screen.

Example: image

Because the documentation isn't wrong, I hope I've opened this in the right repo.

Steps to reproduce

Attempt to connect a signal in C# using the Connect() method, hover over the Connect() method.

Minimal reproduction project (MRP)

N/A

dalexeev commented 2 weeks ago

Is it possible to add scrolling to the popup, like in the built-in editor?

DaelonSuzuka commented 2 weeks ago

This extension doesn't support C# at all, those tooltips aren't coming from here.

dalexeev commented 2 weeks ago

This extension doesn't support C# at all, those tooltips aren't coming from here.

So should it be a proposal in VSCode or a C# extension for VSCode that the author uses? I don't think we should trim documentation just because it doesn't fit into tooltips in some editors (unless there is redundant information).

ggppjj commented 2 weeks ago

This is in the source of Godot for the C# component. The tooltip is generated by Visual Studio 2022 Community, not VSCode. There is no extension, this is defined in Godot's source with comments on the method.

dalexeev commented 2 weeks ago

There is no extension, this is defined in Godot's source with comments on the method.

Yes, C# bindings are generated from the engine API, documentation is added to C# doc comments. But they don't control whether scrollbars should be used. Yes, the connect() method has a rather long description, but other editors correctly display the description by detecting that the content height is too large.

Screenshots VSCode: ![](https://github.com/godotengine/godot-vscode-plugin/assets/47700418/a09679ac-d738-4bbb-91ea-0c3c61a89d0a) Kate: ![](https://github.com/godotengine/godot-vscode-plugin/assets/47700418/52440a76-0914-46b3-9c9a-98659aff5265)

The tooltip is generated by Visual Studio 2022 Community, not VSCode.

I guess you should check if newer Visual Studio versions have the feature. If not, then you should suggest adding the feature to Visual Studio or some extension responsible for displaying documentation tooltips. Also, there's a chance that there's a setting "Display Scrollbars".

ggppjj commented 2 weeks ago

I apologize if this comes across as antagonistic, but Godot is the only project I've seen have such a large summary section in this inline documentation. It seems unhelpful from what I understand to consider this the responsibility of the IDE, as it would seem that the summary tooltips are being used by Godot and (so far in my experience at least) by Godot alone in ways that they weren't intended or designed to support (i.e. full documentation).

The documentation on the Connect() method in particular is really the only one that I can think of even within Godot that is that large in VS2022. I just think of this more of an oversight in the code comments more than a real bug with either Godot or VS2022.

ggppjj commented 2 weeks ago

Also, would it be possible to have this issue reassigned again? I still believe the correct repo is the main Godot repo, but if nothing else I do know that the VSCode extension repo isn't right.

DaelonSuzuka commented 2 weeks ago

@dalexeev Why did you transfer this here in the first place?

dalexeev commented 2 weeks ago

Why did you transfer this here in the first place?

I assumed it was a bug of the plugin, but you said it doesn't support C# at all. Sorry for my mistake.

I apologize if this comes across as antagonistic, but Godot is the only project I've seen have such a large summary section in this inline documentation.

If you suggest shortening the the connect() description to solve the problem, then we should probably transfer the issue to the godot-docs repository, I'll ask for an opinion in #documentation.

However, I think that this is not a bug of the engine, its plugins or the documentation. If a method has many nuances, then it will reasonably have a longer description. We shouldn't limit ourselves just because some editors don't support the feature. In addition, there are other methods with large code blocks.

DaelonSuzuka commented 2 weeks ago

I assumed it was a bug of the plugin, but you said it doesn't support C# at all.

But this issue isn't even for the same program. Visual Studio and VSCode are totally unrelated.

ggppjj commented 2 weeks ago

then we should probably transfer the issue to the godot-docs repository, I'll ask for an opinion in #documentation.

That would be good to my mind. I don't see a #discussion channel in either the official discord or the cafe, so I'm assuming this is an internal chat.

Thanks.

Calinou commented 2 weeks ago

That would be good to my mind. I don't see a #discussion channel in either the official discord or the cafe, so I'm assuming this is an internal chat.

It's a public channel on the Godot Contributors Chat actually :slightly_smiling_face:

Engine development happens there, not Discord.

ggppjj commented 2 weeks ago

Ah, gotcha. Thanks!

dalexeev commented 2 weeks ago
Summary ``` 62 line(s) - 1 item(s) 47 line(s) - 1 item(s) 44 line(s) - 1 item(s) 32 line(s) - 2 item(s) 29 line(s) - 2 item(s) 28 line(s) - 1 item(s) 27 line(s) - 4 item(s) 26 line(s) - 4 item(s) 25 line(s) - 1 item(s) 24 line(s) - 2 item(s) 23 line(s) - 5 item(s) 22 line(s) - 2 item(s) 21 line(s) - 8 item(s) 20 line(s) - 6 item(s) 19 line(s) - 10 item(s) 18 line(s) - 12 item(s) 17 line(s) - 14 item(s) 16 line(s) - 19 item(s) 15 line(s) - 33 item(s) 14 line(s) - 28 item(s) 13 line(s) - 31 item(s) 12 line(s) - 46 item(s) 11 line(s) - 69 item(s) 10 line(s) - 82 item(s) 9 line(s) - 92 item(s) 8 line(s) - 144 item(s) 7 line(s) - 211 item(s) 6 line(s) - 277 item(s) 5 line(s) - 524 item(s) 4 line(s) - 897 item(s) 3 line(s) - 1871 item(s) 2 line(s) - 4534 item(s) 1 line(s) - 10956 item(s) 0 line(s) - 776 item(s) ```
50 longest descriptions ``` method Object.connect 62 method Object._get_property_list 47 method JSON.stringify 44 method @GDScript.range 32 annotation @GDScript.@export 32 constant @GlobalScope.PROPERTY_HINT_TYPE_STRING 29 annotation @GDScript.@rpc 29 method Array.bsearch_custom 28 method ScriptEditor.goto_help 27 method Array.sort_custom 27 method String.format 27 method OS.get_name 27 method Control._make_custom_tooltip 26 constructor Array.Array 26 method OS.execute 26 method Performance.add_custom_monitor 26 method AnimationMixer.get_root_motion_position 25 method EditorPlugin._has_main_screen 24 annotation @GDScript.@export_range 24 method Array.reduce 23 method Engine.get_version_info 23 method Node.get_node 23 method Node.get_node_and_resource 23 method PopupMenu.add_multistate_item 23 method FileAccess.store_16 22 constructor NodePath.NodePath 22 method ImporterMesh.add_surface 21 method RenderingDevice.draw_list_begin 21 method ArrayMesh.add_surface_from_arrays 21 method Node.add_child 21 method Object._set 21 method EditorPlugin._get_unsaved_status 21 method @GlobalScope.abs 21 method @GlobalScope.is_same 21 method Array.all 20 method PhysicsServer2D.shape_set_data 20 method OS.get_cmdline_args 20 method AnimationMixer.get_root_motion_rotation_accumulator 20 method RichTextLabel.get_menu 20 method UPNP.add_port_mapping 20 method Array.any 19 method StringName.format 19 method TextEdit.get_menu 19 method AStar3D.get_id_path 19 method AStar2D.get_id_path 19 method RichTextLabel.install_effect 19 method LineEdit.get_menu 19 method @GDScript.load 19 annotation @GDScript.@export_enum 19 annotation @GDScript.@export_group 19 ```
Script ```python import glob import math import xml.etree.ElementTree as ET data = [] def get_size(description, deprecated, experimental): result = 0 if description is not None: in_codeblock = False in_csharp = False for line in description.splitlines(): line = line.lstrip("\t ") if line == "[codeblocks]" or line == "[/codeblocks]": continue if line == "[codeblock]" or line.startswith("[codeblock "): in_codeblock = True continue if line == "[/codeblock]": in_codeblock = False continue if line == "[gdscript]" or line.startswith("[gdscript "): in_codeblock = True continue if line == "[/codeblock]": in_codeblock = False continue if line == "[csharp]" or line.startswith("[csharp "): in_csharp = True continue if line == "[/csharp]": in_csharp = False continue if not in_csharp: if in_codeblock: result += 1 else: result += math.ceil(len(line) / 120) if deprecated is not None: if deprecated == "": result += 1 else: result += math.ceil(len(deprecated) / 120) if experimental is not None: if experimental == "": result += 1 else: result += math.ceil(len(experimental) / 120) return result def process_file(path): tree = ET.parse(path) root = tree.getroot() class_name = root.get("name") for item_type in ["method", "signal", "constructor", "operator", "annotation"]: items = root.find(item_type + "s") if items is not None: for item in items.iter(item_type): item_name = item.get("name") symbol = f"{item_type} {class_name}.{item_name}" description = item.find("description").text deprecated = item.get("deprecated") experimental = item.get("experimental") data.append([symbol, get_size(description, deprecated, experimental)]) for item_type in ["member", "constant", "theme_item"]: items = root.find(item_type + "s") if items is not None: for item in items.iter(item_type): if item.get("overrides") is not None: continue item_name = item.get("name") symbol = f"{item_type} {class_name}.{item_name}" description = item.text deprecated = item.get("deprecated") experimental = item.get("experimental") data.append([symbol, get_size(description, deprecated, experimental)]) def main(): for path in glob.glob("doc/classes/*.xml"): process_file(path) for path in glob.glob("modules/*/doc_classes/*.xml"): process_file(path) full_string = "" summary = {} for symbol, size in sorted(data, key=lambda item: -item[1]): full_string += f"{symbol} {size}\n" summary[size] = summary.get(size, 0) + 1 summary_string = "" for size, count in sorted(summary.items(), key=lambda item: -item[0]): summary_string += f"{size} line(s) - {count} item(s)\n" with open("doc_size_full.txt", "w") as f: f.write(full_string) with open("doc_size_summary.txt", "w") as f: f.write(summary_string) if __name__ == "__main__": main() ```
akien-mga commented 2 weeks ago

So there are two options:

dalexeev commented 2 weeks ago

See Annex D Documentation comments - D.3.16 \<summary>:

This tag can be used to describe a type or a member of a type. Use <remarks> (§D.3.12) to specify extra information about the type or member.

Mickeon commented 1 week ago

In the past, for editor tooltips, I suggested designing first line of a description to always act as a good summary, which works most of the times (at least that complements my writing style).

I wonder how that would work for the Visual Studio tooltip, with the first line contained inside <summary>, and everything else in <remarks>.

paulloz commented 1 week ago

AFAIK this is heavily dependent on the IDE. For instance, Rider will display a scrollbar, doesn't matter if everything is in <summary>, or broken into <remarks>. Maybe VS decides to never display scrollbars for <summary>. I can't say.

I agree with @Mickeon that whatever is supposed to be displayed in tooltips should have a strong first line. But without this being a hard rule, and with the XML docs being generated without human intervention, it is quite hard for everything to be perfect. This is obviously an extreme example, but I doubt there'd be the manpower to manually edit our XML docs (proof being the C# documentation still lagging behind).

@akien-mga That should answer your first point. As for where are API documentations, I'd say, usually on a webpage (like us with https://docs.godotengine.org).