From #0ad-dev today:

(11:40:34 PM) minohaka: JSInterface_IGUITextOwner.cpp 5>z:\0ad\source\gui\scripting\jsinterface_iguitextowner.cpp(27): error C2065: 'GetTextSize': undeclared identifier 5>Done building project "gui.vcxproj" -- FAILED.
(11:41:59 PM) elexis: GetTextSize -> JSI_IGUITextOwner::GetTextSize

clang -Winconsistent-missing-override:

In file included from ../../../source/ps/GameSetup/GameSetup.cpp:38:
In file included from ../../../source/gui/GUI.h:51:
../../../source/gui/IGUITextOwner.h:67:15: warning: 
      'HandleMessage' overrides a member function but is not marked
      'override' [-Winconsistent-missing-override]
        virtual void HandleMessage(SGUIMessage& Message);
                     ^
../../../source/gui/IGUIObject.h:291:15: note: overridden virtual
      function is here
        virtual void HandleMessage(SGUIMessage& UNUSED(Message)) {}
                     ^
In file included from ../../../source/ps/GameSetup/GameSetup.cpp:38:
In file included from ../../../source/gui/GUI.h:51:
../../../source/gui/IGUITextOwner.h:72:15: warning: 
      'UpdateCachedSize' overrides a member function but is not
      marked 'override' [-Winconsistent-missing-override]
        virtual void UpdateCachedSize();
                     ^
../../../source/gui/IGUIObject.h:207:15: note: overridden virtual
      function is here
        virtual void UpdateCachedSize();
                     ^
In file included from ../../../source/ps/GameSetup/GameSetup.cpp:38:
In file included from ../../../source/gui/GUI.h:51:
../../../source/gui/IGUITextOwner.h:89:15: warning: 
      'MouseOverIcon' overrides a member function but is not marked
      'override' [-Winconsistent-missing-override]
        virtual bool MouseOverIcon();
                     ^
../../../source/gui/IGUIObject.h:129:15: note: overridden virtual
      function is here
        virtual bool MouseOverIcon();
                     ^

This is because this commit uses the override keyword to indicate that GetTextOwner() is in fact overriding that function (.and not implementing a similar one),
but the other functions in that class that also override do not use the override keyword (since the original introduction in rP290).

So basically it asks us to make up our mind whether we want to use the keyword everywhere where it applies or nowhere.

The override keyword has the advantage that it makes it a compiler warning if one intends to override a function that doesn't exist.

Adding the virtual keyword everywhere means changing about 20 files.

After having done so it actually reveals that it was forgotton in CChart to call IGUITextOwner::HandleMessage.

Also some derived classes call their base IGUIObject::HandleMessage prior to their switch statement, others after, with an // Important! comment on it, other classes don't call it altogether.
The reality is that function is empty, and if that is to remain the case, then IGUIObject::HandleMessage can become unimplemented. This way derived classes don't have to philosophize when it's best to call an empty function.

Making HandleMessage pure virtual would have been nice. But no.

AddSetting is called from its and derived constructors and that calls HandleMessage.
Since the base class constructor is called prior to the derived constructor, the overriding HandleMessage hasn't been overriding yet.
Hence the pure virtual would be called at runtime.

This assumption (that overridden will always be called (including ctor)) can be addressed by performing the AddSetting calls in a separate AddSettings function.
Sounds nice too, in particular in points of encapsulation. But no.

It means that the gained freedom from having to call an empty HandleMessage function would then mean the obgliation to not forget calling the base AddSettings function from the derived one.

...

After further implementing this approach, it even seems cleaner at first sight if the inherited function is called explicitly, and if the IGUIObject, IGUIScrollBarOwner, IGUIButtonBehavior, IGUITextOwner functions can set the empty functions as pure virtual.

However it can also cause some issues, since classes test for settings (such as scrollbar) that are defined in other classes, so the order matters.

During programming I even came across this issue, which sounded like GUI UB:

ERROR: JavaScript error: simulation/components/UnitAI.js line 6035
Error: Registered component has unrecognized 'OnMotionUpdate' message handler method
  @simulation/components/UnitAI.js:6035:1
  startHost@gui/gamesetup_mp/gamesetup_mp.js:315:3
  confirmSetup@gui/gamesetup_mp/gamesetup_mp.js:128:7
  __eventhandler53 (press)@continueButton press:0:1

or perhaps the UB has found a unitmotion bug? Not sure why it would complain about that specifically, or even how it gets to load these files in the UI...

Either way, after implementing everything, it doesn't look worse to me to have the IGUIObject an abstract class, i.e. one with some pure virtual functions instead of functions with empty bodies that need to be called.

But that is independent from the consistent override thing, so it may be split into two cleanups.