Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

PhysicalBone 6DOF set/get doesn't check if a property starts with joint_constraints #44320

Closed
Demindiro opened this issue Dec 12, 2020 · 5 comments · Fixed by #45267
Closed

PhysicalBone 6DOF set/get doesn't check if a property starts with joint_constraints #44320

Demindiro opened this issue Dec 12, 2020 · 5 comments · Fixed by #45267
Labels
bug topic:physics
Milestone
4.0

Comments

@Demindiro
Copy link
Contributor

Godot version:

3.2.3.stable.custom_build.662455ee8

OS/device including version:

$ uname -a
Linux pc 4.19.0-12-amd64 #1 SMP Debian 4.19.152-1 (2020-10-18) x86_64 GNU/Linux

Issue description:
PhysicalBone doesn't check whether the first part of a property is "joint_constraints" in set and get if joint_type == JOINT_TYPE_6DOF

godot/scene/3d/physics_body_3d.cpp

Lines 1712 to 1866 in 0762484

bool PhysicalBone3D::SixDOFJointData::_set(const StringName &p_name, const Variant &p_value, RID j) {
if (JointData::_set(p_name, p_value, j)) {
return true;
}
String path = p_name;
Vector3::Axis axis;
{
const String axis_s = path.get_slicec('/', 1);
if ("x" == axis_s) {
axis = Vector3::AXIS_X;
} else if ("y" == axis_s) {
axis = Vector3::AXIS_Y;
} else if ("z" == axis_s) {
axis = Vector3::AXIS_Z;
} else {
return false;
}
}
String var_name = path.get_slicec('/', 2);
if ("linear_limit_enabled" == var_name) {
axis_data[axis].linear_limit_enabled = p_value;
if (j.is_valid()) {
PhysicsServer3D::get_singleton()->generic_6dof_joint_set_flag(j, axis, PhysicsServer3D::G6DOF_JOINT_FLAG_ENABLE_LINEAR_LIMIT, axis_data[axis].linear_limit_enabled);
}
} else if ("linear_limit_upper" == var_name) {
axis_data[axis].linear_limit_upper = p_value;
if (j.is_valid()) {
PhysicsServer3D::get_singleton()->generic_6dof_joint_set_param(j, axis, PhysicsServer3D::G6DOF_JOINT_LINEAR_UPPER_LIMIT, axis_data[axis].linear_limit_upper);
}
} else if ("linear_limit_lower" == var_name) {
axis_data[axis].linear_limit_lower = p_value;
if (j.is_valid()) {
PhysicsServer3D::get_singleton()->generic_6dof_joint_set_param(j, axis, PhysicsServer3D::G6DOF_JOINT_LINEAR_LOWER_LIMIT, axis_data[axis].linear_limit_lower);
}
} else if ("linear_limit_softness" == var_name) {
axis_data[axis].linear_limit_softness = p_value;
if (j.is_valid()) {
PhysicsServer3D::get_singleton()->generic_6dof_joint_set_param(j, axis, PhysicsServer3D::G6DOF_JOINT_LINEAR_LIMIT_SOFTNESS, axis_data[axis].linear_limit_softness);
}
} else if ("linear_spring_enabled" == var_name) {
axis_data[axis].linear_spring_enabled = p_value;
if (j.is_valid()) {
PhysicsServer3D::get_singleton()->generic_6dof_joint_set_flag(j, axis, PhysicsServer3D::G6DOF_JOINT_FLAG_ENABLE_LINEAR_SPRING, axis_data[axis].linear_spring_enabled);
}
} else if ("linear_spring_stiffness" == var_name) {
axis_data[axis].linear_spring_stiffness = p_value;
if (j.is_valid()) {
PhysicsServer3D::get_singleton()->generic_6dof_joint_set_param(j, axis, PhysicsServer3D::G6DOF_JOINT_LINEAR_SPRING_STIFFNESS, axis_data[axis].linear_spring_stiffness);
}
} else if ("linear_spring_damping" == var_name) {
axis_data[axis].linear_spring_damping = p_value;
if (j.is_valid()) {
PhysicsServer3D::get_singleton()->generic_6dof_joint_set_param(j, axis, PhysicsServer3D::G6DOF_JOINT_LINEAR_SPRING_DAMPING, axis_data[axis].linear_spring_damping);
}
} else if ("linear_equilibrium_point" == var_name) {
axis_data[axis].linear_equilibrium_point = p_value;
if (j.is_valid()) {
PhysicsServer3D::get_singleton()->generic_6dof_joint_set_param(j, axis, PhysicsServer3D::G6DOF_JOINT_LINEAR_SPRING_EQUILIBRIUM_POINT, axis_data[axis].linear_equilibrium_point);
}
} else if ("linear_restitution" == var_name) {
axis_data[axis].linear_restitution = p_value;
if (j.is_valid()) {
PhysicsServer3D::get_singleton()->generic_6dof_joint_set_param(j, axis, PhysicsServer3D::G6DOF_JOINT_LINEAR_RESTITUTION, axis_data[axis].linear_restitution);
}
} else if ("linear_damping" == var_name) {
axis_data[axis].linear_damping = p_value;
if (j.is_valid()) {
PhysicsServer3D::get_singleton()->generic_6dof_joint_set_param(j, axis, PhysicsServer3D::G6DOF_JOINT_LINEAR_DAMPING, axis_data[axis].linear_damping);
}
} else if ("angular_limit_enabled" == var_name) {
axis_data[axis].angular_limit_enabled = p_value;
if (j.is_valid()) {
PhysicsServer3D::get_singleton()->generic_6dof_joint_set_flag(j, axis, PhysicsServer3D::G6DOF_JOINT_FLAG_ENABLE_ANGULAR_LIMIT, axis_data[axis].angular_limit_enabled);
}
} else if ("angular_limit_upper" == var_name) {
axis_data[axis].angular_limit_upper = Math::deg2rad(real_t(p_value));
if (j.is_valid()) {
PhysicsServer3D::get_singleton()->generic_6dof_joint_set_param(j, axis, PhysicsServer3D::G6DOF_JOINT_ANGULAR_UPPER_LIMIT, axis_data[axis].angular_limit_upper);
}
} else if ("angular_limit_lower" == var_name) {
axis_data[axis].angular_limit_lower = Math::deg2rad(real_t(p_value));
if (j.is_valid()) {
PhysicsServer3D::get_singleton()->generic_6dof_joint_set_param(j, axis, PhysicsServer3D::G6DOF_JOINT_ANGULAR_LOWER_LIMIT, axis_data[axis].angular_limit_lower);
}
} else if ("angular_limit_softness" == var_name) {
axis_data[axis].angular_limit_softness = p_value;
if (j.is_valid()) {
PhysicsServer3D::get_singleton()->generic_6dof_joint_set_param(j, axis, PhysicsServer3D::G6DOF_JOINT_ANGULAR_LIMIT_SOFTNESS, axis_data[axis].angular_limit_softness);
}
} else if ("angular_restitution" == var_name) {
axis_data[axis].angular_restitution = p_value;
if (j.is_valid()) {
PhysicsServer3D::get_singleton()->generic_6dof_joint_set_param(j, axis, PhysicsServer3D::G6DOF_JOINT_ANGULAR_RESTITUTION, axis_data[axis].angular_restitution);
}
} else if ("angular_damping" == var_name) {
axis_data[axis].angular_damping = p_value;
if (j.is_valid()) {
PhysicsServer3D::get_singleton()->generic_6dof_joint_set_param(j, axis, PhysicsServer3D::G6DOF_JOINT_ANGULAR_DAMPING, axis_data[axis].angular_damping);
}
} else if ("erp" == var_name) {
axis_data[axis].erp = p_value;
if (j.is_valid()) {
PhysicsServer3D::get_singleton()->generic_6dof_joint_set_param(j, axis, PhysicsServer3D::G6DOF_JOINT_ANGULAR_ERP, axis_data[axis].erp);
}
} else if ("angular_spring_enabled" == var_name) {
axis_data[axis].angular_spring_enabled = p_value;
if (j.is_valid()) {
PhysicsServer3D::get_singleton()->generic_6dof_joint_set_flag(j, axis, PhysicsServer3D::G6DOF_JOINT_FLAG_ENABLE_ANGULAR_SPRING, axis_data[axis].angular_spring_enabled);
}
} else if ("angular_spring_stiffness" == var_name) {
axis_data[axis].angular_spring_stiffness = p_value;
if (j.is_valid()) {
PhysicsServer3D::get_singleton()->generic_6dof_joint_set_param(j, axis, PhysicsServer3D::G6DOF_JOINT_ANGULAR_SPRING_STIFFNESS, axis_data[axis].angular_spring_stiffness);
}
} else if ("angular_spring_damping" == var_name) {
axis_data[axis].angular_spring_damping = p_value;
if (j.is_valid()) {
PhysicsServer3D::get_singleton()->generic_6dof_joint_set_param(j, axis, PhysicsServer3D::G6DOF_JOINT_ANGULAR_SPRING_DAMPING, axis_data[axis].angular_spring_damping);
}
} else if ("angular_equilibrium_point" == var_name) {
axis_data[axis].angular_equilibrium_point = p_value;
if (j.is_valid()) {
PhysicsServer3D::get_singleton()->generic_6dof_joint_set_param(j, axis, PhysicsServer3D::G6DOF_JOINT_ANGULAR_SPRING_EQUILIBRIUM_POINT, axis_data[axis].angular_equilibrium_point);
}
} else {
return false;
}
return true;
}

godot/scene/3d/physics_body_3d.cpp

Lines 1868 to 1938 in 0762484

bool PhysicalBone3D::SixDOFJointData::_get(const StringName &p_name, Variant &r_ret) const {
if (JointData::_get(p_name, r_ret)) {
return true;
}
String path = p_name;
int axis;
{
const String axis_s = path.get_slicec('/', 1);
if ("x" == axis_s) {
axis = 0;
} else if ("y" == axis_s) {
axis = 1;
} else if ("z" == axis_s) {
axis = 2;
} else {
return false;
}
}
String var_name = path.get_slicec('/', 2);
if ("linear_limit_enabled" == var_name) {
r_ret = axis_data[axis].linear_limit_enabled;
} else if ("linear_limit_upper" == var_name) {
r_ret = axis_data[axis].linear_limit_upper;
} else if ("linear_limit_lower" == var_name) {
r_ret = axis_data[axis].linear_limit_lower;
} else if ("linear_limit_softness" == var_name) {
r_ret = axis_data[axis].linear_limit_softness;
} else if ("linear_spring_enabled" == var_name) {
r_ret = axis_data[axis].linear_spring_enabled;
} else if ("linear_spring_stiffness" == var_name) {
r_ret = axis_data[axis].linear_spring_stiffness;
} else if ("linear_spring_damping" == var_name) {
r_ret = axis_data[axis].linear_spring_damping;
} else if ("linear_equilibrium_point" == var_name) {
r_ret = axis_data[axis].linear_equilibrium_point;
} else if ("linear_restitution" == var_name) {
r_ret = axis_data[axis].linear_restitution;
} else if ("linear_damping" == var_name) {
r_ret = axis_data[axis].linear_damping;
} else if ("angular_limit_enabled" == var_name) {
r_ret = axis_data[axis].angular_limit_enabled;
} else if ("angular_limit_upper" == var_name) {
r_ret = Math::rad2deg(axis_data[axis].angular_limit_upper);
} else if ("angular_limit_lower" == var_name) {
r_ret = Math::rad2deg(axis_data[axis].angular_limit_lower);
} else if ("angular_limit_softness" == var_name) {
r_ret = axis_data[axis].angular_limit_softness;
} else if ("angular_restitution" == var_name) {
r_ret = axis_data[axis].angular_restitution;
} else if ("angular_damping" == var_name) {
r_ret = axis_data[axis].angular_damping;
} else if ("erp" == var_name) {
r_ret = axis_data[axis].erp;
} else if ("angular_spring_enabled" == var_name) {
r_ret = axis_data[axis].angular_spring_enabled;
} else if ("angular_spring_stiffness" == var_name) {
r_ret = axis_data[axis].angular_spring_stiffness;
} else if ("angular_spring_damping" == var_name) {
r_ret = axis_data[axis].angular_spring_damping;
} else if ("angular_equilibrium_point" == var_name) {
r_ret = axis_data[axis].angular_equilibrium_point;
} else {
return false;
}
return true;
}

