Add descriptions for tile properties

[KoBeWi]

Addresses godotengine/godot-proposals#7177 (comment)

The PR adds a new method to DocTools called add_custom_property_description(), which allows adding a description for arbitrary class/property. This is done outside docs, because these properties are not supposed to be documented (in some cases it's not even possible, like with TileSetAtlasSourceProxyObject), but it's still nice if they had a description in tooltip. This is now possible and likely can be expanded onto dynamic properties (e.g. ItemList's items). While adding documentation descriptions inside editor code is controversial, VisualShader already does it for its various nodes (example).

The PR uses the new functionality in TileSet editor, for various properties. Putting as draft until I fill in all descriptions.
Also I tweaked ID property to be real EditorProperty, otherwise tooltip was not available.

Production edit: closes godotengine/godot-roadmap#80

[akien-mga]

The PR adds a new method to DocTools called add_custom_property_description(), which allows adding a description for arbitrary class/property. This is done outside docs, because these properties are not supposed to be documented (in some cases it's not even possible, like with TileSetAtlasSourceProxyObject), but it's still nice if they had a description in tooltip. This is now possible and likely can be expanded onto dynamic properties (e.g. ItemList's items). While adding documentation descriptions inside editor code is controversial, VisualShader already does it for its various nodes (example).

That's a bit awkward indeed to have these added manually in the source code, as opposed to extracted for documentation like we do for other class properties. But indeed the SourceProxyObject seems a bit special, and it's still relatively self-contained so it may not be too bad.

[groud]

I wonder if it wouldn't be better to be able to expose those internal classes to DocTool somehow, but marking them as not fitting for display in the built documentation. That would be a bit cleaner IMO, as editing this documentation would better be in the XML.

Though, to be fair, I don't know if it deserves the additional work. I don't think there's a lot of places where we use that kind of proxy object. However, I think there's a lot of value in reusing the EditorInspector code (it could probably be done in other places), so maybe that would be a good future-proofing anyway ?

[YuriSizov]

This approach is problematic for all the reasons stated above. It also means we cannot expose this documentation online, so it cannot be referenced by users or used naturally in tutorials. And adding descriptions this way means they will be lost if we need to refresh the DocData at runtime (or simply if your added code runs before the doc gen runs).

I would look for another solution that better integrates with the documentation system.

[groud]

It also means we cannot expose this documentation online, so it cannot be referenced by users or used naturally in tutorials.

I don't think this is doable though. The TileSetAtlasSourceProxyObject isn't supposed to be exposed, and I think there would be no sense in presenting it in the reference of a class that cannot be instantiated. How to present it online or even in the built-in documentation seems out of the scope of this PR to me. If we can at least store the data in the XML files, that's would be already good enough IMHO.

[KoBeWi]

It also means we cannot expose this documentation online, so it cannot be referenced by users or used naturally in tutorials.

Some of the descriptions are copied from the TileSet tutorial. There is no way to keep them in sync automatically AFAIK.

[YuriSizov]

@groud If we can store the data in an XML file, this means we already have it available in online docs at no cost :)

Being non-instantiable is not an issue, we document many classes which are not supposed to be used directly by users. It's still useful.

[YuriSizov]

It also means we cannot expose this documentation online, so it cannot be referenced by users or used naturally in tutorials.

Some of the descriptions are copied from the TileSet tutorial. There is no way to keep them in sync automatically AFAIK.

What I meant is that if descriptions are not available in the online docs, people writing tutorials cannot easily link to them.

[KoBeWi]

Except AtlasTileProxyObject is completely internal and it would only confuse users. These properties are not even available from code, nor are actually properties. They are added using get_property_list().

What I meant is that if descriptions are not available in the online docs

If they are in the tutorial, they are available online, just not linked. I even opened godotengine/godot-docs#8581 because animation properties are not explained.

[groud]

Being non-instantiable is not an issue, we document many classes which are not supposed to be used directly by users. It's still useful.

But I mean, this makes little sense. The proxy object are just internal stuff, to be able to have the convenience to use the EditorInspector instead of an hand-made GUI (EditorInspector needs an edited object). Exposing this documentation would basically be the equivalent to, for example, document the 2D editor toolbar buttons in the reference. That would be very confusing.

Here, those properties are most of them defined in the proxied object anyway. Like TileSetAtlasSourceProxyObject is basically a TileSetAtlasSource with some added properties (I think there's only its ID in the corresponding TileSet). And same for the TileDataProxyObject. So this documentation kind of already exists in other places.

[YuriSizov]

But I mean, this makes little sense.

To me it makes little sense to document them but only in the editor. It limits the accessibility of the documentation, so why even bother? You can't look up the information, can't link others to it, can't copy and paste it. The reason to put it online is not just so programmers can reference it when instantiating classes. We have editor and project settings there as well, classes which you are not supposed to instantiate yourself or even use from code most of the time. Or editor only classes which exist as singletons and are also only referenced.

At any rate, you say that we should put this information in XMLs. I agree. If we do this, it will be available online anyway. So while we don't see this from the same angle, the end result will be the same and the docs will be accessible. Let's focus on "how".

[groud]

To me it makes little sense to document them but only in the editor. It limits the accessibility of the documentation, so why even bother? You can't look up the information, can't link others to it, can't copy and paste it.

It's just a nice to have as tooltips, that's pretty much it. And I don't think we have any tootips of the editor in the documentation, beside the ones that you can interact with (via code or via the editor). Here you can't access it at all. TBH, that make me reconsider the fact we should move it to the XML if we have to expose it in the end. I'd rather go for TTR(...) strings in the editor, that would be consistent with other tooltips.

At any rate, you say that we should put this information in XMLs. I agree. If we do this, it will be available online anyway. So while we don't see this from the same angle, the end result will be the same and the docs will be accessible. Let's focus on "how".

I mean, I don't mind putting it online. But what I don't want is it to appear as a random internal object in the reference.
So I don't know, if you want a custom page for editor-internal objects (not accessible at all, that should be stated somewhere), we could, but that seems a bit weird IMHO.

[YuriSizov]

I'd rather go for TTR(...) strings in the editor, that would be consistent with other tooltips.

Then I don't think it should be mixed with DocData and displayed as other tooltips related to accessible properties.

[groud]

Then I don't think it should be mixed with DocData and displayed as other tooltips related to accessible properties.

Hmm maybe. A solution might be to add a hook in EditorInspector to provide your own tooltip when needed. Something like this I guess:

String TileSetAtlasSourceEditor::give_doc(String p_object, String p_property) {
   return ... // Returns doc depending on the given object and property name
}
editor_inspector->register_docs_generator(callable_mp(TileSetAtlasSourceEditor::give_doc));

Edit: Also, this solution would allow to easily fetch the documentation of the proxyed object, for the applicable properties.

[KoBeWi]

The descriptions are handled by EditorHelpTooltip, which doesn't have any access to the inspector. It directly uses doc class cache.
I went with the current solution, because it requires the least changes and has no unwanted side-effects.

EDIT:
Seems like there might be a way to force a description into tooltip 🤔I'll try that.

[KoBeWi]

Ok I pushed solution that doesn't involve DocData.

[akien-mga]

Looks good to me with this more ad hoc solution.

Would be good for @groud to also confirm you like this approach.

[groud]

Looks perfect to me!

[akien-mga]

Thanks!