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.

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

Readability from Skeleton2D calculate_length_and_rotation() function #7472

Closed
thiagola92 opened this issue Aug 10, 2023 · 1 comment
Closed
Milestone

Comments

@thiagola92
Copy link

Describe the project you are working on

Reading skeleton_2d.cpp to understand the problems with scaling Bone2D and IK (godotengine/godot#80252, godotengine/godot#76087).

Describe the problem or limitation you are having in your project

No problem, the propose will only affect readability of Godot code

Describe the feature / enhancement and how it helps to overcome the problem or limitation

No enhancement, just change the readability from calculate_length_and_rotation() function (https://github.com/godotengine/godot/blob/013e8e3afb982d4b230f0039b6dc248b48794ab9/scene/2d/skeleton_2d.cpp#L431)

void Bone2D::calculate_length_and_rotation() {
        // if there is at least a single child Bone2D node, we can calculate
        // the length and direction. We will always just use the first Bone2D for this.
        bool calculated = false;
        int child_count = get_child_count();
        if (child_count > 0) {
                for (int i = 0; i < child_count; i++) {
                        Bone2D *child = Object::cast_to<Bone2D>(get_child(i));
                        if (child) {
                                Vector2 child_local_pos = to_local(child->get_global_position());
                                length = child_local_pos.length();
                                bone_angle = child_local_pos.normalized().angle();
                                calculated = true;
                                break;
                        }
                }
        }
        if (calculated) {
                return; // Finished!
        } else {
                WARN_PRINT("No Bone2D children of node " + get_name() + ". Cannot calculate bone length or angle reliably.\nUsing transform rotation for bone angle");
                bone_angle = get_transform().get_rotation();
                return;
        }
}

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

Remove local variable calculated and return as soon it finish calculating.

void Bone2D::calculate_length_and_rotation() {
	// If there is at least a single child Bone2D node, we can calculate
	// the length and direction. We will always just use the first Bone2D for this.
	int child_count = get_child_count();
	if (child_count > 0) {
		for (int i = 0; i < child_count; i++) {
			Bone2D *child = Object::cast_to<Bone2D>(get_child(i));
			if (child) {
				Vector2 child_local_pos = to_local(child->get_global_position());
				length = child_local_pos.length();
				bone_angle = child_local_pos.normalized().angle();
				return; // Finished!
			}
		}
	}

	WARN_PRINT("No Bone2D children of node " + get_name() + ". Cannot calculate bone length or angle reliably.\nUsing transform rotation for bone angle");
	bone_angle = get_transform().get_rotation();
}

If this enhancement will not be used often, can it be worked around with a few lines of script?

No

Is there a reason why this should be core and not an add-on in the asset library?

Can't be an add-on

@akien-mga
Copy link
Member

Implemented by godotengine/godot#83397.

@akien-mga akien-mga added this to the 4.3 milestone Jan 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants