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.

Sign up for GitHub

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

Jump to bottom

Modify Array type to encode values type, if possible, when serialized #162

Closed
wants to merge 4 commits into from
Closed

Modify Array type to encode values type, if possible, when serialized #162

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c893926
Modify Array type to encode values type, if possible, when serialized
Dec 13, 2023
173c589
Removed unnecessary if statement and ensured array
Dec 14, 2023
4c268a5
Merge branch 'godot-4.x' into fix-array-parsing
bitbrain Feb 23, 2024
d9daa6d
Merge branch 'godot-4.x' into fix-array-parsing
bitbrain Mar 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions addons/pandora/model/type.gd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ func allow_nesting() -> bool:
static func lookup(name:String) -> PandoraPropertyType:
if name == "":
return UndefinedType.new()
if not ResourceLoader.exists("res://addons/pandora/model/types/" + name + ".gd"):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should avoid using absolute paths, can you make this a relative lookup? (in other places too if applicable)

In case this turns out to be difficult, I am happy to keep it absolute for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked into it and unfortunately it doesn't seem possible for the moment (at least not trivially).

  • DirAccess.open requires an absolute path: "Static methods only support absolute paths (including res:// and user://)." docs
  • load also requires an absolute path: "Important: The path must be absolute. A relative path will always return null." docs
  • ResourceLoader.load doesn't specify but after testing it seems to be the case as well.

It seems that lookup and get_all_types are implicitly covered by some existing tests (because I broke them when trying) but I could add some tests for PandoraPropertyType.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The best way to do this is by taking the resource path of the existing class and using it as the root dir for all loading.

ie.

        var klass = PandoraPropertyType
	var types_dir = klass.resource_path.get_base_dir()
	print(types_dir)
	var script_type = types_dir + "/" + name + ".gd"
	print(script_type)

Prints
res://addons/pandora/model
res://addons/pandora/model/int.gd

return UndefinedType.new()
var ScriptType = load("res://addons/pandora/model/types/" + name + ".gd")
if ScriptType != null and ScriptType.has_source_code():
return ScriptType.new()
Expand Down
65 changes: 45 additions & 20 deletions addons/pandora/model/types/array.gd
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
extends PandoraPropertyType

const SERIALIZED_VALUES = "values"
const SERIALIZED_VALUES_TYPE = "values_type"

const ICON_PATH = "res://addons/pandora/icons/Array.svg"

const SETTING_ARRAY_TYPE = "Array Type"
Expand Down Expand Up @@ -30,34 +33,56 @@ func parse_value(variant:Variant, settings:Dictionary = {}) -> Variant:
if variant is Dictionary:
var array = []
var dict = variant as Dictionary
for i in range(dict.size()):
var value = dict[str(i)]
if not settings.is_empty():
if settings[SETTING_ARRAY_TYPE] == "reference":
value = PandoraReference.new(value["_entity_id"], value["_type"]).get_entity()
if settings[SETTING_ARRAY_TYPE] == "resource":
value = load(value)
if settings[SETTING_ARRAY_TYPE] == "color":
value = Color.from_string(value, Color.WHITE)
array.append(value)
if dict.has(SERIALIZED_VALUES) and dict.has(SERIALIZED_VALUES_TYPE):
var values_type = (
PandoraPropertyType.lookup(settings[SETTING_ARRAY_TYPE]) if not settings.is_empty()
else PandoraPropertyType.lookup(dict[SERIALIZED_VALUES_TYPE])
)
for i in range(dict[SERIALIZED_VALUES].size()):
var value = dict[SERIALIZED_VALUES][str(i)]
array.append(values_type.parse_value(value))

else:
for i in range(dict.size()):
var value = dict[str(i)]
if not settings.is_empty():
if settings[SETTING_ARRAY_TYPE] == "reference":
value = PandoraReference.new(value["_entity_id"], value["_type"]).get_entity()
if settings[SETTING_ARRAY_TYPE] == "resource":
value = load(value)
if settings[SETTING_ARRAY_TYPE] == "color":
value = Color.from_string(value, Color.WHITE)
array.append(value)
return array
return variant

func write_value(variant:Variant) -> Variant:
var array = variant as Array
var dict = {}
var dict = {
SERIALIZED_VALUES: {},
SERIALIZED_VALUES_TYPE: ""
}
if not array.is_empty():
var types = PandoraPropertyType.get_all_types()
var values_type : PandoraPropertyType
for i in range(array.size()):
var value = array[i]
if value is PandoraEntity:
value = PandoraReference.new(value.get_entity_id(), PandoraReference.Type.CATEGORY if value is PandoraCategory else PandoraReference.Type.ENTITY).save_data()
elif value is Resource:
value = value.resource_path
elif value is Color:
value = value.to_html()

if value != null:
dict[str(i)] = value
var found_valid_type = false
for type in types:
if type.is_valid(value):
dict[SERIALIZED_VALUES][str(i)] = type.write_value(value)
if values_type == null:
values_type = type
elif values_type != type:
# Array contains mixed-type values
values_type = UndefinedType.new()
found_valid_type = true
break
if not found_valid_type:
# Array contains a value for which no valid type could be found
dict[SERIALIZED_VALUES][str(i)] = value
values_type = UndefinedType.new()
dict[SERIALIZED_VALUES_TYPE] = values_type.get_type_name()
return dict

func allow_nesting() -> bool:
Expand Down
12 changes: 12 additions & 0 deletions test/model/property_test.gd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,18 @@ func test_array_property_wrong_type() -> void:
new_property.load_data(property.save_data())
assert_that(new_property.get_default_value()).is_equal("")

func test_array_property_custom_parsers() -> void:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we probably also want a test that verifies that when the array is saved to json and then re-read, that the type is still there and not lost somehow. Could we extend one of the other existing tests (in entity backend) that deal with that sort of use case by adding a typed array there?

var array_type = load("res://addons/pandora/model/types/array.gd")
var category = Pandora.create_category("Test Category")
var property = Pandora.create_property(category, "property", "array")
property.set_setting_override(array_type.SETTING_ARRAY_TYPE, "color")
var entity = Pandora.create_entity("entity", category)
var entity_property = entity.get_entity_property("property")
entity_property.set_default_value([Color.WHITE])
entity.load_data(entity.save_data())
assert_that(entity.get_array("property")[0]).is_equal(Color.WHITE)
assert_that(typeof(entity.get_array("property")[0])).is_not_equal(TYPE_STRING)

func test_vector2_property() -> void:
var vector = Vector2.ONE
var property = PandoraProperty.new("123", "property", "vector2")
Expand Down
Loading