Update many Deprecated/Experimental descriptions for consistency #88443
Conversation
|
What do you mean "conflicts with MultiMesh.xml" I just created the branch yesterday- |
Mickeon
force-pushed
the
documentation-hunting-for-deprecated
branch
from
f24bcce to
cc9da3b
Compare
Mickeon
force-pushed
the
documentation-hunting-for-deprecated
branch
2 times, most recently
from
81921d5 to
2e5a734
Compare
| @@ -80,6 +80,7 @@ | |||
| <return type="void" /> | |||
| <param index="0" name="layer" type="int" default="-1" /> | |||
| <description> | |||
| Forces the TileMap and the layer [param layer] to update. | |||
There was a problem hiding this comment.
Trust me, I did not want to say it like this but it's for consistency with the way TileMap is written. Ooooooh boy.
| @@ -14,12 +14,12 @@ | |||
| <link title="Using NavigationMeshes">$DOCS_URL/tutorials/navigation/navigation_using_navigationmeshes.html</link> | |||
| </tutorials> | |||
| <methods> | |||
| <method name="bake" deprecated=""> | |||
| <method name="bake" deprecated="This method is deprecated due to core threading changes. To upgrade existing code, first create a [NavigationMeshSourceGeometryData3D] resource. Use this resource with [method parse_source_geometry_data] to parse the [SceneTree] for nodes that should contribute to the navigation mesh baking. The [SceneTree] parsing needs to happen on the main thread. After the parsing is finished use the resource with [method bake_from_source_geometry_data] to bake a navigation mesh."> | |||
There was a problem hiding this comment.
In my opinion, the message should be as short as possible, details should be placed in the description. In the future we might use deprecated/experimental messages for warnings in GDScript and C#.
There was a problem hiding this comment.
I do agree a lot, but I also wanted to keep my reasoning consistent in the PR. "Why", then "provide alternative". So moving it in the description would go against that.
Mickeon
changed the title
|
Addressed all of the above feedback (except for Dalexeev's, may be thoughtful) |
Mickeon
force-pushed
the
documentation-hunting-for-deprecated
branch
from
2e5a734 to
f9a7587
Compare
| <method name="setup_local_to_scene" deprecated="This method should only be called internally. Override [method _setup_local_to_scene] instead."> | ||
| <method name="setup_local_to_scene" deprecated="This method should only be called internally."> |
There was a problem hiding this comment.
Is this intentional? Is this incorrect advice?
There was a problem hiding this comment.
"Instead" doesn't make sense because it is not direct equivalence. It's not the alternative or the workaround. Overriding the original setup_local_to_scene does nothing because it is not the virtual method.
|
Thanks! |
See #81458 (comment).
TL:DR: Explain why thing is deprecated/experimental as much as possible, use description to explain what it does, even if it does nothing currently.
This PR also changes a few other things along the way for a bit more consistency.
Oh for your interest there's one very long set of instructions which I did not write, that looks as follows in the built-in docs:
