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

constants are actually "flat" #30212

Closed
Scony opened this issue Jun 30, 2019 · 4 comments
Closed

constants are actually "flat" #30212

Scony opened this issue Jun 30, 2019 · 4 comments

Comments

@Scony
Copy link
Contributor

Scony commented Jun 30, 2019

Godot version:
3.1 official build

OS/device including version:
Linux

Issue description & Steps to reproduce:
I'll start with quote from docs Constants are similar to variables, but must be constants(...).

The issue is, constants in godot are flat, i.e. if the constant is set e.g. to Dictionary(), the constant itself is constant, however dict's internals are not.

Let's consider the example below (UTs using Gut):

extends "res://addons/gut/test.gd"

const FLAT_CONST = 123
const DEEP_CONST = {
	'foo': 'bar',
}

func test_flat_cnst():
	var pre = FLAT_CONST
	# FLAT_CONST = 321 # Parser Error: Cannot assign a new value to a constant
	var post = FLAT_CONST
	assert_eq(pre, post, "Should pass")


func test_deep_cnst():
	var pre = str2var(var2str(DEEP_CONST)) # make copy
	DEEP_CONST['foo'] = 'baz'	       # compiles
	var post = DEEP_CONST
	assert_eq(pre, post, "Should pass")    # fails

The second test fails. I suspect why this is happening and even if this is made like that on purpose, IMO, it's counter-intuitive. const should enforce deep copying etc. I've already ran into trouble as I've been assuming consts are really const (IMO deep).

After writing 1.5k LOC in GDScript this is the only weird issue I've faced.

It should either be fixed, or documented better.

@Scony
Copy link
Contributor Author

Scony commented Nov 4, 2019

I had some discussion with colleagues regarding this topic and eventually, my proposal would be to introduce a keyword final. It would work as the current implementation of const (reference is const, but data is mutable) whereas const should extend final in a way that data is immutable.

So:

const c_ref_and_data = {'a':1}
final c_ref = {'b':2}
(...)
c_ref_and_data = {} # illegal
c_ref_and_data['x'] = 3 # illegal
c_ref = {} # illegal
c_ref['x'] = 3 # fine

IMO it would be intuitive for people familiar with Java, C++, etc.

@Calinou
Copy link
Member

Calinou commented Nov 4, 2019

I think changing the behavior of const is good, but I'd prefer something akin to immutable variables rather than final (even though both seem quite close). Immutable variables could also be used in local scope.

We can do this for 4.0 if anyone is currently relying on this behavior.

@Scony
Copy link
Contributor Author

Scony commented Nov 4, 2019

Well, either way, const should make binding as well as data absolutely immutable - even for compound data types.

@KoBeWi
Copy link
Member

KoBeWi commented Oct 28, 2020

This is a feature proposal disguised as a bug, so I'm closing it.


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!

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

3 participants