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

Seralization and deserialized of typed properties in array #175

Merged

Conversation

imothee
Copy link
Contributor

@imothee imothee commented Mar 5, 2024

An alternative approach to #162

The array serialization/deserialization is working in the test for Color.

Outstanding questions
Should a reference deserialize into the entity itself or a reference type?

ie. What would make more sense?

get_array("Equipment") return Array[PandoraReference] and you manually .map(func(r): return r.get_entity())
or we do that step ourselves in the deserialization

get_array("Equipment") return Array[PandoraEntity] which feels a bit more logical to hide the reference.

Old save/load cycle

var old_color_array = old_entity_1.get_array("typed_array")
var new_color_array = new_entity_1.get_array("typed_array")

print("Got Array")
print(old_color_array)
print(new_color_array)

print(typeof(old_color_array[0]))
print(typeof(new_color_array[0]))

[(1, 0, 0, 1)]
["ff0000ff"]
20
4

New array code.

var old_color_array = old_entity_1.get_array("typed_array")
var new_color_array = new_entity_1.get_array("typed_array")

print("Got Array")
print(old_color_array)
print(new_color_array)

print(typeof(old_color_array[0]))
print(typeof(new_color_array[0]))

[(1, 0, 0, 1)]
[(1, 0, 0, 1)]
20
20

However, I am struggling a bit with the test because the minute I call either of

var old_color_array = old_entity_1.get_array("typed_array")
var new_color_array = new_entity_1.get_array("typed_array")

something inside of the ._entities dictionary is loading a recursive reference because running

# Making this call after getting the array from either of the entities (old/new) will throw the error ERROR: Max recursion reached at: write (core/variant/variant_parser.cpp:1942)
assert_that(old_entities).is_equal(backend._entities)

Throws millions of exceptions (forever).
I've tried to introspect the objects in the dictionary but I can't for the life of me work out what reference if circular and what property on PandoraEntity is being introspected. Throwing this up to get feedback on the approach and to see if anyone can help debug why I can't do an is_equal comparison on them.

My hypothesis is that the value loaded into .property_map has a reference to itself but it's hard to print the stack and I can't breakpoint since there's no error, just a bunch of warnings in a loop.

@imothee
Copy link
Contributor Author

imothee commented Mar 5, 2024

Well, I'm not sure what changed between last night and this morning but apparently the tests run now without the recursion error. Haven't changed any code at all so no idea what was happening there. 🤷‍♂️

@imothee imothee force-pushed the array-element-typed-serialization branch from 4ba141c to 4d7e069 Compare March 5, 2024 16:28
@bitbrain
Copy link
Owner

LGTM - @imothee happy for me to merge this or is there something missing?

@imothee
Copy link
Contributor Author

imothee commented Mar 19, 2024

I'm not sure if we want consistency to use _type and _value in the dictionary that's serialized (like the settings) and honestly I'm not sure if the right way to do this is to just ensure that we have access to settings and use it to set the type/hydration rather than writing it with ever single serialized value.
This will at least let you have multi-typed arrays and it seems to work (been testing it at length in my game) so I think it's just a high level decision on how you foresee it working in your original architecture.

@bitbrain bitbrain changed the title [WIP] seralization and deserialized of typed properties in array Seralization and deserialized of typed properties in array Jun 9, 2024
@bitbrain
Copy link
Owner

Let's merge this - in case there are any issues with this approach we can refactor it.

@bitbrain bitbrain merged commit 4beba3e into bitbrain:godot-4.x Jun 18, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants