Use structured data for "deprecated" in gdextension_interface.json#113697
Conversation
100% 🙂 Regex was the way for
This looks like a great start. Regarding a potential @Deprecated("Now obsolete", ReplaceWith("better(i)", "org.acme.better"), DeprecationLevel.WARNING)
fun good(i: Int) { ... }(There's also imports, and the level which can be increased to We would not need that granularity, but one use case I see is automating links to docs. {
// for function `mem_alloc`
"deprecated": {
"since": "4.6",
"message": "Does not take into account padding.",
"replace_with": "mem_alloc2" // <- optional field
}
}We could then generate links to the replacement in Doxygen/Sphinx/Rustdoc/... It's a small convenience, but if it's not too complicated to implement, might be nice 😉 |
dsnopek
force-pushed
the
gdextension-interface-deprecated-pattern
branch
from
2896a66 to
7958a46
Compare
dsnopek
force-pushed
the
gdextension-interface-deprecated-pattern
branch
from
7958a46 to
c351c87
Compare
dsnopek
changed the title
gdextension_interface.jsongdextension_interface.json
|
@Bromeon Thanks for the feedback! I've switched over to using structured data in my latest push :-) I ended up only making "since" required, because retroactively writing all the "message"s would be onerous. Later, if we want, we can fill them in and make it required. Also, the Python code validates that the replacement actually exists and that it isn't deprecated too, which should help to prevent errors. It even found one error right away, which I think already justifies switching to structured data! |
Bromeon
left a comment
Bromeon
left a comment
There was a problem hiding this comment.
Not directly related to this PR, but to double-check, the current logic still adds the deprecation comment in all the possible places, right? Compared to 4.5 gdextension_interface.h:
typedef struct {
...
} GDExtensionClassCreationInfo; // Deprecated. Use GDExtensionClassCreationInfo4 instead.
typedef void (*GDExtensionScriptInstanceFreePropertyList)(...); // Deprecated. Use GDExtensionScriptInstanceFreePropertyList2 instead.
/**
* @name classdb_construct_object
* @since 4.1
* @deprecated in Godot 4.4. Use `classdb_construct_object2` instead.
*
* ...
*/
typedef GDExtensionObjectPtr (*GDExtensionInterfaceClassdbConstructObject)(...);The changes here look good, thanks! 👍
dsnopek
force-pushed
the
gdextension-interface-deprecated-pattern
branch
from
c351c87 to
b15f9de
Compare
Yep! The messages just look a little bit different: typedef struct {
...
} GDExtensionClassCreationInfo; /* Deprecated in Godot 4.2. Use `GDExtensionClassCreationInfo4` instead. */
/**
* @name classdb_construct_object
* @since 4.1
* @deprecated Deprecated in Godot 4.4. Use `classdb_construct_object2` instead.
*
* ...
*/
typedef GDExtensionObjectPtr (*GDExtensionInterfaceClassdbConstructObject)(...); |
|
Thanks! |
akien-mga
changed the title
gdextension_interface.jsongdextension_interface.json
3 participants
In updating godot-cpp to use the new
gdextension_interface.jsonfile, I ended up needing to know which Godot version each interface function was deprecated in, so that I could properly implement adeprecated=nooption.This information is in the deprecated message for all interface functions currently, but the presence of this text isn't enforced.
This PR makes "deprecated" into structured data that looks like:
I ended up only making "since" required, because retroactively writing all the "message"s would be onerous. Later, if we want, we can fill them in and make it required.
Also, the Python code validates that the replacement actually exists and that it isn't deprecated too, which should help to prevent errors. It even found one error right away, which I think already justifies switching to structured data. :-)
(Note: I'm targeting this for Godot 4.6 because it's a tweak to a new feature added in Godot 4.6.)
Original Description
This PR puts a pattern on the JSON schema that enforces that this text will always be there! This way the regex I added in godot-cpp can't be broken in the future.
However, this makes me wonder if this shouldn't be structured data instead? I'm not sure how that should look, though? It could maybe be:
That begs the question of if the replacement (ex
mem_alloc2in this case) shouldn't be in structured data too? But I'm very unsure about that, because a replacement isn't required: a function can be deprecated and replaced with nothing, or there could even be multiple replacements depending on context. That's why this is a free-form text field to begin with.So, this PR takes the easy path, and just mandates that it must start with the pattern that includes the deprecated version. And maybe that's OK?