godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
91.44k stars 21.27k forks source link

Typed GDScript named enums can't be used as types #20368

Closed mrcdk closed 4 years ago

mrcdk commented 6 years ago

Godot version:

Godot master https://github.com/godotengine/godot/commit/4b549faaab40f09756b644d4ad4d9d63e3b5e70c

OS/device including version:

Windows 10

Issue description:

Typed GDScript doesn't let you use named enums as types. https://i.imgur.com/UFSfsLe.png

vnen commented 6 years ago

I thought about this but currently enums are just shorthand for dictionary/constants, they are not a special type. Not sure how to implement it properly (i.e. whether it should accept any int or only the named constants, and whether the value should be checked to be a valid one).

mrzapp commented 6 years ago

As a workaround, you can use int as the type hint for now.

@vnen would it be possible to implement a fallback type search based on the same logic used for member variable suggestion? A challenge with that approach might be differentiating between variables and enums, but I'm not sure about that.

vnen commented 6 years ago

@mrzapp I'm not sure if I follow. Do you mean just showing the enum constants on code completion when assigning?

mrzapp commented 6 years ago

@vnen I was probably a bit quick with that idea. Take 2:

I don't think you need to check if the value is valid, C# doesn't do this either:

public class Program
{
    enum MY_ENUM {
        VALUE_1,
        VALUE_2
    }

    public static void Main()
    {
        MY_ENUM myEnum = (MY_ENUM)33; // <- This compiles and executes
    }
}

It also accepts any int (if you cast it). I personally think it's OK to let GDScript accept any int implicitly, as it still gets the job done. It would be neat to limit the accepted range to the amount of named constants, but it's not a vital feature.

