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

Editor_description documentation for 3.x #93680

Open
wants to merge 1 commit into
base: 3.x
Choose a base branch
from

Conversation

FrederickKDP
Copy link

Documentation for editor_description that was missing for 3.x . In master this issue was solved, even thought the property already existed in 3.x. The documentation is the same as for the master, just copied and pasted from that.

Documentation for editor_description that was missing for 3.x . In master this issue was solved, even thought the property already existed in 3.x. The documentation is the same as for the master, just copied and pasted from that.
@FrederickKDP FrederickKDP requested a review from a team as a code owner June 27, 2024 20:52
@Mickeon
Copy link
Contributor

Mickeon commented Jun 27, 2024

I'm sorry, this is not a valid addition for a few reasons. See godotengine/godot-docs#3621 (comment)

Additionally, you do not add new entries to the XML file directly. A base for the class reference is automatically generated based on the contents of the class's _bind_methods().

@FrederickKDP
Copy link
Author

FrederickKDP commented Jun 28, 2024

I'm sorry, this is not a valid addition for a few reasons. See godotengine/godot-docs#3621 (comment)

Additionally, you do not add new entries to the XML file directly. A base for the class reference is automatically generated based on the contents of the class's _bind_methods().

Hi, thanks for the reply, sorry for not contributing correctly. I read this issue before creating this request, but could not understand why 3.x does not have it. The property is there without any description, I find it very confusing the documentation not addressing it.
Please, I would appreciate clarification.

@akien-mga
Copy link
Member

In 4.x, editor_description is bound as a normal property:

void Node::set_editor_description(const String &p_editor_description) {
        ERR_THREAD_GUARD
        if (data.editor_description == p_editor_description) {
                return;
        }

        data.editor_description = p_editor_description;
        emit_signal(SNAME("editor_description_changed"), this);
}

String Node::get_editor_description() const {
        return data.editor_description;
}

...

        ADD_PROPERTY(PropertyInfo(Variant::STRING, "editor_description", PROPERTY_HINT_MULTILINE_TEXT), "set_editor_description", "get_editor_description");

In 3.x, it's bound as an internal property, and is using metadata internally, instead of a class member.

void Node::set_editor_description(const String &p_editor_description) {
        set_meta("_editor_description_", p_editor_description);
}
String Node::get_editor_description() const {
        if (has_meta("_editor_description_")) {
                return get_meta("_editor_description_");
        } else {
                return "";
        }
}

...

        ADD_PROPERTY(PropertyInfo(Variant::STRING, "editor_description", PROPERTY_HINT_MULTILINE_TEXT, "", PROPERTY_USAGE_EDITOR | PROPERTY_USAGE_INTERNAL), "_set_editor_description", "_get_editor_description");

Changing the property binding to:

ADD_PROPERTY(PropertyInfo(Variant::STRING, "editor_description", PROPERTY_HINT_MULTILINE_TEXT), "_set_editor_description", "_get_editor_description");

might be enough to expose it to the docs.

Note that the editor_description_changed signal doesn't exist in 3.x at all, so it obviously shouldn't be documented.

Overall the 3.x implementation could be replaced by the 4.x one, but then one would need to take special care to compatibility and making sure that pre-existing metadata based descriptions are properly converted.

Personally, I'd tend to say 3.x is old enough with its current API that it should be left as is. Changing the API might confusing existing users more than benefit them, and the 3.x users are mainly pre-existing users with big projects, as most new projects are started on 4.x.

@FrederickKDP
Copy link
Author

Very reasonable, I see now that is not as simple as I thought. Thanks so much for the time
About being worth it or not, I feel it really depends how long 3.x will stay around. "editor_description" is not an obscure feature, and I don't find it intuitive since sometimes the scene has to be reloaded to take effect
image . To be clear, I'm not asking to this to be fixed for 3.x if it's not an easy fix, I agree 3.x should not get new features. But I'm saying that because of this, for someone unfamiliar with Godot it looks like nothing is happening, and they can't search what the intention was.
By itself it's not much, but it adds up to other features that aren't documented. In my opinion, documentation should be as comprehensive as possible, unless, of course, it's not supported anymore. Then I agree it should not be updated further.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants