Localized properties #393

manuq · 2025-06-05T19:01:26Z

Grabacion.de.pantalla.desde.2025-06-05.16-00-33.webm

manuq · 2025-06-06T17:43:41Z

The last commit makes the setter and changer blocks translatable:

Create three constructor functions and use them from the catalog.

manuq · 2025-06-07T01:36:58Z

Ready for review. I also managed to translate the high-level blocks in simple nodes:

This is with an es.po file modified locally. Once this is merged I will do it properly through Transifex.

wjt

Nice work! I spotted what looks like a logic error but I could be wrong – I haven't run the code to check.

wjt · 2025-06-07T08:36:40Z

addons/block_code/ui/blocks/block/block.gd

+	if definition.property_name:
+		var domain: TranslationDomain = TranslationServer.get_or_add_domain("godot.properties")
+		var translated_property: String = domain.translate(definition.property_name.capitalize())
+		# TODO: Ideally we should be also passing the context. See:
+		# https://github.com/godotengine/godot/blob/978b38797ba8e8757592f21101e32e364d60662d/editor/editor_property_name_processor.cpp#L90
+		return tr(definition.display_template) % translated_property.to_lower()
+
 	return tr(definition.display_template)


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!

wjt · 2025-06-07T08:49:21Z

addons/block_code/ui/block_canvas/block_canvas.gd

+		if has_property:
+			return_classname = parent_class_name
+			continue
+		else:
+			break


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.

wjt · 2025-06-07T08:50:19Z

addons/block_code/code_generation/block_definition.gd

@@ -215,11 +215,11 @@ static func new_property_setter(_class_name: String, property: Dictionary, categ
 	var block_definition: Resource = new(
 		PROPERTY_SETTER_NAME_FORMAT % [_class_name, property.name],
 		_class_name,
-		"Set the %s property" % property.name,
+		Engine.tr("Set the %s property") % property.name,


TIL. ¡Gracias, Godot Meetup Argentina!

manuq · 2025-06-09T14:57:08Z

addons/block_code/code_generation/block_definition.gd

+const PROPERTY_SETTER_NAME_FORMAT = &"%s_property_set_%s"
+const PROPERTY_CHANGER_NAME_PATTERN = "(?<class_name>[^\\s]*)_property_change_(?<property_name>[^\\s]+)"
+const PROPERTY_CHANGER_NAME_FORMAT = &"%s_property_change_%s"
+const PROPERTY_GETTER_NAME_PATTERN = "(?<class_name>[^\\s]*)_property_get_(?<property_name>[^\\s]+)"
+const PROPERTY_GETTER_NAME_FORMAT = &"%s_property_get_%s"


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.

Display localized properties whenever possible. Just like the Inspector does when "Localized" is set from the dropdown menu. For this the property name must be stored in the block definition. Once translated, it is added to the display template.

So they use the localized property name. And also, reuse the setter or getter if it's already in the catalog.

Since this is a static function, tr() can't be called from it. But Engine.tr() can. Thanks to Godot Meetup Argentina for unblocking me on this.

Create two constructor functions and use them from the catalog.

Set auto translate to disabled in this scene. This scene has a debug label that is not worth translating.

And log an error when a persisted block can't be parsed.

Mark display template and description as translatable.

These files are not meant to be translated.

There are no breaking changes, so the blocks stay the same.

Move property setter, changer and getter to BlockDefinition

d3a1599

Create three constructor functions and use them from the catalog.

manuq force-pushed the localized-properties branch 2 times, most recently from 883aad6 to 7f704f6 Compare June 7, 2025 01:32

manuq marked this pull request as ready for review June 7, 2025 01:37

manuq requested review from wjt, dylanmccall and dbnicholson June 7, 2025 01:37

wjt requested changes Jun 7, 2025

View reviewed changes

manuq commented Jun 9, 2025

View reviewed changes

manuq added 10 commits June 9, 2025 15:26

Translate properties

7ecd7cb

Display localized properties whenever possible. Just like the Inspector does when "Localized" is set from the dropdown menu. For this the property name must be stored in the block definition. Once translated, it is added to the display template.

Unify property getters and setters done through drag and drop

362c5b8

So they use the localized property name. And also, reuse the setter or getter if it's already in the catalog.

Add localization to property setter and changer

4859cab

Since this is a static function, tr() can't be called from it. But Engine.tr() can. Thanks to Godot Meetup Argentina for unblocking me on this.

Move variable setter and getter to BlockDefinition

947bd28

Create two constructor functions and use them from the catalog.

Spawner example: Do not translate

62683cb

Set auto translate to disabled in this scene. This scene has a debug label that is not worth translating.

Remove deprecated way of persisting parameter blocks

f931209

And log an error when a persisted block can't be parsed.

Simple nodes: Localize

5610402

Mark display template and description as translatable.

Remove example scenes from translated files

38bd90f

These files are not meant to be translated.

Update POT file

d71eff6

Update Pong example

e800b5e

There are no breaking changes, so the blocks stay the same.

manuq force-pushed the localized-properties branch from 7f704f6 to e800b5e Compare June 9, 2025 18:29

manuq merged commit af6cdd8 into main Jun 9, 2025
3 checks passed

manuq deleted the localized-properties branch June 9, 2025 18:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Localized properties #393

Localized properties #393

Uh oh!

manuq commented Jun 5, 2025

Uh oh!

manuq commented Jun 6, 2025

Uh oh!

manuq commented Jun 7, 2025

Uh oh!

wjt left a comment

Uh oh!

wjt Jun 7, 2025

Uh oh!

manuq Jun 9, 2025

Uh oh!

manuq Jun 9, 2025

Uh oh!

wjt Jun 7, 2025

Uh oh!

manuq Jun 9, 2025

Uh oh!

wjt Jun 9, 2025

Uh oh!

manuq Jun 9, 2025

Uh oh!

wjt Jun 7, 2025

Uh oh!

manuq Jun 9, 2025

Uh oh!

manuq Jun 9, 2025

Uh oh!

Uh oh!

Uh oh!

Localized properties #393

Localized properties #393

Uh oh!

Conversation

manuq commented Jun 5, 2025

Uh oh!

manuq commented Jun 6, 2025

Uh oh!

manuq commented Jun 7, 2025

Uh oh!

wjt left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!