GDScript: Improve DocGen #80745
GDScript: Improve DocGen #80745
Conversation
dalexeev
force-pushed
the
gds-improve-docgen
branch
from
df593a9 to
9952eb2
Compare
There was a problem hiding this comment.
I have a couple of minor suggestions, none of which are super critical, but I do think they make the code a bit more readable.
Everything else seems to make sense upon a read of the code. I'm having trouble compiling atm, so I can't test this out unfortunately. But I know any regressions will be fixed quickly since this is @dalexeev 's work :)
I'll try to test this once I manage to get stuff compiling again. I'll use the set of scripts from #72095. We should expand those to now encompass signals as well as arrays!
| if (m_var->initializer->is_constant) { | ||
| prop_doc.default_value = m_var->initializer->reduced_value.get_construct_string().replace("\n", "\\n"); | ||
| } else { | ||
| prop_doc.default_value = "<unknown>"; |
There was a problem hiding this comment.
Could we say <not constant> here instead? At least it explains why a specific value is not known.
There was a problem hiding this comment.
I just copied this from function signature hint. If we want to change this, then we need to do it here as well:
godot/modules/gdscript/gdscript_editor.cpp
Lines 720 to 722 in 5444afa
More options: <non-constant>, <non-const-expr>, <dynamic>, <unresolved>.
There was a problem hiding this comment.
I personally really like <non-const-expr> for some reason! I just feel like it's worth explaining why something is unknown, as opposed to just stating that it is. That way people can more easily and immediately understand what's happening.
I don't think changing gdscript_editor.cpp would lead to significant compat breaking, right?
There was a problem hiding this comment.
It's a bit ugly in my opinion because of the two hyphens and longer text. I would choose <unknown>, <dynamic> or <non-constant>.
There was a problem hiding this comment.
I see your own, and my opinion is certainly very subjective. I don't think <unknown> captures the reason why a value can't be written. I think <dynamic> hints at it but isn't the most explicit, whereas <non-constant> is the clearest to me, though I agree it isn't the prettiest.
There was a problem hiding this comment.
Let's keep <unknown> for now since we haven't reached a consensus and there is a precedent with signature hint. We can fix this later or maybe some other reviewer will give their opinion.
dalexeev
force-pushed
the
gds-improve-docgen
branch
from
9952eb2 to
34d4328
Compare
| if (m_var->initializer->is_constant) { | ||
| prop_doc.default_value = m_var->initializer->reduced_value.get_construct_string().replace("\n", "\\n"); | ||
| } else { | ||
| prop_doc.default_value = "<unknown>"; |
|
Thanks! |
Replace
_property_info_from_datatype()with_doctype_from_gdtype()to get the type info more universally, without intermediate conversion toPropertyInfo/MethodInfousingDocData's static methods, since this has issues with custom types.<unknown>if the value is not a constant expression).Variantinstead of inferred weak type.staticqualifier to static methods.CC @anvilfolk