PlayRho
PlayRho copied to clipboard
Remove all public functions that take a non-const Body reference.
Expected/Desired Behavior or Experience:
When learning how to use PlayRho, I would expect to not become trapped by finding functions that are unusable. It is confusing and takes time trying to figure out that this is not the expected way to use the library.
Actual Behavior:
You have functions like this hanging around: https://github.com/louis-langholtz/PlayRho/blob/a3a91554cf9b267894d06a996c5799a0479c5b5f/PlayRho/Dynamics/Body.hpp#L1211-L1214
As far as I can tell, there is no way to acquire a non-const Body, so functions like these just pollute the namespace and are beginner traps. I spent a lot of time trying to figure out how to alter bodies and these functions really threw me for a loop.
Hi LoneBoco. Thank you for raising the issue. I agree with what you’re saying. I’d started work on this a while back actually. I haven’t had the time however to finish reorganizing things though.
I just realized that I may have partly overlooked what you wrote in terms of my previous response.
Using a Body with a World
As far as I can tell, there is no way to acquire a non-const Body
Any of the following should work to acquire a non-const Body
:
#include <PlayRho/PlayRho.hpp>
int main()
{
static auto bodyWithStaticStorageDuration = playrho::d2::Body{};
auto bodyWithAutomaticStorageDuration = playrho::d2::Body{};
auto* bodyWithDynamicStorageDuration = new playrho::d2::Body{};
}
Do none of these work for you?
Is the confusion with relating a Body
instance with a World
instance? Things are different in this regard from Box2D
as PlayRho is more value semantic oriented. This was also something that did seem like an issue to me in the pre-2.0 code interfaces which I've changed in the current master
branch code.
For example...
In the release-1.1
branch code, a World
instance takes a BodyConf
to create a body while it takes a Body
to override the current state of a created body. I.e. there's these two functions which are asymmetric in this regard:
-
BodyID World::CreateBody(const BodyConf& def = GetDefaultBodyConf());
. -
void World::SetBody(BodyID id, const Body& value)
.
In the master
branch code however this has been changed to the following:
-
BodyID World::CreateBody(const Body& body);
-
void SetBody(BodyID id, const Body& value);
Note that there's also the following functions available to convert between a Body
and a BodyConf
:
Body::Body(const BodyConf& bd = GetDefaultBodyConf());
BodyConf GetBodyConf(const Body& body);
Unusable Functions
I would expect to not become trapped by finding functions that are unusable.
That's what I had read and was thinking about till now and I agree with you on this. The problem also comes up when creating or using the project as a dynamic library. And there's a tension in this regard against unit testing the code. I don't want users concerning themselves with implementation functions for example but I do want that code tested directly. I have it in mind and have made some starts at dealing with that.
Added a new project of Reduce Namespace Pollution to address the concerns I share with this issue. Closing this particular issue in deference to this new project.