Irrlicht 1.7 BETA Phase

[serengeor]

oh this is great news ! Someone had time to fix the timer problem , and also other things that are hard to list

[tomkeus]

Will there be anything on hardware instancing in 1.7?

[hybrid]

No.

[Timo]

http://sourceforge.net/tracker/?func=de ... tid=540676

I see that there was a change made so that IGUIElement constructor doesn't call the virtual addChild() method anymore. This breaks components like CGUIPanel (used in GUIEditor) which requires that children are always added through addChild().

Sure, the implementation of CGUIPanel might be kind of questionable. But it's still kind of weird that addChild() is virtual if it's ignored like that.

[CuteAlien]

Thanks for mentioning that, I always forget to check the editor stuff. But in short - if that element really needs the virtual addChild call then it already didn't work before. The reason for the change was that virtual functions are _not_ called in constructors. That's one of those C++ traps. This change just makes it more obvious what is really happening. If it absolutely is needed then you have no other choice but adding it after the constructor - which is a better idea anyway imho.

Anyway - I try if I can find a few minutes time to look at the editor if it needs any changes.

[Bate]

I would very much like to see your multiple timer class integrated in 1.7 since (from what I've experienced) several timers are needed pretty often. So why not make it an IrrLicht feature?

[CuteAlien]

Sorry, feature-freeze for 1.7 was already some weeks ago, we're only fixing some more bugs right now :-) Also there is the problem that my time-class does actually work a little different (not counting amount of start/stop calls, but stop is stop and start is start) and in general doesn't fit the Irrlicht style too well. Maybe we could make it possible to create more instances of ITimer - that might be useful, so you could put that on the feature-wish tracker.

[Timo]

But in short - if that element really needs the virtual addChild call then it already didn't work before. The reason for the change was that virtual functions are _not_ called in constructors.

I agree. I was just thinking it might be better to change addChild into a non-virtual function. They way it is now is somewhat useless and misleading.

[CuteAlien]

I think the correct solution is rather to remove the parent parameter from the constructor (and preferably all other parameters also). Switching virtual and non-virtual in a base-class is a rather unpleasant interface change and also the virtual is actually useful - the problem here is constructor usage. But that's the kind of interface changes which probably should rather happen in a major version (Irrlicht 2.0) or at least not a few days before a release (I found that problem just recently).

Maybe we should considering adding another constructor and documenting that this one shouldn't be used anymore in 1.8 or 1.7.1. For now I just did what I could do without overhauling the interface. Which is using another non-virtual function in the constructor to make the problem more obvious and changing those places which I did see where it was already needed by using extra addChild call. Just forgot to check the editor. Similar bug is btw. still open for ISceneNode, but probably also has to wait for 1.7.1.

[CuteAlien]

Ok, I just took a quick look at the GUIEditor. And it seems to call IGUIElement::addChild explicit - so seems not to want the overloaded functionality there. Beside that the GUIEditor does simply not seem to work. But I'm not really familiar with that tool so won't be of much help (also no time to get familiar with it currently). Also I must admit I was somewhat horrified after looking at the sources and seeing endless switch-case constructs.

[Nox]

wait: the call is parent->addChild(this); so the addChild methode of parent is called and parent is already constructed, so the vtable is correct. I just tested it right now and it works.
I guess you meant that a virtual methode of a class cant be called correctly while constructing the classe!

[CuteAlien]

Yes, in this case it looks at first correct. Until you start getting a really, really strange bug which takes hours debugging until you finally realize that addChild calls a virtual function of the child you are just adding (which turns out to be this->updateAbsolutePosition) and some gui-element (lets call it CGUIModalScreen a strange element which really even shouldn't be a gui-element in the first place, but really - could be any element here...) did overload that one. Calling virtual functions in the constructor is even dangerous when called for the parent - something I also leared the day I debugged that ^^

[Nox]

Well you got a point. If a call implies a virtual call on the unfinsihed object it can be very harmful. But removing the autoreg can be harmful too. What about removing the virtual part in 2.0, adding a "dirty" flag and fixing the updateorder? This way the problem can be solved without to harsh API changes and the number of update calls can be reduced to a fixed number per object and per frame.

[CuteAlien]

Removing virtual in a base-class while keeping the function-name does actually sound like a pretty bad API change to me. You wouldn't even get a compile warning about stuff no longer working. But the solution for now is just for 1.7 and I wish I would have time to quick-fix ISceneNode also, but I won't.

[Timo]

Removing virtual in a base-class while keeping the function-name does actually sound like a pretty bad API change to me. You wouldn't even get a compile warning about stuff no longer working.

That's already the case. You gotta assume that all the classes that override addChild() or updatAbsolutePosition() stopped working. And not only without compiler error, but also without any indication of it in the interface of IGUIElement. It doesn't behave like it leads you to believe.

To be honest, i think both solutions break a lot more stuff than they fix. I would just keep 1.7 the same way as 1.6, and then in next version do like you said and remove the parent parameter from the constructor. But hey, I'm not in charge here...