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

Some issues with custom class documentation generated from comments #67704

Closed
Cykyrios opened this issue Oct 21, 2022 · 7 comments · Fixed by #80745
Closed

Some issues with custom class documentation generated from comments #67704

Cykyrios opened this issue Oct 21, 2022 · 7 comments · Fixed by #80745

Comments

@Cykyrios
Copy link
Contributor

Godot version

v4.0.beta.custom_build [72b845b]

System information

Linux Mint 21

Issue description

The documentation generated from a script has some issues, below is the list of what I noticed so far:

  • No line breaking unless using [br] (I assume this is on purpose to avoid unwanted line breaks - as a side note, [br] adds spacing to the new line)
  • Private properties appear in Property Descriptions even though they don't in the Properties list
  • Typed arrays always appear as Array
  • Signal parameters always appear as Variant, even when explicitly typed
  • Array properties do not show their default values
  • Typed arrays cannot be displayed in descriptions by typing [Array[int]] for instance, this will only highlight [int] and keep the brackets around Array.
  • I suppose having virtual appear next to virtual methods would be nice, but since other private methods are hidden, this should not be an issue.

See MRP for some screenshots of the issues.

Steps to reproduce

Create a new script, add signals, enums, properties, methods as wanted, then open the help page of the created class.

Minimal reproduction project

GeneratedDocumentation.zip

I wrote a dummy class to test the documentation feature, here are some screenshots of the issues mentioned above:

  • Missing Array variable default value:
    image
  • Private property in Property Descriptions (also note the descriptions show up even though none of the properties has one):
    image
  • Signal parameters are always Variant:
    image
  • Typed arrays as arguments or method return value show up as Array (should be Array[float] here):
    image
@Cykyrios
Copy link
Contributor Author

I had a look at the code and found this in editor_help.cpp when populating Property Descriptions:

godot/editor/editor_help.cpp

Lines 1488 to 1491 in 72b845b

for (int i = 0; i < cd.properties.size(); i++) {
if (cd.properties[i].overridden) {
continue;
}

But when creating the Properties list, we have this:
for (int i = 0; i < cd.properties.size(); i++) {
// Ignore undocumented private.
if (cd.properties[i].name.begins_with("_") && cd.properties[i].description.strip_edges().is_empty()) {
continue;
}

I think we should either check whether the property is private here as well, or filter the list beforehand, as it is done for methods in _update_method_descriptions.

As a test, I copy-pasted the above code and it fixes this particular issue, private properties no longer appear in Property Descriptions.

@dalexeev
Copy link
Member

dalexeev commented Jul 18, 2023

  • No line breaking unless using [br]

This is intentional and documented.

  • [br] adds spacing to the new line
  • Private properties appear in Property Descriptions even though they don't in the Properties list
  • Signal parameters always appear as Variant, even when explicitly typed
  • Typed arrays always appear as Array
  • Array properties do not show their default values
  • Typed arrays cannot be displayed in descriptions by typing [Array[int]] for instance, this will only highlight [int] and keep the brackets around Array.

TODO. This is an Editor Help issue. You can use [Array][[int]] as a workaround.

  • I suppose having virtual appear next to virtual methods would be nice, but since other private methods are hidden, this should not be an issue.

I didn't quite understand what you meant. Perhaps the point is outdated and it has been fixed?

@Cykyrios
Copy link
Contributor Author

  • I suppose having virtual appear next to virtual methods would be nice, but since other private methods are hidden, this should not be an issue.

I didn't quite understand what you meant. Perhaps the point is outdated and it has been fixed?

This last point is only true for built-in virtual functions, I think. If I make a custom script and document a function that has an underscore as its first character, it is treated as a normal function. (see last screenshot in the OP, this still appears the same after pulling the latest commits on the master branch, but I do see "virtual" next to built-in Node functions like in your screenshot)

@dalexeev
Copy link
Member

I still don't understand what you mean, sorry.

All non-static GDScript functions are virtual, since the method implemented in the derived class is called (static functions have some issues: #72973, #77895). Adding the qualifier to the user documentation makes no sense and can be confusing since the virtual keyword (unlike static) is not present in GDScript.

When overriding a virtual built-in method, the user documentation no longer displays the vitual qualifier. In my opinion, this is also correct.

## _ready() override.
func _ready() -> void:
    pass

@Cykyrios
Copy link
Contributor Author

I agree it is correct not to show "virtual" next to an overridden method, and as it would probably be too much of a hassle to determine if a method is supposed to be overridden in the parent class, this point is probably moot. I was just thinking it could be nice to know in the custom documentation if a method was virtual or not.

@dalexeev
Copy link
Member

it could be nice to know in the custom documentation if a method was virtual or not.

Do you mean the "overrides [class name]" text (like for the properties) and the blue arrow indicator? If so, then this is more of a proposal than a bug.


@Cykyrios
Copy link
Contributor Author

What I meant initially was just having "virtual" next to the method, just like the built-in ones in the Node class that you showed in your screenshot (when documenting a script with a method that you intend to be overridden). Pretty much in the same way that methods starting with an underscore are private by convention, those starting with an underscore, but documented, could be considered virtual by convention as well. I agree though this is more of a proposal than a bug either way.

@akien-mga akien-mga added this to the 4.2 milestone Aug 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants