IRRlicht 1.4 - setGravity, value multiplied

[christianclavet]

Hi.

Just implemented a swim function in my engine with this to change the level of gravity depending how you where positionned in the water.

If I set the vector a (0,-1,0), when I create the collisionResponseAnimator, all is ok. Gravity is what I expect.

If I use the same vector (0,-1,0) using this function (setGravity()). I get a gravity effect of about -100 when I expect to have 1.

It's easy to workaround this (set a very low number (/100) fix the problem). But this should be fixed.

[rogerborg]

Try /1000!

Spot the difference:

Code: Select all

CSceneNodeAnimatorCollisionResponse::CSceneNodeAnimatorCollisionResponse(
		ISceneManager* scenemanager,
		ITriangleSelector* world, ISceneNode* object,
		const core::vector3df& ellipsoidRadius,
		const core::vector3df& gravityPerSecond,
		const core::vector3df& ellipsoidTranslation,
		f32 slidingSpeed)
: Radius(ellipsoidRadius), Gravity(gravityPerSecond / 1000.0f), ...

vs

Code: Select all

void CSceneNodeAnimatorCollisionResponse::setGravity(const core::vector3df& gravity)
{
	Gravity = gravity;
}

They should certainly be made consistent: the question is which one to change based on how many users it will screw over. Pragmatically, it might be less invasive to have setGravity() do a /1000.f, even though in principle I favour taking the /1000.f out of the constructor.

[Ico]

I'd stick with "units per second" - so divide both inputs by 1000. Imo it's more convenient to define it that way without going on a "per millisecond" basis.

[Frosty Topaz]

I agree with Ico. It's simpler and I suspect more people have constant gravity and as such don't use setGravity() at all.

[Dorth]

Well, I agree with Rogerborg on both accounts. First, that both should be identical. Second, it should not do an arbitrary modification on the value you give it. If I want to divide a number by 1000, I can do this myself, thank you. And that doesn't help finding things in debug later when a number no longer is what you expected it to be.

[hybrid]

Well, we could return the internal value multiplied with 1000 to keep the same values... It's just that the constructor parameter is meant to be offset per second and the internal value is per millisecond. I think I'll change the get/set methods.