-
-
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
Add access levels for class method and variable in GDScript #11039
Comments
Seems like a duplicate of #641 |
I think this consolidates #641 with some extra usages |
Personally I'd prefer protected keyword over internal. Looking at all the related PRs and proposals they too all seem to use protected. |
The more ambiguous reason is that inner may lead new gdscript users to the concept of This also mentioned me of what dalexeev tipped under that pr of mine. In some languages like Haxe, |
GDScript is a gradually typed language, i.e. it combines the qualities of both dynamically and statically typed languages. Type hints are optional and help with static analysis and performance. However, typed code must easily interoperate with untyped code. We don't want to limit users of untyped GDScript and there are no plans to make GDScript a fully statically typed language, as far as I know. So this limitation raises serious concerns about whether we should include the feature in this form. |
I really don't think we should make GDScript behaviour configurable through project settings. |
Thanks for the reply. The feature is indeed optional. It is a pure static checking for typed code without involving any runtime change. For untyped code, this feature actually has no effect and those codes will work like usual. |
Indeed. I will remove it. |
Sure. I will change |
One other change I'd like to see is pub changed to public. You're using the full word for private and protected, but abbreviating public to pub which is inconsistent and may cause confusion. |
I just pick |
While developers can opt-in to typing, they cannot opt another developer out of typing by "forgetting" the type of a variable. e.g.
But this proposal and associated PR do trivially allow me to bypass somebody else's access restrictions. e.g.
This behaviour may be surprising, especially for the author of While I like the feature, I worry about this limitation. |
reduz's initial core values for gdscript were centered around being easy to learn and fast prototyping. even though gdscript has changed over the years, i still think those values are integral to godot. if the dev needs access to variables/functions that are private in c++, they can compile from source. vice versa if they need to implement access specifiers for classes in gdscript, they can make a module or use gdnative. imo, piling on extra keywords in gdscript can make it overly complex for the developer |
This is currently the 11th most requested feature in godot-proposals and that's not even including all the duplicates of the same request. Suffice to say, the desire by the community is high enough for it to be added to the engine IMO. |
https://www.mtfca.com/discus/messages/506218/541602.png core functionality shouldn't be added just because it's popular. it should be gauged whether it's truly needed. there's already two solutions the developer can do: compile from source or use gdnative. the more stuff that gets piled in gdscript, the more bugs that arise from it (which can negatively effect godot's development). i've seen numerous threads in this section where users are wanting to turn gdscript into some god-like c++ language with STL support, access specifiers, etc. imo it goes against the entire purpose the language was designed for.. |
I've grown to like the simple and succinct Python-style underscore for private stuff, and I believe GDScript manages just fine with that. As a bonus, I like how explicit the underscore is, I can see from the function call-site and variable usage, whether the element is private. Personally, regarding GDScript, I would simply make a single underscore account for both private & protected, and not introduce any additional keywords. |
In my own GDScript code I am currently using _single_underscore for protected (as this seems consistent with GDScript's usage for _process(), ready() and so on), __double_underscore for private and no_underscore for public. It works but I do wish I had some way to enforce it for my project. I would much prefer having the option to use access modifiers rather than relying on my own consistency. Developers can become aware of issues in their code in a number of ways:
The greater the percentage of issues that can be discovered at the early end of this list the better. At the extreme ends of the list you have:
In my view, access modifiers add minimal complexity to users of the language, are easy to explain and understand, and have the potential to push a large number of code issues from late discovery to early discovery (particularly when working in teams or using GDScript libraries written by others). I don't think they are as important as ensuring type safety is always available, but I do think they would be of great benefit to GDScript and its users. I agree that popularity shouldn't be the only metric in determining the importance of a proposal, but issues like this one are popular for a reason - they have the potential to deliver enormous value to users of GDScript. |
Another point in favor of access modifiers is that Godot's current nomenclature for identifying private functions is the same as identifying virtual functions. A Perhaps something as simple as
My personal vote however is to add a
Having the functionality of the Overall, I think that at least the issue of shared nomenclature between virtual and private identification needs to be addressed, whether that be by adding new keywords or by using a different prefix for either private variables/functions or virtual functions, the latter being much easier. |
Access modifier would solve the naming concern and bring to real error-catching for invalid access, which should be pointed out and agreed. However, considering the performance on the analyzer and the flexibility of the feature, a warning system would be better, and since we have a precedent # This is a virtual method that should be overridden by a derived class.
func _my_method() -> void:
pass
# If you need to call the `_method()`, you should call this wrapper, because a wrapper may provide more encapsulation for calling your virtual method.
func call_virtual_my_method() -> void:
<encapsulations>
_my_method() Since you have more encapsulations and you can even override the wrapper to customize your methods, I don't think Note: Only personal view on this. |
I don't really mind but I'm not sure why virtual would be conflated with access modifiers. Based on their usage, the current set of built in virtual methods should all be either private or protected but that doesn't mean that ALL virtual methods should be. I don't think all public virtual methods are anti-patterns, in many simple cases a wrapper only adds bloat. I'd prefer access modifiers (and separate keywords for virtual and abstract) but if the analyzer warning is more "Godot" then that could also be a good solution. They are configurable and (so long as you remember to turn them on when you start a new project) do find errors in the editor while you write. It does make the language stricter however, as it enforces a naming convention. This is also a little inconsistent as other keywords, like "const" are used rather than enforcing constant names to ALL_CAPS. I'd be content with it all the same. Side note: I've been considering most of the built in virtual methods as protected up until now, but on reflection maybe they should all be seen as private. The answer would be a lot more obvious if GDScript used access modifiers! |
This makes sense, however as the current nomenclature stands, there is still an issue of having a private function having an identical prefix as a virtual function, where as it may not be intended for the private function to be overrode. If one were to see the I can think of some solutions that a user could do in order to mitigate this, such as commenting, or naming their functions better to convey their purpose regarding being private or virtual, but I believe a better solution without changing engine code would be to use the separate naming convention for virtual functions like the
I agree that not all virtual functions should be private, this aligns with GDScript's everything is public mindset, and also points to the issue regarding the So here is my updated idea: var my_public_value
var _my_private_value
func my_public_function():
pass
func _my_private_function():
pass
func v_my_virtual_function():
pass
func _v_my_private_virtual_function():
pass Again, this is just changing the naming conventions for identifying the desired private or virtual attribute of functions. As for internal virtual functions like ready, init, and the processes, here what they would look like, assuming they are private: func _v_init():
pass
func _v_ready():
pass
func _v_process(delta):
pass
func _v_physics_process(delta):
pass Not going to lie, I am not a huge fan of the Also for the scope of what I am saying, I think it is related to this proposal but should probably be made into its own for further discussion. |
I think we need to add a note under the documentation about saying that "a built-in method beginning with Note: And forgot to mention that if you looking into the source C++ code you will find that almost each virtual method is called via a wrapper method.
Renaming built-in methods with the prefix would lead to compatibility issues, so imo this is not a good idea... :( |
Note that unlike access modifiers, virtual methods have no runtime penalty1. We always know the base class, no matter if static typing is used or not. So we can implement The underscore would be only an indicator of a private/protected class member and would not be directly related to virtual methods. It just so happens that most native virtual methods are private/protected; you are not supposed to call these methods, at least not from the outside. Where this is not true, there are paired methods like See also:
Footnotes
|
@Brandbob & @Lazy-Rabbit-2001 Unfortunately I'd have to agree that the _v_ prefix is too long. I'd still prefer keywords anyway as they are:
I also agree with @dalexeev on the (lack of a) relationship between virtual and private methods. Personally, I think they should each be their own keyword. edit: also typing _v_ in many web forms, like this one, ends up as v and then you have to do an edit to fix it. |
Yeah it seems as though there are plenty of good proposals out there that resolve the issue I have for distinguishing private/virtual functions. And as @dalexeev said, the Therefore I strongly support the addition of either keywords/annotations regarding virtual functions. The proposals regarding the
|
Describe the project you are working on
A cross-platform app
Describe the problem or limitation you are having in your project
I think hiding class method/variable from outside may reduce potential bugs and makes code more clean.
Describe the feature / enhancement and how it helps to overcome the problem or limitation
Allow access level modifier on class method and variable.
Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams
Implemented in godotengine/godot#98606
UPDATE:
internal
changed toprotected
.pub
changed topublic
.Project setting for global default access level is removed.
Add
@public
annotation.Add warning when the script does not specify default access level explicitly.
Add keywords
public
protected
private
as access level modifier for class method and class variable and an additional keywordreadonly
for class variable.public
the method/variable can be accessed by anyone.protected
the method/variable can be only accessed by the class defining it and its subclasses.private
the method/variable can be only accessed by the class defining it.readonly
the variable can be read by anyone but can be only assigned to by the class defining it.All of the four keywords can be used as identifier.
For class method/variable without explicit access level modifier in one script, add three annotations
@private
@protected
@public
.If no such annotation is applied, default to
public
.Add
IMPLICIT_DEFAULT_ACCESS_LEVEL
warning when a script does not specify any of the three default access level annotations. Programmer may use this warning to prevent exposing script as public accidentally by forgetting setting default access level.This warning is ignored by default.
When
IMPLICIT_DEFAULT_ACCESS_LEVEL
is changed to warn/error,Limitation
This feature works only at compiling time and rely on typed code.
It has no effect on untyped code both at compiling time and at runtime.
For example, the following untyped code will compile and run without triggering any access level check.
It is up to the user to decide to opt-in this feature (by writing types and access level modifiers) or not.
If this enhancement will not be used often, can it be worked around with a few lines of script?
No.
Is there a reason why this should be core and not an add-on in the asset library?
Need to modify GDScript's compiler.
The text was updated successfully, but these errors were encountered: