Rename properties unnecessarily using slash (/) in their names and cleanup Joint3D angular limit bindings #64719
Conversation
akien-mga
force-pushed
the
property-slasher
branch
2 times, most recently
from
7303b7a to
63c62f0
Compare
akien-mga
force-pushed
the
property-slasher
branch
from
63c62f0 to
779e82e
Compare
|
There's been a PR that does some more renames like that (#64553), but it doesn't seem these two have conflicts. |
| @@ -179,6 +173,12 @@ | |||
| <member name="moving_platform_wall_layers" type="int" setter="set_moving_platform_wall_layers" getter="get_moving_platform_wall_layers" default="0"> | |||
| Collision layers that will be included for detecting wall bodies that will act as moving platforms to be followed by the [CharacterBody2D]. By default, all wall bodies are ignored. | |||
| </member> | |||
| <member name="safe_margin" type="float" setter="set_safe_margin" getter="get_safe_margin" default="0.08"> | |||
There was a problem hiding this comment.
Should it be named collision_safe_margin instead? I don't have a strong opinion on it, but it may be less confusing. Same for CharacterBody3D.
There was a problem hiding this comment.
I wondered too, but since the setter was set_safe_margin, and that's the only direct way this could be used until now, I opted for that one. If we prefer to use collision_safe_margin, then the setter and getter should be renamed too. I'm fine with either - since it's a physics body I think it's implicit that a margin refers to collision with other bodies, but we can make it explicit too.
We can also add back a Collision group for this property, with or without prefix, to get back the ordering of before this PR. But it didn't seem particularly useful to me.
CC @godotengine/physics
There was a problem hiding this comment.
I find the "collision_safe_margin" (maybe biased with habit) seems a bit long, however, it is a parameter that rarely needs to be changed, being in a Collision group a bit apart seems relevant to me, so we can rename it for me (in his group).
There was a problem hiding this comment.
So you'd suggest renaming to collision_safe_margin and adding a "Collision" group? (And then also rename the setter/getter to set/get_collision_safe_margin)
There was a problem hiding this comment.
I went with an intermediate approach to keep the short safe_margin name (internally it's even just margin, and safe_margin is used as parameter name in move_and_slide), and just moved it back to the end in a prefix-less "Collision" group.
There was a problem hiding this comment.
I wasn't entirely convinced tbh, this morning I thought of collision_margin (safe is a bit useless), your solution avoid a breaking change.
There was a problem hiding this comment.
And btw, there are some pretty long names currently in the properties, in the "floor" or "moving_platform" group, if you have an opinion/suggestion (or reduz) on that.
This is a legacy of Godot 2 days before the inspector had support for groups. "Properties" with a slash in their name can't be accessed from script unless using `set()`/`get()` so they were not actual properties as far as script languages are concerned. Part of godotengine#17558.
The inspector now supports converting degrees to radians automatically when using the `radians` hint, so all those utility bindings were redundant. This cleans things up by making these properties with slash properly bound to `set_param`/`get_param` which the users can call with the relevant enum.
akien-mga
force-pushed
the
property-slasher
branch
from
779e82e to
2adfcc6
Compare
This is a legacy of Godot 2 days before the inspector had support for groups.
"Properties" with a slash in their name can't be accessed from script unless
using
set()/get()so they were not actual properties as far as scriptlanguages are concerned.
Joint3D: Remove utility method bindings for angular limits
The inspector now supports converting degrees to radians automatically when
using the
radianshint, so all those utility bindings were redundant.This cleans things up by making these properties with slash properly bound
to
set_param/get_paramwhich the users can call with the relevant enum.