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

Discussion: Deprecation #154

Closed
ithinkandicode opened this issue Mar 1, 2023 · 4 comments · Fixed by #156
Closed

Discussion: Deprecation #154

ithinkandicode opened this issue Mar 1, 2023 · 4 comments · Fixed by #156
Assignees
Labels
enhancement New feature or request question / discussion Further information is requested

Comments

@ithinkandicode
Copy link
Collaborator

ithinkandicode commented Mar 1, 2023

Many of the changes outlined in #153 would be breaking, so for those, and for the future in general, it would be good to have a standardised way of handling deprecated method calls.

Suggestion

# A method has changed its name/class
func deprecated_changed(old_method: String, new_method: String, since_version: String):
	ModLoaderUtils.log_fatal(
		str(
			"The method \"%s\" has been deprecated since version %s. " % [old_method, since_version]
			"Please use \"%s\" instead. " % new_method,
			"The old method will be removed with the next ModLoader update, and will break your code if not changed. "
		LOG_NAME
	)

# A method has been entirely removed, with no replacement
# (should never be needed but good to have just in case)
func deprecated_removed(old_method: String, since_version: String):
	ModLoaderUtils.log_fatal(
		str(
			"The method \"%s\" has been deprecated since version %s, and is no longer available. " % [old_method, since_version]
			"There is currently no replacement method. ",
			"The method will be removed with the next ModLoader update, and will break your code if not changed. "
		LOG_NAME
	)

# Freeform deprecation message.
# Allows you to add a deprecation comment without the above conventions
func deprecated_message(msg: String, since_version: String = ""):
	if not since_version == "":
		ModLoaderUtils.log_fatal(str(msg, " (since version %s)" % since_version), LOG_NAME)
	else:
		ModLoaderUtils.log_fatal(msg, LOG_NAME)

Example Usage

class_name OldClass

func foo(var1, var2):
	deprecated_changed("OldClass.foo", "NewClass.bar", "6.0.0")
	NewClass.bar(var1, var2) # Still allow the code to run outside of the editor

Keeping Old Files

If an entire class becomes deprecated, we can create a directory called deprecated and put that class there, with its contents just being all its old func definitions with deprecated_changed used for each one.

@ithinkandicode ithinkandicode added the enhancement New feature or request label Mar 1, 2023
@ithinkandicode ithinkandicode added the question / discussion Further information is requested label Mar 1, 2023
@Qubus0
Copy link
Collaborator

Qubus0 commented Mar 1, 2023

Yeah that seems good. I would make the message a bit harsher, since many beginners do not know what "deprecated" means.

"Calling this method with "%s" has been deprecated. Use "%s" instead. The method will be removed with the next ModLoader update and will break your code if not changed."

Also, not all changes might be as simple as "moved from one class to another", so we could also use a deprecated() method with a freeform message

@ithinkandicode
Copy link
Collaborator Author

Good call @Qubus0. I've updated the methods to mention that. I've also added a version string to the args, with it being optional for the freeform method.

@Qubus0
Copy link
Collaborator

Qubus0 commented Mar 1, 2023

Yep. This sounds exactly like what we need. 🚦🟢

@ithinkandicode
Copy link
Collaborator Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question / discussion Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants