Skip to content
This repository has been archived by the owner on Nov 29, 2022. It is now read-only.

Body2 #73

Closed
wants to merge 3 commits into from
Closed

Body2 #73

wants to merge 3 commits into from

Conversation

faassen
Copy link
Contributor

@faassen faassen commented Mar 20, 2021

This is a draft implementation of #72. Not all convenience shapes are implemented, but I wanted to do a proof of concept.

I've created a BodyColliderBuilder trait that is now implemented by both the Body component as well as by Body2. This allows us to create bodies for Body2 too.

While I'm not displeased by the structure of the code, the debug renderer would need to be adjusted as well.

Since the debug renderer is supposed to work for 2d, we could assume users create Body2 instances and dedicate the debug renderer to this. Alternatively we'd need to support both Body and Body2 in the debug renderer, which would necessitate another trait.

But perhaps we could shift the traits around. We introduce a trait, BodyFactory with a create method that returns Body. Body would implement this by returning self, but Body2 would return Body instead. This way any user code would deal with Body only.

I don't know what's preferable - should user code that's in 2d only deal with Body or would it actually be nicer if they only have to deal with Body2? Is user code expected to deal with Body a lot anyway, outside of the debug renderer?

@faassen faassen marked this pull request as draft March 20, 2021 23:19
@faassen
Copy link
Contributor Author

faassen commented Mar 21, 2021

Interesting, it looks like I finally need to enable clippy. It was quite tricky to come up with a decent name for the BodyColliderBuilder as there's already a ColliderBuilder but it doesn't like it being in a module called "body". :)

@faassen
Copy link
Contributor Author

faassen commented Mar 21, 2021

Adding them on Body directly behind a feature toggle as mentioned on #72 does make the issue I discuss above go away. Though does it? You'd still need to write code that handles the new Body entities directly, but it's still the case that in 2d you'd not worry about Spheres and Cuboids, and that in 3d you may not have to worry about circles (if you never generate them).

@faassen
Copy link
Contributor Author

faassen commented Mar 22, 2021

Concerning this branch: while Body2 is going away would it be worthwhile to preserve some of the behavior? It does read nice in the tests to have a build method on body. But I think I might be able to implement this with an impl block instead of a trait (if I can use impl for a struct defined in another module).

@jcornaz
Copy link
Owner

jcornaz commented Mar 22, 2021

if I can use impl for a struct defined in another module

In another module yes. In another crate no.

@jcornaz
Copy link
Owner

jcornaz commented Mar 22, 2021

would it be worthwhile to preserve some of the behavior?

I think that, ideally, we would not have to touch anything in heron_rapier nor heron_debug for this.

It does read nice in the tests to have a build method on body

Internally yes. But we don't expect user to use it. In fact, if we were to keep that trait, I'd make it private.

@faassen
Copy link
Contributor Author

faassen commented Mar 22, 2021

Yes, it'd be a private trait, purely to help with the structure of the code. I'll give it a shot.

@jcornaz
Copy link
Owner

jcornaz commented May 23, 2021

Thanks @faassen for your contribution.

I will provide a 2d oriented API for creating collision shapes. But not in the form of a new component type. It'll be in the form of new constructors on the existing collision shape enum instead. (see: #72 (comment))

@jcornaz jcornaz closed this May 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants