Added system for deprecating methods, properties, signals and constants.

[Piet-G]

As requested in #4397, I tried to include most suggestions from there.

Description

This PR will allow methods, properties, signals and constants to be marked as deprecated and show a warning when they are first used.

They are deprecated by using the following macros in the file where the methods/properties/signals/constants are bound:

DEPRECATE_PROPERTY("property_name", DeprecationInfo())
DEPRECATE_METHOD("method_name", DeprecationInfo())
DEPRECATE_SIGNAL("signal_name", DeprecationInfo())
DEPRECATE_CONSTANT(CONSTANT_NAME, DeprecationInfo())

The DeprecationInfo struct can be used to set the warning that is shown.
When left empty the default warning will be shown:

The method method_name has been deprecated and will be removed in the future.
The property property_name has been deprecated and will be removed in the future.
The signal signal_name has been deprecated and will be removed in the future.
The constant CONSTANT_NAME has been deprecated and will be removed in the future.

The .replacement string of DeprecationInfo can be set to indicate which method/property/signal/constant should be used instead.

The warning will become:

The method method_name has been deprecated and will be removed in the future. Use replacement_method instead.

The .removal_version string of DeprecationInfo can be set to indicate which version it will be removed entirely, it will replace the vague in the future in the warning message. This can be used in conjunction with the .replacement string.

If a totally custom warning is needed the .custom_warning of DeprecationInfo can be used and that warning will replace the default warning.

This functionality is only present is only present in TOOL builds to maximize performance due to an extra check that occurs when calling methods and such. This was suggested in #4397

Examples

Some stupid examples

ClassDB::bind_method(D_METHOD("print_stray_nodes"), &Node::_print_stray_nodes);
DEPRECATE_METHOD("print_stray_nodes", DeprecationInfo());

ADD_PROPERTYNZ(PropertyInfo(Variant::STRING, "name", PROPERTY_HINT_NONE, "", 0), "set_name", "get_name");

DEPRECATE_PROPERTY("name", DeprecationInfo("better_name"));

ADD_SIGNAL(MethodInfo("renamed"));
DEPRECATE_SIGNAL("renamed", DeprecationInfo("better_renamed", "3.2"));

BIND_ENUM_CONSTANT(DUPLICATE_SCRIPTS);
DeprecationInfo deprecation_info;
deprecation_info.custom_warning = "This constant is no longer in use and is deprecated. PLS NO USE.";
DEPRECATE_CONSTANT(DUPLICATE, deprecation_info)

Possible Future Improvements

• Make the deprecated methods/properties/signals/constants be properly marked as such in the documentation. (I would've done this for this PR but I'm totally unfamiliar with the code that generates the documentation, I might look into this later.)
  This seems very doable with the way the deprecation is stored.

• Make it possible to automatically redirect the deprecated method/property/signal/constant to another one (I'm not sure how needed this functionality really is, let me know what you think).

BTW if needed I can go trough all deprecated methods and mark them using this.

[neikeq]

The code in ClassDB should be surrounded with DEBUG_METHODS_ENABLED instead of TOOLS_ENABLED.

[neikeq]

I don't think the current way to print the warning is good. These methods are not always executed through ClassDB. C# uses ptrcall (with the exception of vararg methods) and so does GDNative IINW. The warning will not be printed in those cases, which can cause confusion.

A different solution could be:

• GDScript prints the warning at parse time. Since this is only possible with static typing or some type inference, it could also be printed when the method is being invoked from GDScriptFunction.
• C# methods/properties will be generated with the [Obsolete] attribute. The glue could also print the warning when invoking the method.
• I'm not familiar with GDNative so ¯_(ツ)_/¯

Also there should be an editor setting or project setting to disable the warning.

---

Regarding DeprecationInfo. I don't like some of its properties.

1. replacement: The replacement is not always part of the same class, the new method can have a different signature or something more complex. Instead this should be mentioned in custom_warning.
2. custom_warning: A custom warning should not include "This constant is no longer in use and is deprecated". This should always be part of the warning, so it shouldn't be typed manually. A correct usage would be custom_warning = "Use X instead." which would result in the warning This constant is no longer in use and is deprecated. Use X instead.. I think a better name for this property would be message.
3. removal_version: I'm not sure if we should have this. How useful is it to know the version in which it will be removed? Most of the times it will be the next x.0.0 release. If that wasn't the case, it could still be calculated erroneously. What if there is a change of plans? What if this version is skipped?

Make it possible to automatically redirect the deprecated method/property/signal/constant to another one (I'm not sure how needed this functionality really is, let me know what you think).

This would be overkill and in many cases it may not even be possible. I don't think people need it either. The point of deprecated is that you can still use the method, but you know it will be removed in the future so you can calmly plan to refactor your code over time.

[neikeq]

I think it would be great if we could also deprecate a class.

[Piet-G]

@neikeq I made a bunch of improvements:

I removed both the replacement and removal_version from the DeprecationInfo, they were indeed pretty unnecessary.

I moved the test of method call to the MethodBind as you suggested.

I also moved most code to a separate find, disconnection deprecating things from the ClassDB this approach felt cleaner and made the code less cluttered IMO, also allows for easier future expansion of the deprecation system.

[Piet-G]

Made a couple of suggested changes.

[akien-mga]

Moving to 3.2 milestone as we're about to enter the beta stage and release freeze for Godot 3.1. Only bug fixes against issues of the 3.1 milestone (or enhancements to core features of the 3.1 roadmap) will be considered until 3.1-stable is released.

Note: If it's merged after 3.1-stable is released, this change may be considered for cherry-picking into a later 3.1.x release.

[akien-mga]

Note that I'm really looking forward to this feature, but it's too late for 3.1 anyway to make good use of it IMO. I'd like to have this merged shortly after 3.1 is out, and some focus on refactoring existing deprecation statements (and writing guidelines for adding new ones while working on 3.2/4.0).

[Faless]

Any news on this? I would love to deprecate some functions in 3.2 (mainly SceneTree's network-related methods/signals)

[akien-mga]

Any news on this? I would love to deprecate some functions in 3.2 (mainly SceneTree's network-related methods/signals)

I wanted to give it a thorough check and make sure that it can fit our upcoming needs to not only deprecate, but also alias methods, etc. to preserve some level of compatibility with 4.0's upcoming API changes. But didn't get to do it, and now it's likely too late for 3.2 to properly integrate and use this.

If you want to give it a try/review though, it would be very welcome, and we can discuss at Godot Sprint if it's sufficient for our future needs or what else should be included. In particular, I saw @reduz already started adding some back-compat mechanisms in the vulkan branch for things he renamed, so we probably need to discuss those together.

[aaronfranke]

@Piet-G Is this still desired? If so, it needs to be rebased on the latest master branch.

If not, abandoned pull requests will be closed in the future as announced here.

[aaronfranke]

This PR has not received any new commits for over a year and is abandoned, closing. This feature is still desired, it can be re-opened or re-created later, and anyone is welcome to salvage it.