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

Inconsistent type styling in Godot editor #44922

Closed
CitrusWire opened this issue Jan 4, 2021 · 18 comments
Closed

Inconsistent type styling in Godot editor #44922

CitrusWire opened this issue Jan 4, 2021 · 18 comments

Comments

@CitrusWire
Copy link

CitrusWire commented Jan 4, 2021

Godot version:
3.2.3

Issue description:
The types: int, bool, and float are all styled as keywords rather than types in the editor:

image

This introduces confusion to new users.

I'd also suggest int, bool, and float should be capitalised to be consistent with the rest of the types. One of the reasons that PHP has such a poor reputation in many circles is because of its lack of consistency. For example its naming of substr or str_replace and countless other string functions.

Edit: enum type too?

@YuriSizov
Copy link
Contributor

YuriSizov commented Jan 4, 2021

I'd also suggest int, bool, and float should be capitalised to be consistent with the rest of the types. One of the reasons that PHP has such a poor reputation in many circles is because of its lack of consistency.

This has nothing to do with consistency and not the same as PHP's problems. Primitive types are not capitalized in most C-like languages, while classes can be capitalized as per individual style guide. If anything, Array and especially String being capitalized is inconsistent. But I guess this is borrowed from the engines C++ core.

@akien-mga
Copy link
Member

akien-mga commented Jan 5, 2021

int, float and bool are plain old data types (Edit: primitive types is more correct, POD types is a wider and potentially more confusing concept). Array and String are templates, which are not trivial. So it makes sense for templated types to be capitalized like other non trivial types (e.g. Object).

@CitrusWire
Copy link
Author

I would ask you both: from the end user's perspective do they care whether something is a primitive type or a class type? That seems like an academic difference that's only pertinent to the engine devs, not to the vast majority of game developers.

In python behind the scenes everything is a Class, but to the users they see types, classes, even functions. The built-in types are all consistently lower-case (str, dict, tuple...) with the exception of None which is also a special value (as True / False).

It's hard to understate the value of consistency for a developer.

@akien-mga
Copy link
Member

akien-mga commented Jan 5, 2021

It's not a matter of engine devs, it's very common in most programming languages that primitive types are identified as such with their case, as they typically have properties which different from non-primitive types (passing by value vs reference, copy/clone, etc.).

For example, C# uses PascalCase for classes but its primitive types bool, float, int are lowercase.

