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

enums duplicated as a dictionary should have their key/values as constants #25102

Closed
blain1972 opened this issue Jan 18, 2019 · 9 comments
Closed

Comments

@blain1972
Copy link

blain1972 commented Jan 18, 2019

Godot version:
3.1 beta 1

OS/device including version:
Ubuntu 18.10

Issue description:
enums are immutable but assigning an enum at run-time seems convenient to me.

However, since they are just dictionaries for constants, would it be possible to have them respect type if, for example, an enum was assigned to an empty dictionary at run-time?

enum input_type {ACTION_1, ACTION_2, ACTION_3}
var my_input_type = {}

my_input_type = input_type.duplicate()

result:

input_type is essentially a dictionary of constants

my_input_type is a dictionary of variables

I would like my_input_type in this scenario to be a list of constants.

In the instances where a user would want this: it would allow the ease of declaring a raft of constants as an enum and therefore maintain code readability. It would also be safer having constants rather than variables, which could be changed by accidental assignment.

I think this would be useful for state machines where entities are assigned their list of available states from global configuration file via autoload.

My use case is for a list of actions in an Utility AI, where the available actions vary for different classes of entity. These are assigned at run-time.

Steps to reproduce:
n/a

Minimal reproduction project:
3.1 Test.zip

edit: added beta version

@bojidar-bg
Copy link
Contributor

Note that currently GDScript is a bit slack on constants. For example, this works (or at least worked recently):

const X = 5
enum Z {a, b, c}
func _ready():
    X = 7; print(X) # 7
    Z.a = 7; print(Z) # (a: 7), (b: 1), (c: 2)
    Z = {g: 7}; print(Z) # (g: 7)

About dictionary of variables and list of constants.. are you sure you don't want input_type.keys() or input_type.values()?

@blain1972
Copy link
Author

blain1972 commented Jan 18, 2019

I was using this behaviour until 3.1 beta not realising it was a bug.

What I want is the keys to have constant values without declaring loads of constants. It's a bit lazy but it is likely to assist with code readability. At least, I am assuming that's it what I would need to do for a state machine or similar.

edit: typo

@vnen
Copy link
Member

vnen commented Jan 19, 2019

@bojidar-bg this does not work on current master, you can't reassign the constants.

However, since enum is just a dictionary you are able to change its contents at will: you can add/remove keys and change any value. So the Z.a = 7 line in @bojidar-bg's example is still valid.

