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

Add a @deprecated annotation #7622

Closed
JeremyStarTM opened this issue Sep 7, 2023 · 9 comments · Fixed by godotengine/godot#100174
Closed

Add a @deprecated annotation #7622

JeremyStarTM opened this issue Sep 7, 2023 · 9 comments · Fixed by godotengine/godot#100174
Milestone

Comments

@JeremyStarTM
Copy link

JeremyStarTM commented Sep 7, 2023

Describe the project you are working on

A framework written in GDScript

Describe the problem or limitation you are having in your project

I want to deprecate a function

Describe the feature / enhancement and how it helps to overcome the problem or limitation

A @deprecated annotation that requires a custom message and a bool if the func/var will be removed in a future version

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

Example with custom message, will not be removed in future version

@deprecated("Has been moved to SomeObject.some_variable, will be kept for compatibility",false)
var some_variable: String = "Some cool variable"

@deprecated("May be inaccurate, use do_that_thing() instead",false)
func do_something() -> void:

Example with a custom message, will be removed in a future version

@deprecated("Has been replaced with SomeGlobalScript.get_some_stuff() and will be removed in the next major release",true)
var some_important_stuff: String = "Something very useful"

@deprecated("Is known to cause major issues and will be removed in the next release",true)
func will_cause_issues() -> void:

If this enhancement will not be used often, can it be worked around with a few lines of script?

A simple function like this could do the job:

func deprecate_message(script:String,function_variable:String,message:String,to_be_removed:bool) -> void:
 if to_be_removed:
  assert(false,"The function/variable \"" + function_variable + "\" in script \"" + script + "\" will be removed in a future version. Message: " + message)
 else:
  assert(false,"The function/variable \"" + function_variable + "\" in script \"" + script + "\" has been deprecated. Message: " + message)

Is there a reason why this should be core and not an add-on in the asset library?

I don't think that a addon can add a custom annotation

Why should this proposal be implemented into the engine?

  1. It would benefit modders/modloaders
  2. It would help addon developers
  3. Saving headaches if a addon's function that your project is relying on and is used heavily in your project is suddenly removed and requires major code refactors or even a rewrite to fix, where this annotation would allow for a smooth(er) transition.
@AThousandShips
Copy link
Member

AThousandShips commented Sep 7, 2023

Already implemented in:

Though without adding a reason

@JeremyStarTM
Copy link
Author

JeremyStarTM commented Sep 7, 2023

The issue #78941 is about the documentation, not about the engine. Or am I wrong somehow?

@dalexeev
Copy link
Member

dalexeev commented Sep 7, 2023

This would be useful not only for scripts, but also for built-in classes. We probably need a system to get the info from ClassDB or something else (since Variant types and scripts are not registered in ClassDB).

However, we can make GDScript get the info from DocData and generated warnings. Not sure if this is correct.

@JeremyStarTM
Copy link
Author

Added a little info that this would be useful for modloaders/modders and addon developers. Added two examples for variables too.

@dalexeev
Copy link
Member

dalexeev commented Sep 7, 2023

It might look like this:

## @deprecated: Has been moved to [member SomeObject.some_variable], will be kept for compatibility.
var some_variable: String = "Some cool variable"

## @deprecated: May be inaccurate, use [method do_that_thing] instead.
func do_something() -> void:

There are no messages because core doesn't support it (is_deprecated and is_experimental are booleans, not strings).

@JeremyStarTM
Copy link
Author

Why not add support for it then? Although it would be too much for such a simple feature addition. And if developers are smart enough to search for things, they should be smart enough too look into the documentation and see it's deprecated and why.

@TheYellowArchitect
Copy link

Until this is implemented, I would like to recommend when a user writes @deprecated it should give an error with an intuitive message like: "The annotation @deprecated requires two ## characters in front of it." instead of the current "Unrecognized annotation: @deprecation" which gaslights the user to think this is not implemented. Had to google to confirm I hadn't used this feature in the past in a dev branch, and am glad I found this issue and the above PR.

@Calinou Calinou changed the title Add @deprecated annotation Add a @deprecated annotation Dec 2, 2024
@dalexeev
Copy link
Member

dalexeev commented Dec 6, 2024

"The annotation @deprecated requires two ## characters in front of it."

@deprecated is not an annotation, but a doc comment tag. We could add a "Did you mean...?" hint for a non-existent annotation, but I'm not sure how common this is.

@Calinou
Copy link
Member

Calinou commented Dec 6, 2024

but I'm not sure how common this is.

We do this in a few other places, e.g. if you try to use ? as a ternary operator in GDScript:

Image

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

Successfully merging a pull request may close this issue.

5 participants