GDScript: Property (get/set) style guide does not make it clear that extra work is being done

[codevogel]

The issue

Since there is no stylistic distinction between a property and a variable, it is hard to tell whether extra work is being done. Properties can perform extra work to get their value, and are more expensive than just referencing a variable. So it would be best to cache the result of a property, instead of directly accessing it three times in the same function.

Can you tell which one of these is a property and which one is just a variable?

An example

Say I have the following class:

class_name MyMap

var tiles: Array[Tile] = []
var map_name: "My map name!"
var connections: int:
   get:
      return neighbours.keys().reduce(func(acc, num): return acc | num, 0)

Now, in other scripts, I can see no difference between whether I am accessing a variable tiles or a property second_to_last_tile.

e.g. this variable access:

func do_something(map: MyMap):
 for n in 100:
    print(map.map_name)

Looks pretty much the same as this property access:

func do_something(map: MyMap):
  for n in 100:
     print(map.connections)

But it would be much more expensive than

 func do_something(map: MyMap):
   var connections = map.connections
   for n in 100:
      print(connections)

As the latter example doesn't have to perform the size calculations and if checks each iteration.

Proposed solution

• You could add a rule to start properties with a p_ prefix.
• Or maybe access properties through .. instead of .
• Or maybe use PascalCase (this would still conflict with types, but it'd be less confusing given that a Type is not used as a value)
• Or something that you come up with that is better than the above

Honestly, I'm not too happy with any of my solutions, they feel too intrusive, but here's three things I came up with that might more realistically help address this:

• It would be a good start to add highlight groups to properties and color them differently from regular methods. But that is not very accessibility friendly.

• It would be good if there were different icons for variables and properties:

  

• An 'expensive invocation' check could be added to the linter. I know that when I was developing in Unity they had such a linting option - it would warn me if I accessed a result from a function or property in a function twice, and recommended me to cache these into a local var. This still wouldn't solve the issue of not being able to discern between props and vars in a non-editor setting, like reviewing pull requests on the Github website.

[Calinou]

While I understand there's appeal to languages with no hidden control flow, this can be in direct opposition with concise, expressive code.

I think all we can do here is use a different icon for properties that have a getter/setter assigned to them in autocompletion.

Built-in memoization methods could be interesting to have, but I haven't seen any language that offers them in their standard library. Some languages have parsers that can perform it automatically, but this only has a limited scope of applications.

• You could add a rule to start properties with a p_ prefix.

This rule is internally used in Godot's source code for function parameters. Either way, the current GDScript style guide doesn't advocate for Hungarian notation, so I don't think this would be a good fit for the style guide.

Requiring .. to access properties would break compatibility with existing scripts, not to mention I've never seen a language that uses .. to access properties. Lua uses .. for concatenation and people already find that weird enough 🙂

An 'expensive invocation' check could be added to the linter.

Not all getters are slow though. This linter would be complex to create in an accurate manner, as the time it takes for the getter to run depends on the current property value, the CPU speed, the build options used for the engine binary and so on. We can't base such a warning based on a time taken in milliseconds, as it would mean the warning would only be emitted on certain machines and not others.

[codevogel]

First off, I wholeheartedly agree that _p ..prop or PascalCase aren't suitable solutions. Maybe someone can come up with something better.

I think all we can do here is use a different icon for properties that have a getter/setter assigned to them in autocompletion.

That would be a great start. Assigning them a custom highlight group could increase readability even more.

Built-in memoization methods could be interesting to have

That seems cool, but I could also see it open a whole lot of complexity, and confusion (memoization can feel like black box for the end user) for little return.

Not all getters are slow though. This linter would be complex to create in an accurate manner, as the time it takes for the getter to run depends on the current property value, the CPU speed, the build options used for the engine binary and so on. We can't base such a warning based on a time taken in milliseconds, as it would mean the warning would only be emitted on certain machines and not others.

I'm definitely not proposing an expensive invocation warning that 'measures performance'. That, as you pointed out, would be pretty much impossible to get right. It instead should warn you when you call access the same property more than once without caching the result, which might be expensive. Here is a good article on why Jetbrains implemented these into Rider for Unity. Here and here is some more info on hinting/warning at expensive invocations.

I think that's why a linting warning would suffice. You can ignore the warning or suppress it where need be (where you are sure you are accessing light calculations), or you are nudged into just working around the warning by caching the variable regardless of whether the computation is expensive or not - which would be in most cases be a good practice anyway.
I think that's pretty much the purpose of these types of linter warnings: they push you to the 'right' thing, without enforcing you to do that thing.

[Meorge]

I may be in the minority on this opinion, but personally I like the idea of (at least the naming conventions) not necessarily being different for properties versus variables. In my mind, properties are like "magic variables", that I can treat from outside the black box just like I would a variable, except that when I interact with them, the black box is smart enough to do extra things. Making it more obvious via naming convention differences that the variables and properties are different things would break this "illusion". It'd probably start to feel (to me at least) that the properties are really just getter/setter functions, when I thought their whole purpose was to look like a variable from the outside.

[dalexeev]

I think one of the advantages of properties is that you can turn any field into a property without replacing it with a method. For example, you changed a logic and decided to keep the old variable for compatibility, adding a setter and getter to it. Requiring a variable to be renamed when a field becomes a property or vice versa is not consistent with this approach.

Also, you can temporarily add a setter to a field for debugging purposes.

And of course, the size property is a bit nicer to use than size() or get_size() methods.

[codevogel]

I would be totally fine with keeping the naming the same, as long as you can differentiate between them in another way.

The 'magic variable' thing is nice, but also exactly what could cause performance issues if you're not careful - as shown in the loop examples in the OP.

[dalexeev]

So it would be best to cache the result of a property, instead of directly accessing it three times in the same function.

Static analysis of whether a getter has side effects is extremely difficult to implement for GDScript right now. Yes, a getter should generally not have side effects, but we can't rely on that. It's ok to have side effects that don't affect the main logic, like printing debug information. Also, a getter can have complex logic where you compute a value through other field(s), so that requires more complex control flow analysis.

I think that function inlining is a more feasible optimization than control flow constancy analysis. It would be useful not only for setters/getters but also for other short functions, since calling a GDScript function is quite an expensive operation.

And of course I agree that we should add a note about performance in the documentation, plus we could display fields/properties differently in autocompletion.

[codevogel]

Static analysis of whether a getter has side effects is extremely difficult to implement for GDScript right now.

Again, I am not asking to perform a static analysis on the code inside a property, I am just interested in seeing a linting warning when you refer to a property twice without caching the result. That should be doable, no?

I think that #3760 is a more feasible optimization than control flow constancy analysis

If done automatically, maybe. I don't think I like the idea of having to add keywords to apply optimization rules. Caching the result into a local variable is a lot clearer in what it does, it is less of a black box solution, if you ask me.