If you need to assign constants at runtime, what you want is likely "immutable variables" (see #23695).

@blain1972
Copy link
Author

@vnen Immutable variables sounds like a great idea.

Especially since I was populating enums at run-time for different entities via a script. This was prior to 3.1.

On testing 3.1 I found that I couldn't and shouldn't be able do this. #25096

It was simply convenient to have the enum assign integers to each key rather than doing it manually with a dictionary.

It is likely that my understanding of enums is incorrect. I thought that since they were constants, their keys/values would also be constants. But that doesn't appear to be the case.

However, since enum is just a dictionary you are able to change its contents at will: you can add/remove keys and change any value. So the Z.a = 7 line in @bojidar-bg's example is still valid.

If this is the case shouldn't it be possible to add key/value pairs to an empty enum:

enum input_type {ACTION_1, ACTION_2, ACTION_3}
enum my_input_type {}

my_input_type = input_type

or

my_input_type = input_type.duplicate()

both of these result in Parser Error: Cannot assign a new value to a constant in v3.1.beta1.official.

I haven't tried beta 2

@vnen
Copy link
Member

vnen commented Jan 19, 2019

However, since enum is just a dictionary you are able to change its contents at will: you can add/remove keys and change any value. So the Z.a = 7 line in @bojidar-bg's example is still valid.

If this is the case shouldn't it be possible to add key/value pairs to an empty enum:

enum input_type {ACTION_1, ACTION_2, ACTION_3}
enum my_input_type {}

my_input_type = input_type

or

my_input_type = input_type.duplicate()

both of these result in Parser Error: Cannot assign a new value to a constant in v3.1.beta1.official.

No, those are different from the example. If you do enum input_type, then the input_type name can never be assigned to something else, because it is a constant (i.e. doing input_type = anything will never work). The dictionary itself can be changed (hence the Z.a example still works) but you can't assign a different dictionary to the name.

@blain1972
Copy link
Author

No, those are different from the example. If you do enum input_type, then the input_type name can never be assigned to something else, because it is a constant (i.e. doing input_type = anything will never work). The dictionary itself can be changed (hence the Z.a example still works) but you can't assign a different dictionary to the name.

So, the enum identifier is constant but its contents are not constant.

If this is the case then should it not be possible to duplicate an enum into a previously declared empty enum? Is that not changing the dictionary itself?

so maybe my_input_type = input_type.duplicate() or something similar would syntactic sugar for

enum input_type {HAS_TARGET, MY_HEALTH, MY_ENERGY, DISTANCE_TO}
enum my_input_type {}

func _ready():
	var i = 0
	for key in input_type.keys():
		my_input_type[key] = i
		i += 1
	print(my_input_type) # {DISTANCE_TO:3, HAS_TARGET:0, MY_ENERGY:2, MY_HEALTH:1}

But, that would still leave the possibility of accidental assignment when doing comparisons. So if immutable variables are implemented I would still have to run a loop.

So maybe setting a flag to fix the key/values of the enum could be included in the duplicate function?

I get it if this is not the way it is supposed to be. In which case just having a flag for dictionaries would be cool. Then it's just a case of duplicating the enum into a dictionary with a flag setting its contents as immutable.

@vnen
Copy link
Member

vnen commented Jan 21, 2019

With immutable variables you would not assign it to an enum, but to a variable. Somewhat like this:

enum input_type {HAS_TARGET, MY_HEALTH, MY_ENERGY, DISTANCE_TO}
immutable var my_input_type

func _ready():
    my_input_type = input_type.duplicate()

so maybe my_input_type = input_type.duplicate() or something similar would syntactic sugar for[...]

This would defeat the purpose of it being a constant. You are rebinding the name to a different value, which is forbidden. That is, doing const dict = {} (which is pretty much the same as an empty enum) allows you to do dict.x = "something" and everybody who captured the reference to the dictionary (which is constant) can see the new key that was added. If you rebind the reference, everybody else will still point to the old reference that will always be empty.

The syntax sugar for populating the dictionary sounds nice in theory but breaks a lot of assumptions (people who read the code will wonder why it allows assignment to a constant) and requires runtime checks to see if the dictionary is really empty when trying to do it (meaning a performance hit to every assignment, since we can't know beforehand if it's a constant dictionary). Also, if the dictionary is emptied in the future, should it allow to be repopulated in this fashion again? If so, you are able to commit mistakes anyway, again defeating the purpose of being a constant. If not, it requires keeping track that it was done already, adding and extra performance hit.

If you really want to do this, there's no need to use an enum or a constant. Even right now you can use a regular variable (essentially the same sample code as I just wrote above without the immutable word). If you want to avoid reassignment you can make a setget that only allows to assign it once.


The dictionary itself can always be changed, the contents are never constant. If you want constant content, you're better off creating a custom class to handle this, using _set() and _get() to control the assignment manually.

@blain1972
Copy link
Author

@vnen thank you for your explanation.

I think though that I haven't been clear enough in what I want.

I was thinking that I want the dictionary filled with constants not the dictionary itself to be constant. This is so I don't have to declare each and every one of them, just to avoid silly mistakes.

The value assigned to each key has no importance, in this case, hence why I was using an enum.

For me, I can't currently envisage an accidental assignment since I am only likely to do comparisons and the parser will pick up any mistakes there.

I hadn't thought of using setget so I will try that and explore your other suggestions to develop my skills.

Also, if it goes against regular programming conventions then it's probably not a good idea.

This has been very helpful.

@Calinou
Copy link
Member

Calinou commented Dec 12, 2020

Closing per the discussion above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants