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

turn enum into @enum #85

Open
Maiori44 opened this issue Feb 6, 2023 · 34 comments
Open

turn enum into @enum #85

Maiori44 opened this issue Feb 6, 2023 · 34 comments
Labels
change Change of an already existing feature experimental The changes may break something or be a bad idea

Comments

@Maiori44
Copy link
Member

Maiori44 commented Feb 6, 2023

when converting old 2.5 code to 3.0 I noticed I changed all enums to a bunch of @define, which made me realise enum would work better as a preprocessor directive that sets many preprocessor variables

@Maiori44 Maiori44 added the change Change of an already existing feature label Feb 6, 2023
@Markos-Th09
Copy link
Member

Markos-Th09 commented Feb 8, 2023

when converting old 2.5 code to 3.0 I noticed I changed all enums to a bunch of @define, which made me realise enum would work better as a preprocessor directive that sets many preprocessor variables

The current implementation of enums is kinda bad already because it doesn't really have named enums (which I prefer). And also it is would be better to remain a keyword

@Maiori44
Copy link
Member Author

Maiori44 commented Feb 8, 2023

If you mean Rust's those are too complex and don't feel necessary in Clue (since tables)

@Markos-Th09
Copy link
Member

If you mean Rust's those are too complex and don't feel necessary in Clue (since tables)

No that. I just mean that the enum should have a name. The variants won't really have any value with them.

enum Something{
     A,
     B
}

@Maiori44
Copy link
Member Author

Maiori44 commented Feb 8, 2023

A name would really only be useful for type checking, which is not yet a thing

@Markos-Th09
Copy link
Member

A name would really only be useful for type checking, which is not yet a thing

but we don't really need types to implement it it would just be compiled to a table

@Maiori44
Copy link
Member Author

Maiori44 commented Feb 8, 2023

That is..unnecessary?

@Maiori44
Copy link
Member Author

Maiori44 commented Feb 8, 2023

The point of enums is to define state constants more quickly
But since those states are just constant numbers it would be better to make them a compile time thing

@Markos-Th09
Copy link
Member

The point of enums is to define state constants more quickly
But since those states are just constant numbers it would be better to make them a compile time thing

but we still need good lua interoperability

@Maiori44
Copy link
Member Author

Maiori44 commented Feb 8, 2023

What?

@Markos-Th09
Copy link
Member

if you were to make a library which should also work in lua the the enum must be easy to use to use in lua

@Maiori44
Copy link
Member Author

Maiori44 commented Feb 8, 2023

I guess that's fair

@Maiori44
Copy link
Member Author

Maiori44 commented Feb 8, 2023

How about just having both enum and @enum?
One compiles to global variables that can be used outside of the Clue code (for libraries) and the other is all compile time?

Edit: ...that pinged somone, oops

@Markos-Th09
Copy link
Member

I guess if the difference is clear for everyone it they can satisfy both use cases

@Maiori44
Copy link
Member Author

Maiori44 commented Feb 8, 2023

local enum could error saying to use @enum instead
While global enum will work but give a warning

@Markos-Th09
Copy link
Member

local enum could error saying to use @enum instead
While global enum will work but give a warning

no need

@Maiori44
Copy link
Member Author

Maiori44 commented Feb 8, 2023

I mean local enum would be deprecated so it would error anyway

@Markos-Th09
Copy link
Member

Markos-Th09 commented Feb 8, 2023

I mean local enum would be deprecated so it would error anyway

what if somebody wanted to just use a named enum in the local scope?

@Maiori44
Copy link
Member Author

Maiori44 commented Feb 8, 2023

They use @enum

@Markos-Th09
Copy link
Member

They use @enum

but @enum is not a named enum, which has some use cases outside of a library

@Maiori44
Copy link
Member Author

Maiori44 commented Feb 8, 2023

I doubt naming enums will be useful..

@Markos-Th09
Copy link
Member

I doubt naming enums will be useful..

it still has its uses and I would use it for some things

@Maiori44
Copy link
Member Author

Maiori44 commented Feb 9, 2023

Forgot to reply-
Ill need to reflect on this

@ashifolfi
Copy link

I doubt naming enums will be useful..

I use them all the time they are extremely useful

@Maiori44
Copy link
Member Author

Maiori44 commented Feb 9, 2023

You use the name all the time?

@ashifolfi
Copy link

ashifolfi commented Feb 9, 2023

You use the name all the time?

Yes I name my enums. Being able to have a single named enum with values of simple names like say FileMode.Read (C# style enum) is very very helpful for code readability and just writing the code in general. Even in my own Lua enum implementation I use a named enum setup.

I see enums as a group of number constants that belong to a single "type" which is the name. Without that name they lose a chunk of their meaning

@ashifolfi
Copy link

ashifolfi commented Feb 9, 2023

Enum values are almost always prefixed with a shared name for organization no matter the language so why not just group them under that name automatically with a named enum system. Cutting out the need to manually type that prefix each time.

@Maiori44
Copy link
Member Author

Maiori44 commented Feb 9, 2023

Because Clue does not have types (yet) so the name would not be used

@ashifolfi
Copy link

ashifolfi commented Feb 9, 2023

Because Clue does not have types (yet) so the name would not be used

?????? That doesn't mean anything? They should all have the type be used somewhere in calling them? Like C# does Type.Value and my Lua implementation initially used Type_Value. (I have since swapped to C# style)

@ashifolfi
Copy link

ashifolfi commented Feb 9, 2023

The point is making them visibly related to each other along with type checking. Not exclusively type checking.

@Maiori44
Copy link
Member Author

Maiori44 commented Feb 9, 2023

How could Clue store a type inside a number?

@ashifolfi
Copy link

ashifolfi commented Feb 9, 2023

How could Clue store a type inside a number?

By defining the type as the name of an enum you'd tell the compiler "hey if the provided value doesn't come from an enum value in this name group throw an error"
Anything else with compiling down to Lua could be done however you see fit but the compiler would just check the provided enum value against those present in the enum name you've specified as the type. And because you'd also need the name (ideally) somewhere in the call to the value it can use that name as well. Nothing would need to be stored in the enum value aside from the number and checking the name would be done based on the actual code you've fed the compiler and not what the code would compile down to (i.e. the actual number/value of the enum value isn't needed for the type check if that makes sense.

@Maiori44
Copy link
Member Author

Maiori44 commented Feb 9, 2023

Could I have an example of valid and invalid code?
It's kinda late so I can't visualize it well

@ashifolfi
Copy link

ashifolfi commented Feb 9, 2023

Could I have an example of valid and invalid code? It's kinda late so I can't visualize it well

enum Enum1 {
    Yes,
    No
}

enum Enum2 {
    Yes,
    No
}

Enum1 thing = Enum1.yes // valid as the name comparison checks out and yes is a value in Enum1

Enum1 thing = Enum2.yes // invalid as the value comes from Enum2

Enum1 thing = Enum1.maybe // invalid as maybe is not a value in Enum1

Enum1 thing = Enum3.yes // invalid as Enum3 does not exist

Here's a quick example using C# style names

@Maiori44
Copy link
Member Author

Maiori44 commented Feb 9, 2023

I see, makes more sense now
This could likely be done in 4.0, alongside all the other type things

@Maiori44 Maiori44 added this to the 4.0 milestone Feb 9, 2023
@Maiori44 Maiori44 added the experimental The changes may break something or be a bad idea label Nov 30, 2023
@Maiori44 Maiori44 removed this from the 4.0 milestone Nov 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
change Change of an already existing feature experimental The changes may break something or be a bad idea
Projects
Status: Unsure
Development

No branches or pull requests

4 participants
@ashifolfi @Markos-Th09 @Maiori44 and others