I think it would be perfectly reasonable to add enum declarations to the "script_type" tokens (if that makes sense, I admit I didn't read all of the code in the PR)

ArthaTi commented 5 years ago

I agree with @mrzapp. Enum typed variables shouldn't be limited to the enum values only. Making them an int implicitly would allow saving enums to files and reading them later. It could optionally limit the var to a certain range. Would be great for suggestions anyway.

vnen commented 5 years ago

We can certainly make an enum-typed variable simply be an int, but then we take a stand to not allow other types (such as String as asked in #20988), nor do we make tagged unions (as asked in #15141).

ArthaTi commented 5 years ago

@vnen Is that a big problem?

enum Test {
    TEST = "foobar"
}

func something(t: Test):
    print(t)

func _ready():
    something(Test.TEST)

If Godot could be able to detect the type of enum values, it could make the enum work as the type of the values.

vnen commented 5 years ago

But what if you have:

enum Test {
    STRING_VALUE = "foobar",
    INT_VALUE = 42
}

var my_var: Test

What is the type that my_var expects?

ArthaTi commented 5 years ago

@vnen I don't know. An union?

mrzapp commented 5 years ago

@vnen @Soaku I don't mean to be a buzzkill, but I think the feature requested in #20988 is highly unorthodox. I may be wrong about this, but I've never seen a language that treats enum values as anything but integers, and it certainly adds unnecessary complexity for very little gain. The problem mentioned in that issue could be solved with a dictionary in any case.

ArthaTi commented 5 years ago

I could understand the point of string enums, but adding to my previous reply, why would an enum allow mixed types? This would cause inconsistency and comparing two values in the enum could fail. For this, a separate enum should be created.

OvermindDL1 commented 5 years ago

@Soaku GADT enums allow for mixed types, it can be quite useful.

TheRealCatherine commented 5 years ago

I'm all for flexibility type-wise with enums, I'd even love arbitrary-type enums. I do kind of think sticking to a single type within the same enum set is reasonable.

Mostly, though, I would like a degree of type safety regarding enums so that I can't say like:

enum Direction {up, down} var dir : Direction = Color.red

(unless explicitly cast anyway)

nhold commented 5 years ago

Couldn't the type just be variant if you wanted arbitrary type enums?

mrzapp commented 5 years ago

I think we're not acknowledging the amount of work needed to make mixed type enums happen. If we settle on int-based enums for now, at least the problem that this issue was opened for (236 days ago ^^") will be solved. @vnen what is your opinion?

embeddr commented 5 years ago

I don't see significant value in adding mixed-type enums, but I do see value in being able to treat Enums as user-defined types and being able to enforce a specific enum type as a function argument type if the user wishes. Even if you don't enforce ranges (which I don't think is typical anyways), it at least pushes the programmer to think about the intention of the enum type instead of passing an arbitrary integer around.

DillonFlohr commented 5 years ago

I would definitely like to have Enums be user defined types. I understand we can define them as ints at the moment because that is what they are under the hood. But I would like to enforce that a value getting passed into a function is indeed a reference to an enum.

For example if I had a function call like this: if (player.canMove(Direction.Left)):

That function can never be considered "safe" to me because I can't enforce it on the other end:

#not valid code
func canMove(direction: Direction):

Being able to enforce that a parameter comes from a particular enum would be a good addition in my opinion.

twincannon commented 5 years ago

As a (probably bad) suggestion, what if enums could be typed and then their type is inferred when attempted to be used in static typing?

enum Fruit:int { APPLE = 0, BANANA = 1, ORANGE = 2 }
var current_fruit:Fruit

or maybe enum(int) ...

Not great since obviously the error still exists, and you'd have to know about this specific edge case to avoid it. Though I'd prefer this in lieu of leaving comments around saying that a function that expects an int is really expecting a specific enum. :)

rafaelgdp commented 4 years ago

Just stumbled upon this issue as I found the need for this in my code. Maybe in future versions of Godot it will be available. xD

Calinou commented 4 years ago

@rafaelgdp Please don't bump issues without contributing significant new information; use the :+1: reaction button on the first post instead.

ee0pdt commented 4 years ago

I think someone suggested above that the enum-as-type pattern is unorthodox, yet it is present in modern languages like typescript.

See this example from the TS docs:

enum Response {
    No = 0,
    Yes = 1,
}

function respond(recipient: string, message: Response): void {
    // ...
}

respond("Princess Caroline", Response.Yes)
mnn commented 4 years ago

The "enum-as-type" is quite common (often accepting everything of that type, like the example above from TS). I guess it's a start, but it doesn't check much - you could freely mismatch enums or pass invalid values. It would be nice to have true sum+product types like in Haskell:

-- compiler will check argument of type Bool to be either True or False,
-- nothing else is accepted (no random int)
data Bool = True | False

-- "enum" with values (I believe something similar is also in Scala in several forms)
data Shape = Circle Int | Box Int Int

I guess product types in the context of GDS would be typed Dictionaries (interfaces).

It's worth noting that even in TypeScript are unions, which can check if you are passing only allowed subset of other type (e.g. int, string) and are often used in place of enums because of better type checking:

type Bit = 0 | 1;
const b1: Bit = 0; // ok
const b2: Bit = 1; // ok
const b3: Bit = 2; // compilation error

type Animal = 'dog' | 'cat';
const a1: Animal = 'dog'; // ok
const a2: Animal = 'snake'; // compilation error

While I would love to see full algebraic data types ("enum"s with data like Shape above), I think for most users the sweet spot would be in fully typed unions (Bool, Bit and Animal examples).

tillre commented 4 years ago

I like how Rust or Haxe does that. You can have c-like enums:

enum Color {
    Red = 0xff0000,
    Green = 0x00ff00,
    Blue = 0x0000ff,
}

But also enums that contain different and more complex types:

enum Event {
    Init,
    AssetLoad = 0,
    AssetUnload = 1,
    KeyPress(char),
    Message(String),
    Click { x: int, y: int },
}

I think in the end its important how this works together with pattern matching. Sum types without pattern matching support are half as useful as they could be. If matching on the variant type of an enum is possible, this would be nice to have.

Javascript does not have runtime Typescript type information, so it cannot support matching on types. I don't know how that is with gdscript, how much type information it has or could have, but if matching on types would not be supported, doing as Typescript does with numeric enums and a union type could be a good way to go. I think its similar for Mypy.

clayjohn commented 4 years ago

Feature and improvement proposals for the Godot Engine are now being discussed and reviewed in a dedicated Godot Improvement Proposals (GIP) (godotengine/godot-proposals) issue tracker. The GIP tracker has a detailed issue template designed so that proposals include all the relevant information to start a productive discussion and help the community assess the validity of the proposal for the engine.

The main (godotengine/godot) tracker is now solely dedicated to bug reports and Pull Requests, enabling contributors to have a better focus on bug fixing work. Therefore, we are now closing all older feature proposals on the main issue tracker.

If you are interested in this feature proposal, please open a new proposal on the GIP tracker following the given issue template (after checking that it doesn't exist already). Be sure to reference this closed issue if it includes any relevant discussion (which you are also encouraged to summarize in the new proposal). Thanks in advance!

Zireael07 commented 4 years ago

One, I would think it's a bug rather than enhancement, and two, isn't it covered in @vnen's rework?

ArthaTi commented 4 years ago

@Zireael07 it is an enhancement, since currently enums in GDScript are more of a syntax sugar rather than a separate type. Also, I'm pretty sure this message is sent to most or all issues labeled "feature proposal".

vnen commented 4 years ago

It's not a bug because it's working as intended. But yes: I'll likely add enum as types in the new GDScript code.

phil123456 commented 3 years ago

I just can't believe such a feature is not available just mindblowing

Calinou commented 3 years ago

@phil123456 Please don't bump issues without contributing significant new information. Use the :+1: reaction button on the first post instead.

Also, I believe this was implemented in the GDScript rewrite that was merged in the master branch (what will become Godot 4.0 in the future).

cgbeutler commented 3 years ago

@vnen Does the rewrite of GDScript also involve exposing the enums in GlobalScope by name? If so, I may make note on this issue: https://github.com/godotengine/godot/issues/20015

chandujr commented 3 years ago

Funny thing is, we can't use enums as a type like this:

var game_state : Enums.GameState

But we can use it like this:

export(Enums.GameState) var game_state
MoxyFoxy commented 2 years ago

You also cannot use enums as parameter types. For example, I have a Type enum to designate the element type of an attack and a Terrain enum to designate the type of terrain, and am using it like func deal_damage(damage: int, type: Type, phy: bool, terrain: Terrain, ...) and I get the error The identifier "Type" isn't a valid type (not a script or class), or couldn't be found on base "self"., despite the enum being in the same file (with Type but not Terrain, but Terrain is imported with a preload). Are there any plans to allow for this in the future?

Also in reference to the previous message by @chandujr, I had the opposite issue. I can use my enums Type and Terrain as the type definition of a variable but NOT as an export, which would also be a great feature to have, possibly a dropdown with the different values of the enum?

MoxyFoxy commented 2 years ago

Update on my last message's second paragraph (wanted to add a new one rather than edit to keep context): So it turns out that Godot evaluates errors a little weirdly. Had the errors go on and off every now and then. When I made some changes, the errors would completely disappear without me even interacting with them in any way. When I fixed the function errors, after a while I got the same error on the variables. So ignore the response I had on that. I still couldn't use my enum as an export, though. Maybe I was just doing it wrong? Idk, I was doing export(Type) and export(Terrain).