Another example: Rust primitive data types are lowercase: https://learning-rust.github.io/docs/a8.primitive_data_types.html
But it's non-primitive data types (e.g. structs, enums) are PascalCase.
In Godot, types like Vector3 or Color are not primitive data types (they're collections of primitive data types, here float) and in a language like Rust, they'd typically be represented by structs (in C++ they're classes).

There's no inconsistency here, this is on purpose, and this has not been a problem in the past 6 years for Godot users, whether experienced or beginners.

@YuriSizov
Copy link
Contributor

I second Akien about capitalization, but I want to remind everyone that the first part of this issue is about syntax highlight 🙂

So to address that... Of the top of my head I cannot remember if any other language in any other editor highlighted primitive types the same as keywords or not, but this can probably be arranged anyway on our end. We can make a color for primitives a separate setting and let users decide if they want it to be the same as keywords, as classes, or have a unique color all together.

@CitrusWire
Copy link
Author

@akien-mga - I see your C#/Rust examples, but isn't Godot meant to be Python like?

as they typically have properties which different from non-primitive types (passing by value vs reference, copy/clone, etc.).

Is this distinction a thing in Godot? As far as I can see the Class types are a mixture of Reference (i.e. Array) and Value (i.e. PoolStringArray).

As a end-user why do I care about whether something is primitive or classed? I can't see much of any difference, and if there is a different I'd suggest it should be explicitly documented in the docs rather than hoping the dev intuits it from capitalisation.

Of the top of my head I cannot remember if any other language in any other editor highlighted primitive types the same as keywords or not

PyCharm groups all built-ins which includes types (str) and functions(len()); they're distinct from keywords.
I prefer Godot's way in that regard in that types get their own style.

@dalexeev
Copy link
Member

dalexeev commented Jan 5, 2021

I'd also suggest int, bool, and float should be capitalised to be consistent with the rest of the types.

I've already suggested. Also, it would solve the first problem automatically.

There's no _in_consistency here

No, there is inconsistency with the following naming conventions:

var name: Name
func name(name: Name) -> Name
name is Name
name as Name

All types except these three have a capitalized name. Any exception is inconsistency. The only question is whether there are any advantages to having the exception.

this has not been a problem in the past 6 years for Godot users, whether experienced or beginners

This is only because this inconsistency affects only appearance, but not behavior. Just like the fact that these types are highlighted in a different color than the rest.

We can make a color for primitives a separate setting and let users decide if they want it to be the same as keywords, as classes, or have a unique color all together.

Sorry, but in my opinion this is useless. All types should be highlighted with the same color. Otherwise it will be too colorful: primitive types - in one color, built-in types - in another, classes - in a third, custom classes - in the fourth, 2D types - in the fifth, 3D types - in the sixth, etc.

I would ask you both: from the end user's perspective do they care whether something is a primitive type or a class type?

Absolutely agree. What does knowing whether the type is primitive or not? What is the difference between a primitive type and a non-primitive type? I could understand if types passed by value were written with a small letter, and types passed by reference with a capital letter. Or built-in types with small, and classes with capital. The only reason these three types are written with small letters is tradition, habit, and blind copying of C/C ++.

it's very common in most programming languages

Yes, you are right, it is, although it is absolutely pointless.

But in some languages (Kotlin, Haxe, Ada, etc) all types (even "primitive" ones) start with a capital letter, and this warms my perfectionist soul.

But since it does not affect behavior, this is not a change I would fight for. It's just a little annoying that people think in such fictitious categories as “primitive” types. In comparison, the illogical and counter-intuitive behavior of the division operator creates much bigger problems, but people don't want to notice them and react extremely negatively due to the strength of their habits.

@dalexeev
Copy link
Member

dalexeev commented Jan 5, 2021

primitive types are identified as such with their case, as they typically have properties which different from non-primitive types (passing by value vs reference, copy/clone, etc

In this regard, in GDScript bool/int/float behave exactly like String, Vector2, Packed*Array and most other types, i.e. passed by value. Only Array, Dictionary and Object are passed by reference.

@YuriSizov
Copy link
Contributor

Also, it would solve the first problem automatically.

No, it wouldn't. The way GDScript tokenizer works in terms of syntax highlight has nothing to do with the capitalization.

Sorry, but in my opinion this is useless. All types should be highlighted with the same color. Otherwise it will be too colorful: primitive types - in one color, built-in types - in another, classes - in a third, custom classes - in the fourth, 2D types - in the fifth, 3D types - in the sixth, etc.

This is not what was suggested. I only propose we solve the problem presented here with a separate option for primitive types. By default, it would be the same color as keywords, as it is now, but whoever would like to highlight them differently, be it matching the color of classes or another color, would be able to do so. In the end, it will be as colorful as each individual user wants, which is fitting, as this issue is about individual user's preferences.

@akien-mga
Copy link
Member

Again, there's two things in this issue:

  • The original report that primitive types are wrongly colored. That's a bug and I'm not even sure why it warrants discussion, they should be colored like any other type. PR welcome, that's likely easy to fix.

  • Renaming primitive types to Int, Bool, Float -> https://github.com/godotengine/godot-proposals
    I'm not interested in discussing this further here for the sake of perfectionism.

@dalexeev
Copy link
Member

dalexeev commented Jan 5, 2021

No, it wouldn't. The way GDScript tokenizer works in terms of syntax highlight has nothing to do with the capitalization.

As far as I know, the problem is not in the tokenizer, but in the mechanism for highlighting the code of the TextEdit class. It's just that int is highlighted like a built-in python-like function (like len and str) and this overrides the type highlighting. If we split it into int (function) and Int (type), then the problem will be solved automatically.

There are no bool(), int(), float() functions on the @GDscript documentation page (although they are highlighted as built-in functions), in fact they are built-in type constructors like String() and Vector2(). It's funny that there is no difference between str() and String(), apparently str() was created to make GDScript look more like Python.

This is just a note, actually the problem is broader, for example load is always highlighted in red, even in func load() and self.load.

This is not what was suggested. I only propose we solve the problem presented here with a separate option for primitive types.

Does the user have a real need for this? Why do we need to provide the user with the ability to highlight "primitive" types with a different color than non-primitive ones? Maybe we need to add an option to change the color for each type? I mean, primitive types aren't that different from non-primitive types that they deserve a separate color.

@dalexeev
Copy link
Member

dalexeev commented Jan 5, 2021

@akien-mga I respect your efforts to maintain order in discussions, but the two issues are related. I tried to count the differences between primitive types and non-primitive types and counted two differences:

  1. Primitive types are highlighted in a different color (recognized as a bug).
  2. Primitive types are written with a lowercase letter (a tradition of many C-like languages).

I did not find any more differences (maybe someone will find it). Please note that both differences concern only appearance (color and case).

@name-here
Copy link

name-here commented Jan 5, 2021

Primitive data types have a few properties different from other data types:

  • They don't act like objects, and so don't have functions (things like Vector2.normalized())
  • Faster to move around and perform operations on

Also, I was just messing around in GDScript, and it seems anything inheriting from Object (Reference, Node, etc.) isn't reserved, allowing you to do things like var Control : int = 10, which prevents you from accessing Control until your variable goes out of scope (can't do Control.new(), for example).

@dalexeev
Copy link
Member

dalexeev commented Jan 6, 2021

@name-here These are not significant differences, just implementation details. In JavaScript, for example, you can do 1.2345.toFixed(2). GDScript is a high-level language, so the low-level details of C++ are not important here enough to emphasize these three types. Knowing about their primitiveness in C++ does nothing for a GDScript developer.

Once again, this is not a thing that I would fight for. I refer to it as "It would be nice." I just wanted to show that "primitive" types are a strange tradition. But I face the objection that there are some real reasons to classify primitive types into a separate category. It surprises me.

@name-here
Copy link

It GDScript really all that high-level? It basically just interfaces with Godot's built-in C++ classes (a limited subset, at that), with python-like syntax and a few handy shortcuts like iterating over arrays more easily. And since GDScript does not exist outside of Godot—and never will—implementation details are important, since GDScript is its implementation, nothing more. There is nothing outside of the implementation that dictates what GDScript is supposed to be, or how it is supposed to work (the docs are only meant to describe what it is). This is a major difference between GDScript and "mainstream" languages.

GDScript has a lot of ties to the C++ of the engine it interacts with—those "implementation details" being some of them. If Godot used semicolons and braces instead of line breaks, colons and indents, would you feel the same way about this?

The real problem, though, is that any potential benefits of a change do not outweigh the obvious costs. True, there's no reason GDScript has to use "int" instead of "Int", but changing it will break every current Godot coder's code and intuition with no real benefit. One reason it may have ended up this way is that it mirrors the names of those things in C++ (what the engine devs are seeing all day), where all the types added by the engine are PascalCase, and primitive types are all lowercase. Since the C++ functions GDScript interacts with all use the C++ built-in bool, int and float types, and Godot's own Vector2, Variant, Array, etc. types, the current situation makes GDScript code nicely mirror the same code in C++. This isn't very useful to those who only use GDScript, but is nice for anyone also using other languages with Godot. So I'll have to disagree with your that "it would be nice" to change it.

In general, I'd say the engine's syntax highlighting (and, relatedly, which things in GDScript are reserved words) is a little strange in 3.x (why are built-in functions like print and floor reserved?), but I just did some testing, and it seems master doesn't really have globally reserved words (probably because of the parser rewrite). It does, however, still have weird highlighting if for some inexplicable reason you do var Node or var int (Node is highlighted green and int red when they should be white like other variable names).

@dalexeev
Copy link
Member

dalexeev commented Jan 6, 2021

GDScript is a high-level language because there are no pointers or other low-level memory manipulations. In terms of design and syntax, GDScript is a mixture of Python, JavaScript, and C++. But some things taken from C++ don't fit well with the fact that GDScript is a language whose goal is to be simple for beginners. I disagree with the opinion that GDScript should look familiar to C++ developers.

This change is highly optional and insignificant, as most people don't care. But even if it were accepted, the old code would be easy to fix automatically, unlike some of the other recent GDScript changes.

To summarize: in GDScript, primitive types are written in lowercase only because they are written this way in C++. There is no other reason.

@CitrusWire
Copy link
Author

@akien-mga - I disagree that there are two things here. There's two flavours of the same thing. Put simply:

Are primitives a special type of Type?
Yes = Then different styling and capitalisation could make sense
No = Then they should be styled and capitalised the same way as other types

This is not a "perfectionism" thing. This is a consistency thing. It's in the same vein of why Godot has a contributors style guide: Because anything that reduces the cognitive load on a developer is a good thing and consistency does that.

Now personally I don't see how primitives are special to GDScript users, but @dalexeev has done a better job covering that than I did.

@akien-mga
Copy link
Member

Are primitives a special type of Type?
Yes = Then different styling and capitalisation could make sense
No = Then they should be styled and capitalised the same way as other types

For the last time: YES, they are a "special type of type".

I'm tired of this bikeshedding. I tried to advocate for keeping this issue focused on the actual bug that can be fixed (syntax highlighting), but you keep bringing it back to advocating for breaking compatibility in the language design as if this was the same level of implication.

@godotengine godotengine locked as too heated and limited conversation to collaborators Jan 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants