Skip to content

Rename scale to anim_scale #219

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lukehorvat
Copy link
Contributor

Another small rename I hope you'll consider.

From what I can ascertain, actors may have several different scales in their def files: actor_scale, mesh_scale, bone_scale, and scale. My PR concerns this last one, scale. The current naming is a bit ambiguous and the purpose of it is not clear. At first I thought it might be associated with actor->scale, but no, that's dynamic scaling that is applied via ADD_NEW_ENHANCED_ACTOR.

After digging through the code a bit I can see this scale is only used in 3 places: for scaling the Cal3D animation keyframe translations. So it's an animation scale. For this reason, I'd like to suggest renaming it to anim_scale. It'd be nice to rename the scale elements in the xml files too.

Fun fact:
For animations it's typically best to use the skeleton scale (bone_scale) or you risk the mesh becoming deformed when animating, so I questioned why EL has this separate animation scale in the first place. Looking at the actor def files, I found that big spiders are the only actors with an animation scale that is different from the skeleton scale. Their skeleton is much smaller than the mesh, which results in the mesh becoming a bit deformed when animating. Here's their walking animation slowed down:

spider_scale1.mp4

Increasing the skeleton size to match the mesh results in a much nicer animation and mesh:

spider_scale3.2.mp4

I was going to make a PR removing the redundant animation scale entirely in favor of skeleton scale, but then when I set the animation speed back to normal I saw why the deviation was needed for big spiders – the walking animation simply doesn't look good when the skeleton is properly sized:

spider_walk.mp4

So unfortunately because of spiders the separate animation scale is still needed.

@@ -489,7 +489,7 @@ static void draw_actor_banner(actor *actor_id, const actor *me, float offset_z)
glGetDoublev(GL_PROJECTION_MATRIX, proj);
glGetIntegerv(GL_VIEWPORT, view);
// Input adjusted healthbar_y value to scale hy according to actor scale
gluProject(healthbar_x, healthbar_y, healthbar_z * actor_id->scale * actors_defs[actor_id->actor_type].actor_scale + 0.02, model, proj, view, &hx, &hy, &hz);
gluProject(healthbar_x, healthbar_y, healthbar_z * get_actor_scale(actor_id) + 0.02, model, proj, view, &hx, &hy, &hz);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also found a couple of places where get_actor_scale can be called. Included that in the PR too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant