Ambient light does not work (includes fix!)

[Jedive]

I am using the SVN version of IrrLicht and noticed that IVideoDriver::setAmbientLight() does not work. It is extremely easy to fix it:

1) Open the file "ISceneManager.h", and add these two methods to the ISceneManager class:

Code: Select all

virtual void setAmbientLight(video::SColorf ambientColor) = 0;
virtual video::SColorf getAmbientLight() = 0;

2) Recompile the engine, and now use ISceneManager::setAmbientLight() instead of IVideoDriver::setAmbientLight().

Would be cool to have this added to SVN.

[vitek]

I personally feel that it is a bug for the scene manager to be setting the ambient light, especially since it breaks compatibility with both old and expected behavior.

I can understand why Niko would want it there. If you have multiple scene managers, you can more easily maintain different lighting setups per scene and it allows him to persist the ambient light so that no code change is required to make a scene look 'right'. The problem is that moving the ambient light information into the scene manager is just the start. It is likely that the fog color/type settings will need to be moved there also.

Maybe the IVideoDriver::setAmbientLight() and setFog() methods should both be moved out of the video driver interface and put somewhere that the scene manager implementation can call them and the end user cannot.

Either that, or the scene manager should just store these settings and provide a method for applying them to a video driver before rendering.

Travis

[rogerborg]

I personally feel that it is a bug for the scene manager to be setting the ambient light, especially since it breaks compatibility with both old and expected behavior.

Damn skippy it does. I just had a Senior Moment trying to figure out why device->setAmbient() wasn't doing anything. Durr, because the scene manager keeps overriding it with its own ambient setting.

If setAmbient() is staying in ISceneManager, then the driver version should be hidden from users to avoid this confusion.

[Saturn]

The confusion is only, because we are so used to directly access the driver functionality, because it is needed in some cases. But to me "expected behavior" means, I only use the scene management / material interface and the scene manager itself does the necessary steps using the IVideoDriver interface.

This is a problem conceptionally, because we have two different approaches. A declarative API with ISceneManager and Material/SceneNodes/etc and the imperative API of the driver. Having to use both APIs leads to these problems.

Imho, the best were to hide the driver interface as a whole (or at least make clear, that it is only to be used by people who know what they're doing) and expose the functionality through the SceneManager or specialised classes in a declarative way. This is not much really. Only problem are the 2d drawing functions, which don't have a declarative equivalent yet. Personally I love Ogre's ManualObject, it shows a way for how this can be done.

[rogerborg]

With respect, it is a problem, because it just caught me out. Quod erat demonstrandum.

A comment in the driver interface saying that setAmbient() should only be called by the scene manager would help. But if it's necessary to comment it, then it's better to enforce it, making the comment unnecessary.