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 public/private access modifiers to GDScript and autocompletion improvements #641

Open
2plus2makes5 opened this issue Mar 27, 2020 · 206 comments · May be fixed by godotengine/godot#98557
Open

Comments

@2plus2makes5
Copy link

Describe the project you are working on:
2.5d beat'em up

Describe the problem or limitation you are having in your project:
Inside a GDScript class i often use "private" variables and functions that shouldn't be visible ouside but that instead are "public", hence visible and usable by everyone.
More or less related is the fact that autocompletion is cluttered, yes alphabetic order is good but the list always starts with lots of constants and the function or variable we want is usually buried in lots of other things that aren't useful in that moment.

Describe the feature / enhancement and how it helps to overcome the problem or limitation:
Let us specify if a variable or function is private or protected(or whatever keywords you want to use), a private variable can only be used by the functions of the same class, protected means that it's also accessible by the classes that inherit from the class the variable is in.

About autocompletion in my opinion it would be even more useful if suggestions were grouped by class, ordered from the leaf of the class tree back to the root class, or maybe add to the editor's options the possibility to order and filter what is shown.

Describe how your proposal will work, with code, pseudocode, mockups, and/or diagrams:
var v(no access modifier means public)
protected var v(accessible only in the class it's in and the ones that inherit it)
private var v(accessible only in the class it's in)

Autocompletion suggestions would be grouped by class, ordered from the leaf to the root of the class tree, maybe with their class written somewhere.
It would be also helpful if the suggestions were colored the same way as in the editor, this way it would be even faster and easier to find what we want.

If this enhancement will not be used often, can it be worked around with a few lines of script?:
Access modifiers and autocompletion are essential features so they would be used very often and probably they can't be made with few lines of script.

Is there a reason why this should be core and not an add-on in the asset library?:
It seems to me that a GDScript feature like these ones should be core.

@Calinou
Copy link
Member

Calinou commented Mar 27, 2020

Related to godotengine/godot#18411 and godotengine/godot#23110.

@Xrayez
Copy link
Contributor

Xrayez commented Mar 27, 2020

I'm going to repeat myself but, I've always found GDScript to be very similar to C++. Yeah I know it's a weird statement but it just the way it feels, especially with static typing being a thing now. In fact when I ported some of the C++ classes back to GDScript, I had to make mostly minimal changes in order for those classes to work. Allowing access modifiers is just a natural step to making GDScript feel like C++ on the mental level. I'm mostly biased as a Godot modules developer, where I dedicate ~30% of development time to C++ programming.

@denormative
Copy link

As an experienced programmer just starting to learn godot idly, I'm also bouncing off this as a vague irritation.

I'm fine with 'public by default', and honestly prefer it, but not having a way of making this 'private' makes it more difficult 'remember' how to use the code I've written later. And yes, you can _underscore things but they still appear visible and clutter the autocomplete list even further. :(

@Shadowblitz16
Copy link

this would allow encapsulation which I support
however what about marking nodes as private, protected and public in the editor too?
this way people can hide nodes for modding

@Wavesonics
Copy link

Totally agree, public by default, but I really want to be able to mark things as private.

@Calinou Calinou changed the title Access Modifiers in GDScript and autocompletion improvements Add public/private access modifiers to GDScript and autocompletion improvements May 26, 2020
@hilfazer
Copy link

hilfazer commented Jun 9, 2020

I'd like to be able to have a variable with private setter and public getter.
Or the other way around.

@hilfazer
Copy link

hilfazer commented Jun 11, 2020

There's one detail i'd do differently: i wouldn't use the verbose syntax like the one in C# but rather the less verbose syntax of C++

public:		# can be omitted since it's the default
var all
var my
var Public
var vars

private:
var All
var My
var Private
var Vars


protected:
	
func _init():
	pass
	
func _ready():
	pass

public:

func my_public_function():
	pass

To avoid typing extra keyword before every single var/func. It works better when the code is grouped by access modifier which is recommended by GdScript Style Guide.

@Calinou
Copy link
Member

Calinou commented Jun 11, 2020

@hilfazer I'm not really fond of disconnecting the access modifier from the variable/method name. It makes it harder to visually check an individual member's access modifier. Also, if you copy-paste a snippet around, the access modifier will be lost.

@Byteron
Copy link

Byteron commented Jun 25, 2020

I am very much in favour of having access modifiers in gdscript.
all public is fine for small games / prototypes, but for more serious projects the absence of those become a burden.

also, the good old SOLID principles are virtually impossible without access modifiers.

@Calinou
Copy link
Member

Calinou commented Jun 25, 2020

also, the good old SOLID principles are virtually impossible without access modifiers.

You can have de jure access modifiers via annotations or conventions and still obey SOLID principles. They don't have to be enforced by the compiler (that's how I see it at least).

@me2beats
Copy link

private methods are vital when creating plugins.
I also vote for the implementation of the protected methods in gdscript

@MikeSchulze
Copy link

GD script definitifly needs such option to mark a class/function/members as private/proteced or by default as public.
I'm current developing an plugin and all my classes are listed in the available class list. This is unwanted because this classes only internall stuff to work within the plugin.

A Project will be unmaintainable when the amount of classes increases.
Also the fact to prevent direct access to members is very important.
To only using a prefix "_funcname" not prevents others to use this functions/members.

Please add private and protected modifiers to can manage such stuff, this will help a lot.

I prefear to use like in common languages

private func foo():
protected func bar()
# or 
func_private foo():
func_protected bar()

like the 'static' keyword
same for classes e.g.

# private class not visible outside the package and not able to inherit from
class_name_private Foo
extends Resource
..

# protected class not visible outside the package 
class_name_protected Foo2
extends Resource
..

# public class 
class_name Bar
extends Resource

Best regards Mike

@Shadowblitz16
Copy link

Shadowblitz16 commented Sep 30, 2020

if this was implemented I think the best way to go about it is make everything protected by default. that way people can choose what they want to expose.

@me2beats
Copy link

me2beats commented Oct 1, 2020

if this was implemented I think the best way to go about it is make everything protected by default. that way people can choose what they want to expose.

I don't think you want protected for most of the methods and properties. At least you wouldn't want this for most utility methods and singleton methods and properties. Otherwise, the codebase would be full of public keywords.
It would also require a more thoughtful code organization and architecture, which would be difficult for beginners to do.

@Wavesonics
Copy link

@me2beats agreed, I think public by default makes the most sense. It gives beginners the save experience as today.

@Shadowblitz16
Copy link

Shadowblitz16 commented Oct 2, 2020

I strongly disagree.
Then you have a bunch of members you might not want modders to use.
protected is there for the purpose of the derived object to use and then choose what to expose.
if you expose everything your taking away control from the developer which is not people want when creating their own games

EDIT: also I understand thinking about beginners, but godot tends to take away control from the user to make it simple for beginners. and while making it easier for someone to use is good, taking away control is worse.
and I honestly think this is one of godot's biggest flaws right now.

@Wavesonics
Copy link

@Shadowblitz16 I'm saying public by default, but with private and protected as explicit options.

@MikeSchulze
Copy link

@Shadowblitz16 public should be the default, private and protected be optional

@kintrix007
Copy link

kintrix007 commented Oct 10, 2020

I really do want to have private members.

My biggest problem is the following:

class ParentClass:
  var a = 10
  var random_var = a/2 # I want to make this private

class ChildClass extends ParentClass:
  var random_var # throws an error, since it already exists in parent class.
  # I don't want to limit all the child classes just because I needed a helper variable for something

Also, it would be great if I could make a variable private in the following example as well.

class Enemy extends Node2D:
  var max_health : int = 100
  var health := max_health setget _health_changed
  var health_percent : float = 1.0 # This is the variable I want to make private
  
  func _health_changed(val : int):
    health = val
    health_percent = float(health) / max_health

  func _process(delta):
    scale.y = health_percent
    # Other uses of health_percent

This health_percent variable should not be accessed by its children. And I would like to make sure it isn't.
Also, it would show up in the auto-completion, which is just annoying.

@Shadowblitz16
Copy link

Shadowblitz16 commented Oct 26, 2020

I think something like this should be done..

#allow accessing friend members in Sprite and AnimationPlayer
extends Node2D friends Sprite, AnimationPlayer

pub var my_public_variable
pro var my_protected_variable
pri var my_private_variable
fri var my_friend_variable

pub func my_public_function(): pass
pro func my_protected_function(): pass
pri func my_private_function(): pass
fri func my_friend_function(): pass

It seems more pythonic since the function and variable keywords are shortened I don't see why these can't be
its also easier to type

@kintrix007
Copy link

kintrix007 commented Oct 26, 2020

I think something like this should be done..

#allow accessing friend members in Sprite and AnimationPlayer
extends Node2D friends Sprite, AnimationPlayer

pub var my_public_variable
pro var my_protected_variable
pri var my_private_variable
fri var my_friend_variable

pub func my_public_function(): pass
pro func my_protected_function(): pass
pri func my_private_function(): pass
fri func my_friend_function(): pass

It seems more pythonic since the function and variable keywords are shortened I don't see why these can't be
its also easier to type

I'm not really against this one... But I'm not sure about the readablity either.
I understand that with a long line like this it might help

pub export(int) onready var things

vs

public export(int) onready var things

But it's already kind of long.
I'm nore on the side of typing these out.

However I still like the C++ style the most. Although people don't seem to like it as much.
But that would decrease the typing the most, and with some indenting it's even readable.

That being said, if this was the final choice, I wouldn't mind that much...

@hilfazer
Copy link

hilfazer commented Nov 5, 2020

I just saw some code on Discord which reminded me of this issue. The code:

GodotLotsOfOnreadyVars

Imagine putting private or protected in front of each of these declarations.

@Jummit
Copy link

Jummit commented Nov 5, 2020

Imagine putting private or protected in front of each of these declarations.

This is an instance where #641 (comment) would make it more readable.

My stance on access modifiers is that they are not necessary for following OOP standards (In my time using Godot I never had a case where I needed them. They only add visual noise, the information they provide can be easily added by using underscores:

private var health = 10 setget set_health
public var name = "name"

private var inventory = []

public func set_health(to):
  health = to

vs

var health = 10 setget set_health
var name = "name"

var _inventory = []

func set_health(to):
  health = to

One actually useful keyword would be virtual, which would notify you if you didn't override a certain method.

@Calinou
Copy link
Member

Calinou commented Nov 5, 2020

One actually useful keyword would be virtual, which would notify you if you didn't override a certain method.

This can be implemented using a @virtual annotation. I think the same should be done with @private eventually.

@Shadowblitz16
Copy link

Shadowblitz16 commented Nov 6, 2020

Too many annotations are a bad thing, it makes code ugly and unreadable.
Access modifiers and things like virtual and abstract should not be annotations.

@Meorge
Copy link

Meorge commented Jul 10, 2024

I dunno how many people (if anyone) was watching this eagerly, but just in case since I said I was going to work on it:

I put this project on hold for now. tx350z's and JojOatXGME's notes made me realize there were flaws with my implementation, especially with the fact that I was comparing on a per-object basis rather than a per-class basis (i.e., different instances of the same class couldn't access each others' private members). That, along with the fact that it does seem like a pretty big change to the language that would need feedback and support from members of the GDScript team, made me feel like it wasn't worth it to continue working on right now. I wouldn't want to spend too long on an implementation only to be told that it doesn't fit well into the existing code.

Perhaps a little later, when I've got fewer active projects on my plate, I'll take another stab at it, propose my implementation to the GDScript team, and see how things go from there?

@anniryynanen
Copy link

I'm still watching this proposal eagerly. I'm in a similar boat of wanting to work on it, but not having a lot of experience with the codebase.

While you're taking a break, I'll take a shot at making it work. See how far I can get. It seems that people are happy with the idea of the @private annotation so that's certainly progress.

@Meorge
Copy link

Meorge commented Jul 10, 2024

Sounds good - I wish you luck! 😄

One of the things I learned/saw, I think both here and in the Godot Contributors RocketChat, was that people seemed to prefer it being a keyword rather than an annotation, so that may be something to consider. That being said, I imagine the core behavior of refusing access when the callee and caller types are different should be the hard part, so switching that behavior from an annotation to a keyword should be relatively trivial?

Anyways, good luck again!

@anniryynanen
Copy link

One of the things I learned/saw, I think both here and in the Godot Contributors RocketChat, was that people seemed to prefer it being a keyword rather than an annotation, so that may be something to consider.

That's good to know! I'll start working on a keyword then.

@anniryynanen
Copy link

Implementing the private keyword or @hidden label was discussed in the contributors chat, and the conclusion is that a leading _ is the preferred way to mark private members.

@produno
Copy link

produno commented Jul 10, 2024

Implementing the private keyword or @hidden label was discussed in the contributors chat, and the conclusion is that a leading _ is the preferred way to mark private members.

What about private methods? We already use the _ there. Does that mean we will never have private methods? Declaring a private method to a private variable will be different? Or we change the current use of an underscore in methods?

All of the above is bad imo. The options have already been wildly discussed here with lots of very good points against using an _.

@anniryynanen
Copy link

anniryynanen commented Jul 10, 2024

Underscore for both methods and variables. I would have been happier with an annotation or keyword, but at least this can be used as an official convention.

Clipboard - July 10, 2024 15_48

@anniryynanen
Copy link

@Meorge
Copy link

Meorge commented Jul 10, 2024

I've found that another (albeit perhaps minor) downside of prefixing private members and properties is that, if you decide to change something from private to public or vice versa, the symbol itself has to change. And when you change the symbol, you have to through your code to fix all the instances of it. VS Code is smart enough to be able to rename all the instances of the symbol, but last I recall, the built-in Godot editor only has a simple string-match capability. At the same time, Godot users are used to that prefix, and being able to recognize script-internal symbols from other places in the code at a quick glance is nice.

I would be a bit concerned with tying behavioral changes to the use of the underscore as the first character. Removing it from autocomplete when in another class would be relatively okay, I think, but making the editor throw errors could break existing projects where people have "cheated" and referenced private symbols from other classes since they weren't enforced. Maybe the editor could have an opt-in toggle for "strict control access mode" that enforces underscored symbol access restrictions?

@anniryynanen
Copy link

There won't be any editor changes, if I'm understanding things correctly it's purely a naming convention. Or at least I don't see any point in having editor support for hiding things with a leading underscore.

@produno
Copy link

produno commented Jul 10, 2024

Underscore for both methods and variables. I would have been happier with an annotation or keyword, but at least this can be used as an official convention.

The current style guide is for what we currently use. For example using an _ and not using the variable in that class, will throw a warning. So this works currently. But if this proposal is implemented using an _, how will my code know if my method is private or supposed to be overridden? How will auto complete know I have a method in a class that is expected to be overridden if its now been hidden because it thinks its private?

Or we just state only built in functions can be overridden which are excluded? But to me it still seems very awkward. Others have also expressed they don't use underscore (myself included) because they are very hard to see or read.

There are some that want to use an _ because prefixing a var with more text is quite verbose. So if that's case, what would be the thoughts on excluding the var keyword if using the private keyword?

@anniryynanen
Copy link

anniryynanen commented Jul 10, 2024

I'm sorry everyone, I misunderstood the intent in the contributors chat. The desired feature seems to be to hide things with a hidden underscore, in some situations at least. I'm stepping down from being the implementer, if anyone wants to do that it's up for grabs again.

@Meorge
Copy link

Meorge commented Jul 10, 2024

No worries, thanks for looking into it! It sounds like there's still not a great consensus on how exactly it should work or be triggered 😅

@shinspiegel
Copy link

Implementing the private keyword or @hidden label was discussed in the contributors chat, and the conclusion is that a leading _ is the preferred way to mark private members.

This isn't a "solution",
Support for private on the language is hugely useful for lots of projects, and with the influx of people coming, having these options would benefit people who make addons and work in groups.

@kotx
Copy link

kotx commented Jul 11, 2024

I'm for automatically hiding variables with a _ prefix (Go is similar with Uppercase-public lowercase-private); but won't this break existing scripts that access variables prefixed with _? Unsure if Godot has any compatibility guarantees in GDScript, so maybe this is ok.

@shinspiegel
Copy link

But Go does have "public/private", it just does not use a keyword and uses the naming for that. Doing the same on the GDScript would not work, since the _ means "private" or "virtual".
And since it can be virtual, we never know if this is a real private or just a virtual method.

@tx350z
Copy link

tx350z commented Jul 11, 2024

Another $0.02.

I've always questioned the seemingly inconsistent style guide recommendation to use '' prefix to denote "private" properties and methods since Godot also uses the '' prefix for virtual method overrides which are similar in concept to "protected" in fully OOP languages.

In most OOP languages private, protected, and public support full implementation of OOP encapsulation. These access (or scope) modifiers are primarily implemented during the symbol parsing and resolution phase of compilation (or byte-code generation). For example, class-private properties are conceptually the same as local variables defined within a method. It's really that simple (yes I know local variables are commonly stored on the stack - that's an implementation detail unrelated to this discussion).

Use of a "keyword" or a "decoration" to designate scope is semantic and explicit. Using '' is implicit and potentially creates edge cases that complicate implementation and code readability. For example, is a method prefixed with an '' a private method or an override of a 'virtual' method?

Somewhere in this thread is a recommendation to use optional @Private and @public decorations with @public being the default. This approach is non-breaking to existing code and requires extra typing ONLY if you want to use private properties and/or methods, or want to be explicit about scope by tagging important properties/methods. If you don't want to type those few extra characters where needed, you can completely ignore that modifiers exist in the same way you can ignore variable and method type hints.

Ok, that's probably more like $0.20.

@Mickeon
Copy link

Mickeon commented Jul 11, 2024

Please use backticks `Like this` to avoid pinging users on Github when writing annotations. @private @public.

Sidenote, it is generally agreed upon that if private itself is implemented (and not an annotation with a similar purpose), it should be a keyword and not an annotation because it is a very commonly understood keyword in a programming context.

@freehuntx
Copy link

freehuntx commented Jul 31, 2024

A current workaround could be to internally use get_meta/set_meta and externally define a getter.
E.g.

class Foo:
    var bar: String:
        get: return get_meta("bar")
    
    func _init():
       set_meta("bar", "5")

func _init():
    var foo := Foo.new()
    print(foo.bar)
    foo.bar = "5"
    print(foo.bar)

Maybe some tooling could polyfill or transpile an @private annotator to replace any write/set to that schema.
But im not sure about performance since it adds extra calls to any read/write operation.

@WhalesState
Copy link

WhalesState commented Sep 13, 2024

# Public by default, for compatibility.
var public_one: int

@private
var private_one: int
var private_two: int

@protected
var protected_one: int

@public
var public_two: int


public func public_func():
    pass


private func private_func():
    pass


protected func protected_func():
    pass


func public_by_default():
    pass


public func _ready():
    # Print Error, can't use access modifiers for overriden functions.
    pass

@Meorge
Copy link

Meorge commented Sep 18, 2024

I agree that

@private
var private_one: int
var private_two: int  # This is also private

doesn't make sense to me - I would expect private_one to be private, and private_to to be public since it doesn't have an annotation immediately before it, in much the same way that

@export
var exported_var: int
var non_exported_var: int

works.

Being able to repeat annotations across multiple statements could be nice. For example, if you had several variables you wanted to export, you could do

@export:
    var exported_1: int
    var exported_2: int
    var exported_3: int
    var exported_4: int

or something like that. I'm not sure how I feel about the bracket syntax, since GDScript's syntax is overall more Python-like, which uses colons and whitespace to denote this sort of nesting.

Regardless of the precise syntax though, I think repeating annotations like this would make more sense as an independent proposal that this one could benefit from.

@AThousandShips
Copy link
Member

That's already covered in:

So let's not focus on that as that would be covered by that proposal

@WhalesState
Copy link

WhalesState commented Sep 18, 2024

Was thinking about how to implement this without adding an extra property usage.

for property in object.get_property_list():
  if property["access"] == PROPERTY_ACCESS_PRIVATE:
    print("private")

We can introduce this for properties only at first, it will be usefull to prevent users from editing core variables when making a plugin or a mod in gdscript, for example in my project, a plugin can access all the project classes and it can easily break the whole release if they messed up with the private variables since most of them gets saved after any change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: On Hold