GDScript: Begin making constants deep, not shallow or flat

[vonagam]

Current behavior of const is troubled. They want to act like deep, but they are shallow and used by some as static variables:

const inconsistent = [0]

inconsistent[0] = 1 # error

var zero := 0
inconsistent[zero] = 1 # no problem
inconsistent.push_back(2) # no problem
print(inconsistent) # [1, 2]

print(inconsistent[0]) # 0
print(inconsistent[zero]) # 1

It is possible to resolve this in two ways:

1. Either declare them as mutable and protected only from reassignment. Then do not expect them to produce constant results outside of constant context. (In this case, maybe, it is actually better to name them static const, as they are static.)

2. Or declare them as deeply immutable. Then do not allow any modifications to the stored value, making it read-only.

In yesterday discussion between GDScript contributors it was decided to go with later option. This PR is a first move into that direction. It recursively makes array/dictionary literals to be read-only and disallows subscript assignments on constant base.

This does not cover all bases - other builtin types need to implement read-only mode too and other expressions than simple literals need to be covered too, also set_read_only most likely should be removed from public api for Array.

But this one should cover most popular cases, making it clear what is expected and what is not. This change is a breaking one (resolution of this inconsistency in either direction will be breaking), preferably it should be included earlier than later.

Connected issues:
Related to #30212 (expectation about it being deep).
Related to #32063 (expectation about it being shallow and non-static).
Related to #54718 (expectation about it being deep).
Related to #56306 (expectation about it being shallow, problem with modifying).
Closes #61274 (expectation about it being deep).
Closes #61919 (expectation about it being shallow, problem with modifying).
Closes #62680 (expectation about it being deep, inconsistent behavior).
Closes #70349 (expectation about it being deep).
Closes #70469 (expectation about it being shallow, inconsistent behavior).
Closes #70730 (expectation about it being shallow, inconsistent behavior).
Closes #4576 (proposal to make them deep).

[akien-mga]

Note for future work: we should make this error message more explicit.

[anvilfolk]

Also, hopefully as an analyzer error rather than as a runtime error?

[akien-mga]

Looks good to me as a start.

[vnen]

other builtin types need to implement read-only mode too

Which types? PackedArrays probably could use this since they're also passed by reference, but the others are passed by copy so you wouldn't be able to change the original constant anyway.

The only other case would be objects which does introduce some hurdles to guarantee immutability.

[vnen]

I believe this was done to allow mutability on assignments, e.g.:

const arr = [1, 2, 3]
var copy = arr # Create copy so it's a regular variable.
copy.push_back(4) # Only changes `copy`.
print(copy) # [1, 2, 3, 4]
print(arr) # [1, 2, 3]

Which wouldn't work with this change (push_back() would fail). But I don't think there's a valid use case for this and the user can call duplicate() if this is needed.

[akien-mga]

Thanks!

[ExplodingImplosion]

So does this mean we'll no longer be able to modify constant dictionaries and arrays in future builds?

[YuriSizov]

@ExplodingImplosion That seems to be the case, yeah. Which sucks since we still don't have static properties.

[KoBeWi]

We do have static properties and it's still a hack ;)
class_name classes are just scripts and scripts are objects and every object has metadata. Solution -> use class' script's metadata. The tricky part is getting script from a class name. Here's a working example:

func _ready() -> void:
	if not (MyClass as Script).has_meta("test"):
		MyClass.assign_test()
	else:
		print((MyClass as Script).get_meta("test"))

in MyClass code:

class_name MyClass

static func assign_test():
	(MyClass as Script).set_meta("test", 2)

Attach the first script to 2 nodes and it will print the value of test property.

[YuriSizov]

Just for the record, the GDScript team does want to have proper static properties in the future, but that's not ready yet, won't be ready for 4.0. Yet, fixing this behavior and making it more reliable and predictable for its intended use case was important to do before 4.0.

[vonagam]

Which types? PackedArrays probably could use this since they're also passed by reference.

Yes, If implementation of read-only functionality is moved from Array to Vector<T> it will allow to use them in constant context - const colors := PackedColorArray([ ... ]) which is forbidden at the moment.

but the others are passed by copy so you wouldn't be able to change the original constant anyway.

Third linked issue was about changes allowed on constant color. But now second change in this PR (that an assignment to subscript of constant base is forbidden) solves it.

So, I guess, true - all currently allowed types in constants are covered.

[Anutrix]

The changes all seem right but shouldn't this be changed to immutable keyword instead of const?

Devs coming from other languages might find the behaviour confusing otherwise.

[anvilfolk]

The changes all seem right but shouldn't this be changed to immutable keyword instead of const?

