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

Rename WeldJoint to AngleJoint #25

Closed
NauticalMile64 opened this issue Jul 30, 2017 · 7 comments
Closed

Rename WeldJoint to AngleJoint #25

NauticalMile64 opened this issue Jul 30, 2017 · 7 comments
Labels
Enhancement For suggestions or changes that enhance any part of the project and isn't a bug.

Comments

@NauticalMile64
Copy link
Contributor

I've thought over the years that not all joints were created equal, and some joints that people never used were unstable or something like that. After playing around with the testbed, love2d, etc... I realized that there were really neat behaviors created by the WeldJoint among others which I will open other issues for. It struck me, that the WeldJoint is functionally an angular analogue of the DistanceJoint. I discuss this in depth with pictures and gifs in this GDSE answer, so I won't repeat it here.

I think it's pretty unlikely for box2d to adopt this change. It would also be a poor choice as it now has so many flavors, bindings, and re-implementations across the net that would need to update. For PlayRho, however, a change like this would be very possible in its current state, and would communicate the purpose of the joint more clearly.

What does everyone else think about renaming the WeldJoint to AngleJoint? Is there another name that would fit better?

@louis-langholtz louis-langholtz added the Enhancement For suggestions or changes that enhance any part of the project and isn't a bug. label Jul 30, 2017
@louis-langholtz
Copy link
Owner

Interesting suggestion! I'm looking at the code for the two classes now in this light.

I love the images you provided by the way for the GDSE answer! Based on the images, I think your suggestion is a helpful one. I think the name WeldJoint confused me actually some years ago when I first had started using Box2D. Calling it an AngleJoint instead seems like it would be more clear.

OTOH, Joint instances are constraints. Based on that concept - of being a constraint - would it be more natural to call a DistanceJoint actually an AngleJoint (since that joint actually constrains the angle) and the WeldJoint actually the DistanceJoint (which actually constrains the distance)?

Admittedly though I'm getting sleepy while I'm writing this; I'd better revisit this in the morning.

@Alan-FGR
Copy link

Actually, I personally find WeldJoint not too bad, however, Unity call it "FixedJoint" and I find that name really appropriate, overall the names for joints in Unity are quite OK imho and maybe we could take a look at them for reference. "AngleJoint" is confusing for both joints you guys mentioned imho.

@NauticalMile64
Copy link
Contributor Author

@Alan-FGR Well you're right AngleJoint is also not immediately obvious, but to me the names WeldJoint and FixedJoint suggest that the two bodies are locked in a particular configuration, but in my GDSE answer I show that this is not how the joint behaves! It be haves as an analogue of the distance joint, and IMO this is the part of the behavior that should be emphasized. We can mention that the WeldJoint should be used to fix 2 bodies in some situations, but that should be an afterthought, not the emphasis.

That being said, you are right; Unity did rename a bunch of joints with better meaning behind the names, and I'm seeing now that we can't just discuss renaming one joint at a time, we need to settle on a convention that makes sense for all joint names. At some point today I'll create a big table which shows the behaviors of each joint, the box2d names and descriptions, the unity names and descriptions, and my proposed names and descriptions.

@louis-langholtz I think I will make a new issue for this, since the scope of the issue has been increased. Make sense?

@Alan-FGR
Copy link

Alan-FGR commented Jul 30, 2017

@NauticalMile64 while it's true that the WeldJoint sometimes may not behave as the name suggest, its purpose is to actually 'glue'/'lock' two bodies together. However, due to the nature of the solver, it's not possible to achieve a perfect sync, but given enough solver iters a good approximation can be achieved (what's generally not the case). Attention needs to be paid to mass ratios too.

In my opinion the fact that the WeldJoint accidentally behaves differently than one would intuitively expect shouldn't overcome the fact that its intention is to actually 'weld' two bodies together. It's much better in my opinion to just instruct the user on how the solver works in general, and how iters and large mass ratios will make things do funny stuff, instead of renaming joints based on some particular observations.

Also, I'm OK with renaming stuff, however, "AngleJoint" really doesn't sound good to me, it sounds like something that will allow bodies to translate freely while trying to lock their angles (like the gear).

Afaik the joints use impulses just like contacts to try to 'sync' bodies properties, that's why the fixed joint needs a 'point' somewhere to apply the impulses, and naturally changes in angular momentum will occasionally cause it to look springy. That is a limitation in the solver itself, but if you increase the iterations the weld joint will approximate the expected behavior better than that.

EDIT: When you create the table showing the behaviors, it would be nice if you could also make variants with different iters and mass ratios

@louis-langholtz
Copy link
Owner

Given the conversation so far, I'm going to hold off at least for now on renaming WeldJoint.

On a perhaps related note, PlayRho so far has less unit test code coverage of the Joint code than most other code. That's partly due to the cyclomatic complexity of this code. I'd like to break it apart into simper methods using more free and pure functions. Looking at the code, I suspect there's duplication of the sources across different joint types that could be refactored out into the free and pure functions I like better. How this is related, is that the more similarity between WeldJoint and DistanceJoint the more possible I believe it should be to distill these classes differences and similarities. Like are some Joint type effectively achievable by combining several simpler joint types perhaps? And any renaming might best be done when that's clearer.

@NauticalMile64
Copy link
Contributor Author

Alright, I've moved the discussion to Issue #27.

@Alan-FGR I haven't shown the behaviour with different iters yet, but I'll get there. Also, can you clarify what you mean by mass ratios -> to you mean the ratio of the masses connected by the joint?

@Alan-FGR
Copy link

@NauticalMile64 yep, I've seen a number of people on physics engines forums complaining about say joints not holding wheels in place when they try to use a simple joint to connect 4x 1kg wheels to a 1500kg chassis. That will never work unless with some special solver like featherweight or PhysX articulations, at least when it comes to real-time physics engines they all work by simply overlapping shapes and changing velocities, thus they all have that kind of limitation, that's why I don't think we should resort to observed behavior for naming, we should name our joints according to what they're supposed to do, if they don't work perfectly well in practice, many people are already used to that and know how to tune the settings to their needs, while the other people could just be easily instructed about it. Take a look at the Box2D forums for example, there are tons of people who had problems with joints.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement For suggestions or changes that enhance any part of the project and isn't a bug.
Projects
None yet
Development

No branches or pull requests

3 participants