-
-
Notifications
You must be signed in to change notification settings - Fork 97
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
Make the script editor not emit "shadowing" warnings when reusing non-static names in static functions #6898
Comments
Probably the warning message needs to be improved, but personally I find name collisions to be error-prone (even between static and non-static members). Static members are not separated from non-static members by barbed wire, and you are not required to use the |
Turns out the part related to errors was a bug, so I edited the proposal to focus on the warnings. |
While this discussion seems to be dead, I still want to add my thoughts: I use shadowing all the time, to me it's an important core feature of any language. I try to give my variables descriptive, but short and concise names and avoid using any kind of prefixes. This results in getting the warning a lot. The most common case is constructor methods: class_name WebSocketPacket
var type := ""
var data: Variant = null
@warning_ignore("shadowed_variable")
static func create(type: String, data: Variant):
var instance = WebSocketPacket.new()
instance.type = type
instance.data = data
return instance In this case But there are also non-static cases where shadowing is exactly what I want: signal status_changed()
enum Status { WAITING, RUNNING, STOPPED }
var status := Status.WAITING
@warning_ignore("shadowed_variable")
func update_status(status: Status):
if self.status == status:
return
self.status = status
self.status_changed.emit() In my opinion, this is very readable code, with the exception of that pesky In the end, I think it's personal preference. I understand that the warning is helpful to beginners or those who simply don't like shadowing. But I feel like the warning system should not be used to dictate a certain code style. Shadowing is a feature of the language and users should be allowed to use it. But to me, I found myself writing worse code because of the warnings, as tagging functions with Now, the feature that tries to give a solution is obviously that I can configure my editor to just not show that warning. But unfortunately that does not help at all, since I want to publish my project to the asset library. The guidelines say that I should suppress all script warnings and I definitely agree. This basically forces a certain code style on me that I'm not happy with. Maybe the editor should just not show warnings from assets, but I don't know if that is possible right now. Another possible workaround might be a more powerful annotation that suppresses warnings for whole files. |
|
Describe the project you are working on
A level editor.
Describe the problem or limitation you are having in your project
When a static function has a parameter/variable with the same name as an instance variable, the
SHADOWED_VARIABLE
andSHADOWED_VARIABLE_BASE_CLASS
warnings are emitted, but nothing is actually being shadowed.Describe the feature / enhancement and how it helps to overcome the problem or limitation
In static functions, the GDScript editor should fully ignore non-static instance variables and methods, including avoiding the warnings.
Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams
(same as above ?)
If this enhancement will not be used often, can it be worked around with a few lines of script?
It can be worked around by ignoring the warnings.
Is there a reason why this should be core and not an add-on in the asset library?
Warnings are core.
The text was updated successfully, but these errors were encountered: