Rename translate(d) to translate(d)_local in Transform 2D/3D

[bluenote10]

This is an accompanying PR for #55923 / godotengine/godot-proposals#1336.

It isolates the renaming for translated to translated_local (and in-place versions) without re-adding the same function with the new global semantics in the same PR to ensure that we have refactored 100% of occurrences if CI passes. This PR does nothing else.

If we want to move forwards with #55923 the idea would be to merge this PR first, and then I'll rebase the other PR, which adds all remaining variants of translated/scaled/rotated. This will simplify the other diff and should make its review more convenient.

[aaronfranke]

Looks good except we should also do the same change in C#.

https://github.com/godotengine/godot/blob/master/modules/mono/glue/GodotSharp/GodotSharp/Core/Transform2D.cs#L350

https://github.com/godotengine/godot/blob/master/modules/mono/glue/GodotSharp/GodotSharp/Core/Transform3D.cs#L242

[bluenote10]

Looks good except we should also do the same change in C#.

Thanks, good catch, adapted now as well. Was this missed by a CI check or is the API consistency towards C# only verified manually in general?

[aaronfranke]

@bluenote10 Only manually, we don't have any checks for this. At some point we should add test cases to C# which we can start by copying the ones from C++ (which would help verify equivalent behavior), but this is not a priority right now.

This PR looks good to me. Even without the rest of the discussed changes, this change makes sense on its own, so I think this is good, and then the rest of the changes can be in follow-up PRs.

[bluenote10]

@BastiaanOlij Hello Bastiaan, is there anything I can do to move forward with this PR and #55923? If it would e.g. help to have a call in person to walk you through the plan behind godotengine/godot-proposals#1336 I could surely arrange that.

[BastiaanOlij]

Hey @bluenote10,

Sorry for not noticing this until now, also very interested in knowing about your Guitar VR project so if you have a link to what you're working on, I'd love to check it out.

On topic, we had a big discussion some months ago about which way around things should be and what default behavior we expect. I can't fully remember what the outcome was there but it came down that the current approach was mostly correct.

The problem being that there is a big disjoint between what is expected behavior for the different functions and that the need for their functional behavior is more important than consistency in behavior.

I can't remember what the outcome of the naming there was, whether we agreed that it would be assume that transform was local, and global_transform global, I vaguely remember we did want to replace the global name because that name can be misleading and rotation and scaling on global_transform is still a locally applied (and should be).

So I'm not sure what the best forum is for getting consensus on the naming and whether we actually want to change anything here.

[aaronfranke]

@BastiaanOlij We had a discussion about this about a month ago. It was discussed that having both versions makes sense. I remember specifically that we brought up if there should be _global suffixes, but this was rejected because using the word global for parent-relative things was confusing, so it was decided that the self-relative versions should be named local and the parent-relative versions should have no suffix. Reduz commented on #55923 that it looks good as-is. However, there were some concerns in the discussion about mid-air conflicts with other PRs since it renames an existing method and adds a new one with the old name. This PR is a subset of the other one that only has the method rename to try and avoid mid-air conflicts.

See the discussions in #55923 and godotengine/godot-proposals#1336 for more information.

Also, these renames have nothing to do with transform or global_transform. This is about translate.

[bluenote10]

the need for their functional behavior is more important than consistency in behavior.

I think everyone involved in the discussion on various GitHub threads was agreeing that the current inconsistency with mixing local and global operations is a design flaw that should be fixed before 4.0. The main question then became how to resolve the inconsistency: Either by breaking the behavior of the two functions (scaled and rotated) or the behavior of only translated. What I'm suggesting is mainly motivated by pragmatism: Breaking just 1 function is better than breaking 2 functions, and going down that path even fits better to the existing code in Basis, which already has _local variants anyway. Going down the other path would be a much bigger breaking change.

[YuriSizov]

Mentioned in the PR review meeting, decided that @reduz would take a look separately.

[aaronfranke]

This still looks good to me. It needs a rebase when you can.

[akien-mga]

Checked against current master and I don't see new translate() / translated() calls added since the last rebase, so should be good to go.

[akien-mga]

Thanks!