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

[Godot] Implement access modifiers, private and protected, to GDScript to protect a class member from being accessed externally. #734

Conversation

Lazy-Rabbit-2001
Copy link

@Lazy-Rabbit-2001 Lazy-Rabbit-2001 commented Oct 16, 2024

This is my first contribution to redot. If there is any mistake, please let me know and I'm here to hear that. :)

Trying implementing redot-proposal-#7.

This pr provides RDScript users a way to protect their members accessibility, and prevent any illegal access from external classes.
We will have private and protected keywords in RDScript soon:

Introduction of new annotations

private

The private keyword allows a member (currently a constant, a variable or a method) to be accessible only within the current class scope:

# In Class A:
private static var a = 1
# In Class B:
var b = A.a # Error: Could not access external class member "a" because it is private.
# In Class C that inherits from A:
var c = A.a # Error: Could not access external class member "a" because it is private.

protected

The protected keyword allows a member (currently a constant, a variable or a method) to be accessible only within the current class scope, or the scope of derived classes:

# In Class A:
protected static var a = 1
# In Class B:
var b = A.a # Error: Could not access external class member "a" because it is protected but accessed from a class that is not derived from "A".
# In Class C that inherits from A:
var c = A.a # OK

Common features:

  • A member modified with private or protected will hide its documentation from auto-generated doc API, even if you use ## to document it (except @export_*-ed properties and signals).
  • These two annotations CANNOT modify local members.

Spelling rules:

  • [optional access modifier] [optional static] var/func/signal/const <identifier> (Note: signal and const cannot be modified with static.)
  • static must be placed after private or protected

Note:

This pr only prevents from direct access to external members. For accessing members by reflection (i.e. by set(), get() or call() and the variants of these methods), due to potential performance impact during runtime and complexity of implementation, it's not protected. This means that this pr only provides shallow access restriction, and the following styles of access will be protected:

# Here we use variable as an example
private var a = 10
# Access directly
var t = a # Protection triggered
# Access via attribute
var t = SomeClass.a
var u = class_ref.a # All of these will trigger the protection
# Used as an expression alone or as a real paramter:
func foo(a): # Protection triggered

And allowing reflectional methods to access private/protected method is how oops languages like Java and C# works.

Remaining WIP

  • Support protection for signals. (RDScript)
  • Move protection in static analyzer into reduce_identifier() and reduce_call(). (RDScriptAnalyzer)
  • Discussion about if @private and @protected should be keywords or annotations. Allow @export_* work with @private and @protected (Having checked the documentation about whether it is better to use keywords or annotations, I decided to use keywords instead)
  • Not sure whether an invalid access should pop an error or a warning. (Warning / error system)(Thinking twice and I turned it into a warning system, which is an error by default)
  • Some fixes in reducing an identifier.
  • Hide RDScript autocompletion hint of private and protected members, or making private and protected members display in different colors in auto completion hint.
  • Unit tests in .gd and .out files. (Unit test)

Remaining WIP after being converted into Review

  • Footprint for private/protected signals and exported variables in their auto-generated documentations. (RDDoc)

Tracking after this PR gets merged:

  • More optimizations (Such as speeding up, which needs other contributors' help as I'm just a freshman in C++ programming)

Closes Redot-Engine/redot-proposals#7

@Lazy-Rabbit-2001 Lazy-Rabbit-2001 marked this pull request as draft October 16, 2024 15:24
@Lazy-Rabbit-2001 Lazy-Rabbit-2001 changed the title [Transplatation PR] Implement access modifiers, private and protected, to GDScript to protect a class member from being accessed externally. [Transplatation PR] Implement access modifiers, private and protected, to RDScript to protect a class member from being accessed externally. Oct 16, 2024
@Spartan322 Spartan322 changed the title [Transplatation PR] Implement access modifiers, private and protected, to RDScript to protect a class member from being accessed externally. [Godot] Implement access modifiers, private and protected, to RDScript to protect a class member from being accessed externally. Oct 16, 2024
@Spartan322 Spartan322 added the tracked:godot Issue already tracker on the godot issue tracker label Oct 16, 2024
@Spartan322
Copy link
Member

It is simply GDScript, also what is the reasoning for closing your Godot PR?

@filipworksdev
Copy link

filipworksdev commented Oct 16, 2024

This is great! Has anyone tested this ?

Also does this work with autocomplete?

@Lazy-Rabbit-2001
Copy link
Author

This is great! Has anyone tested this ?

Also does this work with autocomplete?

Autocomplete, or intelliguess, is planned (in the check list of the top thread). But according to original proposal, it seems that the original proposer wanted to make a ordered list of the autocomplete.
My idea is to hide unaccessible members in a external class that is going to get access to these members. But i've no idea which one will be more preferred. Needs discussion on this.

@Lazy-Rabbit-2001
Copy link
Author

Btw, currently a C# script can painlessly access a private or protected member in GDScript, but I haven't checked runtime and vm how C#'s Get() work so that I may track the flow to check its root code and find a solution...

@Spartan322 Spartan322 changed the title [Godot] Implement access modifiers, private and protected, to RDScript to protect a class member from being accessed externally. [Godot] Implement access modifiers, private and protected, to GDScript to protect a class member from being accessed externally. Oct 18, 2024
@Lazy-Rabbit-2001
Copy link
Author

I just now reviewed my Java guidebook and surfed the internet and found that one can access a private/protected member/method via reflection. So I removed two tasks about access protection from being accessed via set(), get() and call()

@Lazy-Rabbit-2001 Lazy-Rabbit-2001 marked this pull request as ready for review October 22, 2024 14:02
@Lazy-Rabbit-2001 Lazy-Rabbit-2001 marked this pull request as draft October 22, 2024 16:22
@Lazy-Rabbit-2001
Copy link
Author

Closed because i need some devs' help from godot who are expert in GDScript, plus the current setting of this pr is kinda unsatisfying.
A new pull will be posted after I find a way more proper to implement access modifiers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement topic:gdscript tracked:godot Issue already tracker on the godot issue tracker
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Godot] Add access modifiers, private and public, to GDScript
3 participants