GDScript: Implement soft access restriction for members prefixed with _ and __ (as warning by default)

[Lazy-Rabbit-2001]

Supersedes: #98136
Closes: godotengine/godot-proposals#641

After the discussion with some devs, with the philosophy of "implementing new features without game crashing". Here is the temporary discussion result in my opinion:

Rather than keywords and annotations, it's better to keep the original form that _ as prefix of private/protected members. There are three reasons for that:

Reasons

1. Based on @HolonProduction and KaiN's opinions, a virutal method should be private or protected, but normally it is protected, so a virtual method that begins with _ should be a protected method. At first, I thought this would be redundant, until the moment I imagined encapsulation on calling a custom virtual method:

func _custom_virtual():
    <contents>

## Wrapper and encapsulator function to call the custom virtual method:
func call_custom_virtual():
    <some preparations related to the virtual method>
    _custom_virtual()

When you need to call a custom virtual method, it is better to call it through a wrapper.
2. From the perspective of technical reasons, private and protect need to do checks during the reduction. The original pr did checks by retracing the source GDScriptParser::Node, and even to retrace not only its super classes, but also the head attributes, which is much more performance-costly. However, by operating on p_identifier->name, we do not need to trace back what its GDScriptParser::Node is and where it comes from; Instead, we just check if the class or the super classes has the member or the method. For protected identifiers, we just iterate the super classes of parser->current_class, which greatly reduces the consumption of CPU performance. Compared with the implementation of original pr, this is of balanced cost.
3. Based on the philosophy of "implementing new features without crashing the games", I turned the error into a warning. Of course you can turn it back to an error in GDScript warning settings. However, due to the fact that this pr is merely a soft implementation of access restriction, this does not break the runtime or throw an error if there is any invalid interclass access, so if your game runs out of your expectation, it's time to check your code, especially maybe somewhere you referenced a _-prefixed variable?

Implementation effect

When you trying to access a _-prefixed (or __-prefixed) identifier outside the class that defines it, a warning (by default) will be thrown at the bottom of the script editor about trying accessing a private/protected member externally.

Note: For attributes, this also take the invalid ones into consideration, e.g.:

class A:
    var _test = 1

class B:
    var a = A.new()

    func _init():
        a._tet = 2 # This is, in fact, an invalid attribute and may cause runtime error. However, this would be considered as warning of accessing private/protected member externally as well.

For protected members, this warning would be eliminated if the accessor class is derived from the class where the target identifier is declared.

Discussions

I know that this would bring to thumbups from those who think this is correct, or thumbdowns from those who think that I'm standing for official devs. No matter what this post will be given with, feel free to share and discuss your ideas here.

Plans

• Distinguishing color for _-or-__-prefixed members in auto-completion.(Off-topic)
• Distinguishing color for _-or-__-prefixed exported members in the editor inspector.(Off-topic)
• Introducing __ as the prefix of protected members (or private ones, needs to be discussed) I implement this before the For protected identifiers, we just iterate the super classes of parser->current_class. discussion on this. If this is no more needed, I'll remove this feature.
• Unit tests for members prefixed with _ and __.
• Peer reviews and tests.

[Lazy-Rabbit-2001]

See also #98606 for another alternative

[HolonProduction]

I don't think we should treat _ like private in other languages. It should behave like protected IMO.

[HolonProduction]

Dead code

[HolonProduction]

Dead code

[HolonProduction]

Please reset this change

[HolonProduction]

Dead code

[Lazy-Rabbit-2001]

Why Linux / Editor w/ Mono failed? I checked the reason but it's too weird and seems to be unreasonable

[AThousandShips]

Because it's the wrong name, you've added the same name twice, it added the one you missed

[Summersay415]

See also #98606 for another alternative

Personally I prefer this one instead of #98606.
Because, GDScript is simple language. Why you need to write "@protected/private" every time you declare a variable when you can just use a '_' prefix? And what you will do in moments when you forced to violate access limitation? Here, you can just ignore the warning. And in #98606 you can do... nothing. In GDScript there is no need for annotation complexity. Instead, we can just properly document this prefixes.

Also, I think '__' should be private, not protected, because private access is very strict, and I think it will be rarely used - protected is enough in most cases

[Lazy-Rabbit-2001]

See also #98606 for another alternative

Personally I prefer this one instead of #98606. Because, GDScript is simple language. Why you need to write "@protected/private" every time you declare a variable when you can just use a '_' prefix? And what you will do in moments when you forced to violate access limitation? Here, you can just ignore the warning. And in #98606 you can do... nothing. In GDScript there is no need for annotation complexity. Instead, we can just properly document this prefixes.

Also, I think '__' should be private, not protected, because private access is very strict, and I think it will be rarely used - protected is enough in most cases

In one word: it not only is coherent with the philosphy of godot's implementation of new features, but also greatly simplifies the code structure and maintains the code compatibility with the one in previous godot versions at the most extend. It's both technically and executabily simple, imo.

[Lazy-Rabbit-2001]

Next I want to introduce __ as private members' prefix, as someone mentioned this directly or indirectly in this post.
But whether this is needed or not still needs discussion🤔

[Lazy-Rabbit-2001]

As some of updates contains with off-topic elements, I reverted them (except coloring the private and protected members exported in the inspector, which is kinda on-topic, but i think, yes, gray color should be more effective). Meanwhile, I fixed some incorrect codes that led to pollution to some other unit tests that should not trigger the warning. However, some of unit tests does calling _-prefixed methods, for which I think I will soon add @warning_ignore() annotations for them to avoid triggering the warning and pollution the result of them.
Once the unit tests are done, I will convert this into review stage

[Lazy-Rabbit-2001]

As the unit test of GDScript got an update (that was breaking-compatibility), the unit test should be reworked as I have time available for it...

[Lazy-Rabbit-2001]

Btw, I'm considering if it will be possible to get this pr suitable for 4.5, as currently 4.4 is going to enter the stage of beta cycle.

Addition:
I just looked through the proposals and found one in which a comment (godotengine/godot-proposals#10024 (comment)) proposes allowing exported private variable to be displayable only if the node is root, which sounds like a cool idea. Maybe I'll try to implement it in one of the coming pushes.

[elenakrittik]

Addition: I just looked through the proposals and found one in which a comment (godotengine/godot-proposals#10024 (comment)) proposes allowing exported private variable to be displayable only if the node is root, which sounds like a cool idea. Maybe I'll try to implement it in one of the coming pushes.

I'm probably dumb, but that feature doesn't seem to be related to this PR's topic. Should it be implemented in a different PR instead?

[Lazy-Rabbit-2001]

Bad news:
I checked the latest commit again and it seemed that the previous commits of mine had gone nowhere. Maybe they were overriden by the latest update of gdscript source files.
Good news:
I've manually restored most of them and sooner or later there will be a commit that is going to be pushed soon.

[Lazy-Rabbit-2001]

After some discussion in the rocket chat, I'm planning to close this pr and supersedes this pr with a new one. The discussion result was that we should only leave private and public in the Godot, just like how Haxe handles it, in which private refers to protected in other languages like C#, C++, Java and Python.
The new pr will still be a soft restriction, i.e., explicitifing the naming convention and turn it into a warning system.
I closed this pr and all codes in this pr will be a reference to the new one