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

inst2dict do not serialize object correctly due to calling getter methods #33015

Closed
Tracked by #33348
ykoc opened this issue Oct 23, 2019 · 2 comments · Fixed by #33018
Closed
Tracked by #33348

inst2dict do not serialize object correctly due to calling getter methods #33015

ykoc opened this issue Oct 23, 2019 · 2 comments · Fixed by #33018

Comments

@ykoc
Copy link

ykoc commented Oct 23, 2019

Godot version: 3.1 stable

Maybe this is not a bug but a documentation incompleteness.
While dict2inst function is setting values without using setter methods, inst2dict on the opposite calls get on every class field preventing any code-efficient way to store internal state of object (even simple data structure). Giving that there is no code-efficient way to clone objects either (return dict2inst(inst2dict(self)) is the only solution) this leads to severe problems with such simple operations as cloning and serializing objects with great (and constantly evolving) number of fields (since there is no way to manually manage serialization of 200-300 fields because of around 5 of them have getter methods).

demonstration code:
`
class test:
var value setget setter, getter
func setter(val):
print('setter called')
func getter():
print('getter called')
return value
func _init():
print('init called')

func _ready():
var tmp = test.new()
var dict = inst2dict(tmp)
var tmp2 = dict2inst(dict)
`

@Xrayez
Copy link
Contributor

Xrayez commented Oct 23, 2019

Is the problem with getter methods consists of the possible side effects of calling such methods?

The relevant piece of code which does this seems to be:

while (p) {
for (Set<StringName>::Element *E = p->members.front(); E; E = E->next()) {
Variant value;
if (ins->get(E->get(), value)) {
String k = E->get();
if (!d.has(k)) {
d[k] = value;
}
}
}
p = p->_base;
}

So instead of calling the ins->get method, it would need to be rewritten to access members directly.

Refined snippet:

extends Node2D

class test:
	var value setget setter, getter
	func setter(val):
		print('setter called')
	func getter():
		print('getter called')
		return value
	func _init():
		print('init called')

func _ready():
	var tmp = test.new()
	var dict = inst2dict(tmp)
	var tmp2 = dict2inst(dict)

Also, you could eventually stumble upon this problem using dict2inst: #30572.

@ykoc
Copy link
Author

ykoc commented Oct 23, 2019

Is the problem with getter methods consists of the possible side effects of calling such methods?

But 'side effects' of getter methods are the only reason they exist - cause without them getter do nothing above default getter (i.e. returning value directly). Still there are non-destructive side effects and there are logic-breaking side effects. For example, getter that do auto-converting some id to pooled object link (one of the simplest application) breaks working of serializing since there is no way to get to that id from list of properties. And since both trivial auto-serializing and customizable setters/getters are ones of the basic tools to proper system, it is very annoying (and clearly non-intuitive) that they are mutually incompatible.

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

Successfully merging a pull request may close this issue.

3 participants