const data structures in C++, for instance, are immutable. The idea here was actually to make GDScript more like other programming languages! :) Perhaps a little less like some scripting languages though, true!

[ExplodingImplosion]

I will miss the hacky implementation of const arrays and dictionaries. Like, inherently the idea that since they were reference passed, etc, only the reference itself was the constant part of the type. But the fact that you couldn't assign a class to a constant was this weird frustrating inconsistency, Even though from a code implementation perspective it makes total sense that you can only make built-in-types constant. Anyway, good to know that static variables are on the soon(ish) to-do list! And making GDScript more consistent is a net positive in my book. Thanks, everyone!

[b0z1]

Now my project is impossible to realize in Godot 4...
Whoever decided constant instances (e.g. Dictionaries) are a bad thing should be ashamed.
Such a critical change 👎

[akien-mga]

@b0z1 You can use autoloads to store static data until static variables are implemented.

@freehuntx Please refrain from making this kind of unhelpful comments.

[Zireael07]

That's a pretty big change that snuck in pretty much just before 4.0. Docs need to be updated ASAP otherwise people will run into a LOT of trouble

[hafofu]

Seriously? So you break a crucial functionality and tell us with Godot 5 we will get an alternative (static variables)?
Why do you remove features when you have no alternative?
How do you explain that in an migration guide?

Atleast add a workaround until you offer us static variables -.-
E.g.

const constDictionary = { $deepConst=false, someValue=0 }

func _ready():
  constDictionary.someValue = 1

[akien-mga]

We never told anyone to wait until Godot 5. We'll likely implement static variables for one of the first 4.x releases. We did this change now precisely so that we can implement static variables in the future without breaking compatibility.

We did not remove a feature - we fixed an unintentional inconsistency, which some users relied upon. This const behavior was not reliable, not expected by the majority of users, and you'd lose data if the script reference count would reach 0.

As mentioned, with autoloads you can have shared memory accessible to your script instances.

[YuriSizov]

Docs need to be updated ASAP otherwise people will run into a LOT of trouble

Out of curiosity, @Zireael07, which part of the docs recommends using a const dictionary like that? If we have something like that in the official documentation, it needs to be removed regardless of this PR, because while a nice trick, it's not really a supported, or indeed intentional, feature.

[Zireael07]

Docs don't recommend this usage (or mention it) anywhere that I can see so far, but I know many people use this "trick" so it should be pointed out that this no longer works.

[freehuntx]

Since objects, arrays and dictionaries are passed by reference, constants are "flat". This means that if you declare a constant array or dictionary, it can still be modified afterwards. They can't be reassigned with another value though.

[YuriSizov]

@freehuntx Where is that quote from?

@Zireael07 We can reinforce the idea that constant members are deeply constant in the GDScript intro doc, but otherwise I don't see how to inform users that a particular hack doesn't work anymore. We don't have an area in the docs for documenting such things.

[Zireael07]

Not freehunt, but I found the quote: https://docs.godotengine.org/en/latest/tutorials/scripting/gdscript/gdscript_basics.html, Ctrl+F "Constants", note on a blue background.

Yeah, this should be reinforced in the docs.

[freehuntx]

We did not remove a feature - we fixed an unintentional inconsistency, which some users relied upon. This const behavior was not reliable, not expected by the majority of users, and you'd lose data if the script reference count would reach 0.

So the documentation cant be trusted?
I mean according to you an unintended feature got clearly documented.
But im glad you removed it now without an alternative :) That was a good decision.

[anvilfolk]

Chill, friend :) I appreciate your enthusiasm, but we're all in this together. We're doing our best, most of us in our free time, to make Godot behave as consistently & well as it possibly can in the long run.

This is a hiccup, yes. I think we are all in agreement here that ideally this would've been spotted & fixed during the alpha stage! Unfortunately, the Real World happens, and it's getting handled during beta. Functionality should be reinstated sooner rather than later, in a more intentional & logical way, which is better in the long run :)

Throwing shade and being snarky just breeds negative emotions, and lessens the motivation for people to keep working on making Godot better :)

[hilajour]

@Zireael07

Docs don't recommend this usage (or mention it) anywhere that I can see so far, but I know many people use this "trick" so it should be pointed out that this no longer works.

The Array class description has a Note which claims that when declaring with const, it can still be mutated.

[CorbinSteele]

This remains incorrect in a Note on Constants within the GDScript basics reference:
https://docs.godotengine.org/en/latest/tutorials/scripting/gdscript/gdscript_basics.html#constants

Since objects, arrays and dictionaries are passed by reference, constants are "flat". This means that if you declare a constant array or dictionary, it can still be modified afterwards.