CuteAlien wrote: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.
No, that is wrong - I made nothing stop working and did not change any interfaces. You can still call addChild() and updateAbsolutePosition() as you could before and the overloaded functions will still be called as they are still virtual. The change I have made is that those functions are now no longer called in the constructor - where they _never had_ worked - so nothing gets broken that did work before. The only difference now is that the functions which are now called in the constructor are non-virtual so if you stumble upon that problem it is a lot easier to debug and see what goes wrong than before. And I fixed the places where the parent in the constructor was used for elements which did actually overload addChild (which caused actual bugs) by not using that parameter anymore in those constructors and calling addChild explicit. Which is basically the thing that should probably be recommended throughout - simply don't ever use the parent parameter. I can't fix it in the user-code right now - and I can't fix it correctly without breaking the interface (well, using a dirty-flag might work, but it has other disadvantages).
I could probably add another line to the documentation strongly recommending not to use the constructor parameters unless you are really sure you know what you do. Maybe I do that.
CuteAlien wrote:The change I have made is that those functions are now no longer called in the constructor - where they _never had_ worked - so nothing gets broken that did work before.
You can't say they never worked. Even though virtual calls in constructor aren't recommended, their behavior is still well defined, and a derived class may rely on that behavior. Overriding addChild() was working perfectly if you just knew what you were doing.
Right now there's no way of getting it to work without more fixes in irrlicht code. Calling addChild() explicitly only helps if you are creating elements manually. If you load a GUI from XML, irrlicht never calls addChild().
Well yes - it is well defined. When the virtual function is called in the base-class (as is here the case) then it won't call the virtual functions of derived classes within the constructor even in the creation of the derived classes. So the non-virtual function with which I replace it does contain that old base-class behavior and so you get the same result. The main difference is that it no longer looks as it would be doing something else which it never did. You never got the overloaded virtual function called in there.
There is also the situation where someone might have called addChild _once more_ in a derived constructor. But as addChild is still virtual that works also the same as it always did (you get in that case the addChild overload of the class constructing it). Although that is not clean and the user would be better off using a new function which can be called in the constructor instead and then adding that new function also to his addChild overload. But if he doesn't - nothing gets changed.
So I must admit I don't see any place where I did break any old code or interface. Unless I'm missing something you can still do everything which you could do before. Only the stuff that never worked is now made more obvious and a few places where it caused bugs have been fixed.
This is a bit off topic to the discussions above but how do we remove a IRenderTarget? Do we just remove the texture in the IRenderTarget like a normal texture?