Merge pull request #290 from endlessm/T35547-spawner-compat

Handle SimpleSpawner compatibility

addons/block_code/serialization/block_serialization.gd

@@ -1,13 +1,71 @@
+@tool
 extends Resource
 
 const BlockSerialization = preload("res://addons/block_code/serialization/block_serialization.gd")
 
-@export var name: StringName
+@export var name: StringName:
+	set = _set_name
 @export var children: Array[BlockSerialization]
-@export var arguments: Dictionary  # String, ValueBlockSerialization
+@export var arguments: Dictionary:  # String, ValueBlockSerialization
+	set = _set_arguments
 
 
 func _init(p_name: StringName = &"", p_children: Array[BlockSerialization] = [], p_arguments: Dictionary = {}):
 	name = p_name
 	children = p_children
 	arguments = p_arguments
+
+
+# Block name and arguments backwards compatibility handling.
+const _renamed_blocks: Dictionary = {
+	&"simplespawner_set_spawn_frequency": &"simplespawner_set_spawn_period",
+}
+
+
+func _set_name(value):
+	var new_name = _renamed_blocks.get(value)
+	if new_name:
+		print("Migrating block %s to new name %s" % [value, new_name])
+		name = new_name
+		if Engine.is_editor_hint():
+			EditorInterface.mark_scene_as_unsaved()
+	else:
+		name = value
+
+
+const _renamed_arguments: Dictionary = {
+	&"simplespawner_set_spawn_period":
+	{
+		"new_frequency": "new_period",
+	},
+}
+
+
+func _set_arguments(value):
+	if not value is Dictionary:
+		return
+
+	var renamed_args = _renamed_arguments.get(name)
+	if not renamed_args:
+		# Try with the new block name if it hasn't been migrated yet.
+		var new_block_name = _renamed_blocks.get(name)
+		if new_block_name:
+			renamed_args = _renamed_arguments.get(new_block_name)
+
+	if renamed_args:
+		var changed: bool = false
+		value = value.duplicate()
+		for old_arg in renamed_args.keys():
+			if not old_arg in value:
+				continue
+
+			var new_arg = renamed_args[old_arg]
+			print("Migrating block %s argument %s to new name %s" % [name, old_arg, new_arg])
+			value[new_arg] = value[old_arg]
+			value.erase(old_arg)
+			changed = true
+
+		if changed and Engine.is_editor_hint():
+			EditorInterface.mark_scene_as_unsaved()
+
+	arguments = value

addons/block_code/serialization/value_block_serialization.gd

@@ -1,9 +1,62 @@
+@tool
 extends Resource
 
-@export var name: StringName
-@export var arguments: Dictionary  # String, ValueBlockSerialization
+@export var name: StringName:
+	set = _set_name
+@export var arguments: Dictionary:  # String, ValueBlockSerialization
+	set = _set_arguments
 
 
 func _init(p_name: StringName = &"", p_arguments: Dictionary = {}):
 	name = p_name
 	arguments = p_arguments
+
+
+# Block name and arguments backwards compatibility handling.
+const _renamed_blocks: Dictionary = {
+	&"simplespawner_get_spawn_frequency": &"simplespawner_get_spawn_period",
+}
+
+
+func _set_name(value):
+	var new_name = _renamed_blocks.get(value)
+	if new_name:
+		print("Migrating block %s to new name %s" % [value, new_name])
+		name = new_name
+		if Engine.is_editor_hint():
+			EditorInterface.mark_scene_as_unsaved()
+	else:
+		name = value
+
+
+const _renamed_arguments: Dictionary = {}
+
+
+func _set_arguments(value):
+	if not value is Dictionary:
+		return
+
+	var renamed_args = _renamed_arguments.get(name)
+	if not renamed_args:
+		# Try with the new block name if it hasn't been migrated yet.
+		var new_block_name = _renamed_blocks.get(name)
+		if new_block_name:
+			renamed_args = _renamed_arguments.get(new_block_name)
+
+	if renamed_args:
+		var changed: bool = false
+		value = value.duplicate()
+		for old_arg in renamed_args.keys():
+			if not old_arg in value:
+				continue
+
+			var new_arg = renamed_args[old_arg]
+			print("Migrating block %s argument %s to new name %s" % [name, old_arg, new_arg])
+			value[new_arg] = value[old_arg]
+			value.erase(old_arg)
+			changed = true
+
+		if changed and Engine.is_editor_hint():
+			EditorInterface.mark_scene_as_unsaved()
+
+	arguments = value

