I might add a note to the display_template docstring to the effect that

## If [member property_name] is set, this template is assumed to be a format string with a `%s` placeholder; in this case, any literal `%` signs must be escaped as `%%`.

You could imagine this being a problem because % is a unit that a property might have. (I think it's fine to do it this way.)

Oh good point. It is a bit weird that this PR adds 2 paths for display_template, one that formats the translated string and one that doesn't. At least let's document it.

Addressed, thanks for the wording!

Are the continue and break here the wrong way around?

I think what you're trying to do here is to walk up the ancestor classes and return the first class that defines a property of that name. If D is a subclass of C is a subclass of B is a subclass of A, and B and A both define property foo then _get_classname_for_property("C", "foo") should return "B", as should _get_classname_for_property("D", "foo").

If that's correct then I believe this function is wrong:

• _get_classname_for_property("C", "foo") will return "A"
• _get_classname_for_property("D", "foo") will return "" because it breaks out of the loop at the first class that does not have the property

Assuming I'm right here, you can swap continue and break and the function will behave correctly.

Personally I find the control flow easier to read by returning from the body of the loop, rather than having a var return_classname and returning only at the end of the function:

static func _get_classname_for_property(_class_name: StringName, property_name: StringName) -> StringName:
	for parent_class_name in BlocksCatalog.get_parents(_class_name):
		var property_list := ClassDB.class_get_property_list(parent_class_name, false)
		var has_property = property_list.any(func(dict): return dict.name == property_name)
		if has_property:
			return parent_class_name

	return &""

This is doing the right thing but in a weird way. I'm listing with inheritance (passing to the 2nd argument no_inheritance = false ), and then recording the last class that had the property, then breaking. Instead I should list without inheritance (passing true) and return the first class that has the property. The parameter no_inheritance being negated confused me.

This function also screams for a unit test so I will try to add one.

Oh! I completely missed the false parameter. I don't like the design of that API: boolean parameter with negated sense!

I did this in a TDD fashion: first I wrote the unit test, then I changed the implementation and confirmed that the test was still passing.

TIL. ¡Gracias, Godot Meetup Argentina!

Now that I read the current unit tests I remember that we have a migration for the spawn blocks. I wonder if I should either add a migration to the previous block names like CLASSNAME_set_PROPERTYNAME, or just go back to those block names and avoid the breaking change. I think I'll go with the latter.

So yes, I managed to make the PR not having any breaking changes, and rebased.