[declined] missing getDevice()

[REDDemon]

I realized now that OSOperator, GUiEnvironment, SceneManager, VideoDriver every one of those subsystems, don't have "getDevice" method. I think that will be very usefull.

[CuteAlien]

Hm, not sure why - they are only attached to one device anyway and if you need that pointer somewhere you can just pass it to whatever function/class needs it. Enforcing that in the interface doesn't seem necessary unless we allow passing around those classes between different devices (which is not possible so far).

[REDDemon]

I see no point in that. IDevice is like a parent for IVideoDriver, IGUIEnvironment etc. Like any other hierarchy you usually want access to parent from children and to children from parent. You usually Iterates over a hierarchy to gets wanted elements. Actually the interface FORCE you to pass IVideoDriver everywhere even if needed rarely. In my case passing IVideoDriver required lot of extra work (call it design flaw, certain things just can't be predicted, and store pointers to everything just because you don't know what you'll need later I think is really bad practice), so I added that getDevice() and saved lot of time in refactoring. That changed the API but didn't break old stuff. Is just one extra method. And worked very well. Probably for next api review you should seriously consider adding that method.

Enforcing that in the interface doesn't seem necessary unless we allow passing around those classes between different devices (which is not possible so far).

eh? I'm not passing different devices. I'm not doing "setDevice", there is only 1 device so if you call "getDevice", only 1 device will be returned and there is no risk to deal with multiple devices(even better, the correct device is returned). IDevice is the "parent" of everything, 1000+1 reasons why you need access to it. If in my scene I want RootSceneNode I can just iterates scene hierarchy up to the root, I'm not always forced to ask the Root to SceneManager (and so I'm not forced to store pointer to SceneManager), why is not possibile iterating irrlicht classes?. I see many many reasons for accessing stuff indirectly instead of passing everything as parameter, just my opinion, then do what you want

Or maybe there are reasons for not allowing users access IDevice. Probably yes, wich ones?

[hybrid]

Maybe you should just check the API twice. Even if we don't call the method getRoot, you can get access to the scene manager (which is in turn also the root node) from each and every scene node. You can't get the root node from a texture, though, simply because that one does not belong to the scene graph directly. Same holds for gui elements, which have access to the gui environment, but not to the scene graph. Just as much as required for each scenario.

[REDDemon]

Well if you make me the example of SceneManager and GUIenvironment I can agree, but those are not the only 2 classes in irrlicht (I just sad you had 2 ways of accessing Root scene node: ask it to scene manager, or just iterates up to the root. Device is still not accessible)

What I sad, there's a lot of stuff that can be done indirectly by calling stuff getted from some other stuff, but there is stuff that is accessible only from the Device, and the Device is not accessible unless you constantly pass it as parameter:

Code: Select all

 
getCursorControl() ;
getLogger();
getVideoModeList();
getTimer();
getVersion();
getEventReceiver();
postEventFromUser(const SEvent& event);
setInputReceivingSceneManager(scene::ISceneManager* sceneManager);
 

Just take as example the Logger and the Timer, you need to pass continuosly IDevice as pointer if you want to bebug/profile/time something just because IDevice is not accessible from its "children". (in my case I just needed to post some events and to gain access to cursor control, but that pointed me to investigate deeper that aspect of the API). If you find you later to need Logger and timer when you have many classes, you can't just change all the classes for passing that extra pointer. No one will do that.

That's a very long list and i think is a big flaw because heavily force users to do things in a certain (and really verbose) way.

Jsut take as example the CameraFPS scene node:

Code: Select all

 
/! Constructor
        CSceneNodeAnimatorCameraFPS(gui::ICursorControl* cursorControl,
            f32 rotateSpeed = 100.0f, f32 moveSpeed = .5f, f32 jumpSpeed=0.f,
            SKeyMap* keyMapArray=0, u32 keyMapSize=0, bool noVerticalMovement=false,
            bool invertY=false);
 
 

It need cursor control (probably very similiar to my case). Irrlicht have cursor control in SceneManager because Device provide a cursor control to SceneManager, that's like cheating because CursorControl nor Device are declared in public API of SceneManager, in the same way when loading a texture fails the logger prints messages, but nor Texture or VideoDriver have access to logger or Device(at least according to what API says). And also saying "that's ok because CursorsControl is a private variable of a concrete implementation" is wrong because if user have access to Device than he/she can still obtain CursorControl and change it, while it is supposed to be a private variable of CSceneManager or CVideoDriver.

So at this point or you grant access to Device, or you grant access to everything from everything (so Give freedom to SceneManager and gui envinronment to return the Timer, the logget, to post event from users etc.)

I think that letting possible accessing Device is much more simple.

[CuteAlien]

I see no point in that. IDevice is like a parent for IVideoDriver, IGUIEnvironment etc. Like any other hierarchy you usually want access to parent from children and to children from parent.

That's maybe an argument, but aside from making the whole interfaces fatter - and thanks to an extra state-pointer even the object would get fatter - it is just not such a good architectural decision.

eh? I'm not passing different devices. I'm not doing "setDevice", there is only 1 device so if you call "getDevice", only 1 device will be returned and there is no risk to deal with multiple devices(even better, the correct device is returned).

Yes - and that's exactly why the objects/classes do not need that pointer. If there could be different ones then they would indeed need to have a device variable to remember which device they would have to access. But just adding the variable to save passing around a pointer or avoid some typing does not seem like such a good idea when it means you suddenly re-implement the same state-variable all over the objects.

Inside Irrlicht we have no problem with that so far. And if you need it in _your_ implementation classes you are always free to pass every pointer you ever may want - for example in your constructors. So does Irrlicht in the implementation classes where it needs that. That way it is easy to see which of your classes need access to which other classes for anyone reading your code. If we just pass along device everywhere then suddenly every object can access everything in Irrlicht (!) - and one of the reasons for writing object-oriented code at all is suddenly lost. We could as well just make everything global then.

Yes, passing around pointers is a little extra work on writing - but it leads in the end to code that is way easier to read and understand because in every class you can immediately see which other classes are needed. Otherwise the textures might for example suddenly access gui-elements and you couldn't find out about that anymore except reading through the whole implementation of a class!

So no - this is a bad idea - it only saves you some typing which is in the long run that task in programming which costs the least time. The idea is to make applications easy to read and understand - and for that it is optimal to restrict access to every other class as much as possible.

[REDDemon]

Yes you are very clear and now I understand. Thanks for sharing. _-_