While working with the main menu again (FPS counter, refs D2389/rP23091) I noticed some small improvements that could be applied to the code from rP22854/D2240, in particular before the code is extended.
- MainMenuPage class: As learnt in D2310/rP22985 after having moved everything into classes, there will be one umbrella class remaining owning everything else (hierarchy).
Hence this diff implements that for the main menu page. This has the advantage that if there were global procedures introduced, they'd not be mixed in with class definitions, discouraging their implementation in preference over class architecture.
- The BackgroundHandler class does two acts per background layer instance. That logically qualifies the introduction of a class that does these two things (BackgroundLayer). In particular this has the advantage that the Engine.GetGUIObject call is (always) in the constructor, which seems to be a benefitial pattern and makes the code also slightly faster. Notice that the previous code also defined functions inside the functions that are now just members. This also seems to be a pattern (if there are local functions, its an indication one may consider adding a new class to avoid redefintion of the local). The tiny classes are now owned too, merely for consistency (and hypothetical extensibility/moddability).
- Marginal performance improvement of the BackgroundHandler. Amongst not obtaining the GUIObject reference every frame anymore, the getComputedSize() call only needs to be done when resizing the window. (I tried avoiding new GUISize call too but failed to, since we may not modify the parent size cache)
- constructor(menuItems, menuSpeed = 1.2, margin = 4, buttonHeight = 28) still is a hardcoding and out of place. As the last 7 weeks past that commit showed, the prototype chain is a good place to store such values. Notice the disadvantage of the prototype as a storage facility is that its slower than a member of the instance (two steps to traverse instead of one), but that doesn't justify less modifiable code.
- bind: As Krinkle had recommended in the revision proposal bind is a nice feature, and it was used often in later commits, so this makes it consistent.
- "100%-2 " + this.submenu.size.bottom + " 100% 100%"; is bad hardcoding. As learnt in the past months, size values are best stated in XML and only in XML. This diff only modifies the relevant property of the size and reuses XML stated values otherwise, thus removing almost all hardcoded literals.
- Some global references were replaced by passing the references through the constructor.
- onTick assignment moved from XML to JS. I thought that didn't work, but it does. Leaves XML free of JS.