Steps to reproduce:
Change PhysicalBone to PhysicalBone3D for Godot 4.0

extends SceneTree


var bone = PhysicalBone.new()
var t = 0


func _init():
	bone.joint_type = bone.JOINT_TYPE_6DOF
	root.add_child(bone)


func _idle(_delta):
	var x = bone.get("blahblah/x/angular_limit_upper")
	print(x)
	t += 1
	bone.set("foo/x/angular_limit_upper", t)

Minimal reproduction project:
See above
@Calinou Calinou mentioned this issue Dec 12, 2020
Closed
@Favkis
Copy link

Favkis commented Dec 19, 2020 •
edited
Loading

I don't get what's exactly wrong with .set code though?I really need to fix it but I don't see anything wrong with .set or .get code

I did search in all .cpp files in godot source files and JointData::_set happens only in scene/3d/physics_body.cpp, which is weird, shouldn't I have also found it in editor? That either means editor is not in .cpp or JointData::_set is not correct function name, because setting these params in editor do work just fine
image

@Demindiro
Copy link
Contributor Author

@Favkis have you seen #44535?

@pouleyKetchoupp
Copy link
Contributor

Junior job info:
SixDOFJointData::_get and SixDOFJointData::_set needs to be updated so bone.get("blahblah/x/angular_limit_upper") would fail and only bone.get("joint_constraints/x/angular_limit_upper") would work (same with set).

The current code uses const String axis_s = path.get_slicec('/', 1); to get to the axis part and String var_name = path.get_slicec('/', 2); to get to the property name part, by splitting the string based on separators. Instead, it should start with validating that the path starts with joint_constraints and then process the remaining part of the path.

@madmiraal
Copy link
Contributor

See #44876.

@pouleyKetchoupp
Copy link
Contributor

@madmiraal This issue doesn't have to wait for #44876.

This issue is still open for junior job.

@HipsterPenguin HipsterPenguin mentioned this issue Jan 17, 2021
Merged
@akien-mga akien-mga added this to the 4.0 milestone Jan 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug topic:physics
Projects
None yet
Milestone
4.0
Development

Successfully merging a pull request may close this issue.

Fixed 6DOF set/get check for the path starting with joint_constraints
6 participants
@Calinou @pouleyKetchoupp @akien-mga @madmiraal @Demindiro @Favkis