addons/block_code/simple_spawner/simple_spawner.gd

@@ -168,3 +168,40 @@ static func setup_custom_blocks():
 	block_list.append(block_definition)
 
 	BlocksCatalog.add_custom_blocks(_class_name, block_list, [], {})
+
+
+# Backwards compatibility handling
+func _get_property_list() -> Array[Dictionary]:
+	return [
+		{
+			# spawn_frequency was renamed to spawn_period
+			"name": "spawn_frequency",
+			"class_name": &"",
+			"type": TYPE_FLOAT,
+			"hint": PROPERTY_HINT_NONE,
+			"hint_string": "",
+			"usage": PROPERTY_USAGE_NONE,
+		},
+	]
+
+
+func _get(property: StringName) -> Variant:
+	match property:
+		"spawn_frequency":
+			return spawn_period
+		_:
+			return null
+
+
+func _set(property: StringName, value: Variant) -> bool:
+	match property:
+		"spawn_frequency":
+			print("Migrating SimpleSpawner spawn_frequency property to new name spawn_period")
+			spawn_period = value
+		_:
+			return false
+
+	# Any migrated properties need to be resaved.
+	if Engine.is_editor_hint():
+		EditorInterface.mark_scene_as_unsaved()
+	return true

tests/data/simple_spawner_compat.tscn

@@ -0,0 +1,66 @@
+[gd_scene load_steps=14 format=3 uid="uid://br3g0jiuidqmb"]
+
+[ext_resource type="Script" path="res://addons/block_code/simple_spawner/simple_spawner.gd" id="1_liuhg"]
+[ext_resource type="Script" path="res://addons/block_code/block_code_node/block_code.gd" id="2_xxmhe"]
+[ext_resource type="Script" path="res://addons/block_code/serialization/block_serialization_tree.gd" id="3_4xc0e"]
+[ext_resource type="Script" path="res://addons/block_code/serialization/block_serialization.gd" id="4_ao2cd"]
+[ext_resource type="Script" path="res://addons/block_code/serialization/value_block_serialization.gd" id="5_387nk"]
+[ext_resource type="Script" path="res://addons/block_code/serialization/block_script_serialization.gd" id="5_a3p2k"]
+[ext_resource type="Script" path="res://addons/block_code/code_generation/variable_definition.gd" id="6_1vau2"]
+
+[sub_resource type="Resource" id="Resource_kt8ln"]
+script = ExtResource("4_ao2cd")
+name = &"simplespawner_set_spawn_frequency"
+children = Array[ExtResource("4_ao2cd")]([])
+arguments = {
+"new_frequency": 10.0
+}
+
+[sub_resource type="Resource" id="Resource_4x50y"]
+script = ExtResource("5_387nk")
+name = &"simplespawner_get_spawn_frequency"
+arguments = {}
+
+[sub_resource type="Resource" id="Resource_ar5am"]
+script = ExtResource("4_ao2cd")
+name = &"print"
+children = Array[ExtResource("4_ao2cd")]([])
+arguments = {
+"text": SubResource("Resource_4x50y")
+}
+
+[sub_resource type="Resource" id="Resource_yeqkk"]
+script = ExtResource("4_ao2cd")
+name = &"ready"
+children = Array[ExtResource("4_ao2cd")]([SubResource("Resource_kt8ln"), SubResource("Resource_ar5am")])
+arguments = {}
+
+[sub_resource type="Resource" id="Resource_o6xkk"]
+script = ExtResource("3_4xc0e")
+root = SubResource("Resource_yeqkk")
+canvas_position = Vector2(54, 47)
+
+[sub_resource type="Resource" id="Resource_u6cb0"]
+script = ExtResource("5_a3p2k")
+script_inherits = "SimpleSpawner"
+block_serialization_trees = Array[ExtResource("3_4xc0e")]([SubResource("Resource_o6xkk")])
+variables = Array[ExtResource("6_1vau2")]([])
+generated_script = "extends SimpleSpawner
+
+
+func _ready():
+	do_set_spawn_frequency(10)
+	print((spawn_frequency))
+
+"
+version = 0
+
+[node name="Root" type="Node2D"]
+
+[node name="SimpleSpawner" type="Node2D" parent="."]
+script = ExtResource("1_liuhg")
+spawn_frequency = 5.0
+
+[node name="BlockCode" type="Node" parent="SimpleSpawner"]
+script = ExtResource("2_xxmhe")
+block_script = SubResource("Resource_u6cb0")

tests/test_compatibility.gd

@@ -0,0 +1,150 @@
+extends GutTest
+## Tests for scene file backwards compatibility
+
+
+func get_scene_state_node_index(state: SceneState, name: String) -> int:
+	for idx in state.get_node_count():
+		if state.get_node_name(idx) == name:
+			return idx
+	return -1
+
+
+func get_scene_state_node_prop_index(state: SceneState, node_idx: int, name: String) -> int:
+	for prop_idx in state.get_node_property_count(node_idx):
+		if state.get_node_property_name(node_idx, prop_idx) == name:
+			return prop_idx
+	return -1
+
+
+func _get_block_names_recursive(block: Resource, names: Array[String]):
+	names.append(block.name)
+	if "children" in block:
+		for child in block.children:
+			_get_block_names_recursive(child, names)
+	for value in block.arguments.values():
+		if not value is Resource:
+			continue
+		if not value.script:
+			continue
+		if value.script.resource_path != "res://addons/block_code/serialization/value_block_serialization.gd":
+			continue
+		_get_block_names_recursive(value, names)
+
+
+func get_block_script_block_names(block_script: Resource) -> Array[String]:
+	var names: Array[String]
+	for tree in block_script.block_serialization_trees:
+		_get_block_names_recursive(tree.root, names)
+	return names
+
+
+func _get_block_argument_names_recursive(block: Resource, names: Array[String]):
+	names.append_array(block.arguments.keys())
+	if "children" in block:
+		for child in block.children:
+			_get_block_argument_names_recursive(child, names)
+	for value in block.arguments.values():
+		if not value is Resource:
+			continue
+		if not value.script:
+			continue
+		if value.script.resource_path != "res://addons/block_code/serialization/value_block_serialization.gd":
+			continue
+		_get_block_argument_names_recursive(value, names)
+
+
+func get_block_script_argument_names(block_script: Resource) -> Array[String]:
+	var names: Array[String]
+	for tree in block_script.block_serialization_trees:
+		_get_block_argument_names_recursive(tree.root, names)
+	return names
+
+
+func test_simple_spawner():
+	const old_block_names: Array[String] = [
+		"simplespawner_get_spawn_frequency",
+		"simplespawner_set_spawn_frequency",
+	]
+
+	const new_block_names: Array[String] = [
+		"simplespawner_get_spawn_period",
+		"simplespawner_set_spawn_period",
+	]
+
+	const old_argument_names: Array[String] = [
+		"new_frequency",
+	]
+
+	const new_argument_names: Array[String] = [
+		"new_period",
+	]
+
+	var scene: PackedScene = load("res://tests/data/simple_spawner_compat.tscn")
+	assert_not_null(scene)
+	assert_true(scene.can_instantiate(), "Scene should be instantiable")
+
+	var scene_state := scene.get_state()
+	var spawner_idx := get_scene_state_node_index(scene_state, "SimpleSpawner")
+	assert_gt(spawner_idx, -1, "SimpleSpawner node could not be found")
+
+	# The packed SimpleSpawner node should have a simple_frequency
+	# property but no simple_period property.
+	var frequency_idx := get_scene_state_node_prop_index(scene_state, spawner_idx, "spawn_frequency")
+	var period_idx := get_scene_state_node_prop_index(scene_state, spawner_idx, "spawn_period")
+	assert_gt(frequency_idx, -1, "Old SimpleSpawner node should have spawn_frequency property")
+	assert_lt(period_idx, 0, "Old SimpleSpawner node should not have spawn_period property")
+
+	var packed_frequency = scene_state.get_node_property_value(spawner_idx, frequency_idx)
+	assert_typeof(packed_frequency, TYPE_FLOAT)
+	assert_eq(packed_frequency, 5.0)
+
+	var block_code_idx := get_scene_state_node_index(scene_state, "BlockCode")
+	assert_gt(block_code_idx, -1, "BlockCode node could not be found")
+	var block_script_idx := get_scene_state_node_prop_index(scene_state, block_code_idx, "block_script")
+	assert_gt(block_script_idx, -1, "BlockCode node block_script could not be found")
+	var packed_block_script = scene_state.get_node_property_value(block_code_idx, block_script_idx)
+	assert_typeof(packed_block_script, TYPE_OBJECT)
+	assert_eq("Resource", packed_block_script.get_class())
+	assert_eq(packed_block_script.script.resource_path, "res://addons/block_code/serialization/block_script_serialization.gd")
+
+	# Unlike Nodes, Resources are created immediately when loading the
+	# scene, so the block names and arguments should already be migrated.
+	var packed_block_names := get_block_script_block_names(packed_block_script)
+	for name in old_block_names:
+		assert_does_not_have(packed_block_names, name, "Block script should not have old name %s" % name)
+	for name in new_block_names:
+		assert_has(packed_block_names, name, "Block script should have new name %s" % name)
+
+	var packed_argument_names := get_block_script_argument_names(packed_block_script)
+	for name in old_argument_names:
+		assert_does_not_have(packed_argument_names, name, "Block script should not have old argument %s" % name)
+	for name in new_argument_names:
+		assert_has(packed_argument_names, name, "Block script should have new argument %s" % name)
+
+	# Instantiate the scene and check the Node properties.
+	var root := scene.instantiate()
+	assert_not_null(root)
+	autoqfree(root)
+
+	var spawner: SimpleSpawner = root.get_node("SimpleSpawner")
+	assert_eq(spawner.spawn_frequency, 5.0)
+	assert_eq(spawner.spawn_period, 5.0)
+
+	# Pack the scene and check that the old properties won't be saved.
+	var err: Error = scene.pack(root)
+	assert_eq(err, OK, "Packing scene should not cause an error")
+
+	scene_state = scene.get_state()
+	spawner_idx = get_scene_state_node_index(scene_state, "SimpleSpawner")
+	assert_gt(spawner_idx, -1, "SimpleSpawner node could not be found")
+
+	# The newly packed SimpleSpawner node should have a simple_period
+	# property but no simple_frequency property.
+	period_idx = get_scene_state_node_prop_index(scene_state, spawner_idx, "spawn_period")
+	frequency_idx = get_scene_state_node_prop_index(scene_state, spawner_idx, "spawn_frequency")
+	assert_gt(period_idx, -1, "New SimpleSpawner node should have spawn_period property")
+	assert_lt(frequency_idx, 0, "New SimpleSpawner node should not have spawn_frequency property")
+
+	var packed_period = scene_state.get_node_property_value(spawner_idx, period_idx)
+	assert_typeof(packed_period, TYPE_FLOAT)
+	assert_eq(packed_period, 5.0)