[mythtv-commits] Ticket #2533: setting-revamp : review + test/fix + merge

[MythTV]

#2533: setting-revamp : review + test/fix + merge
---------------------+------------------------------------------------------
 Reporter:  danielk  |        Owner:  danielk
     Type:  task     |       Status:  new    
 Priority:  trivial  |    Milestone:  unknown
Component:  mythtv   |      Version:  head   
 Severity:  low      |   Resolution:         
---------------------+------------------------------------------------------
Comment (by danielk):

 (In [11722]) Refs #2533. Settings inheritence cleanup.

 I've talked about this a bit over the last year or so, but I finally had
 some time to fix it early last week, I found a couple problems and fixed
 them but it has been fairly trouble free.

 The settings code has been violating Qt rules for some time. The two rules
 it has been violating are:
   no multiple inheritence of QObject
   no virtual inheritence of QObject

 The first breaks the Qt signal/slot mechanism because qmake assigns a
 unique integer to each slot iff there is no multiple or virtual
 inheritence of QObjects. We've avoided the problems by always placing
 parents without slots last in the inheritence order and by making all but
 one of QObject derived parents virtual. When someone accidentally inherits
 from two QObject derived classes with slots, or tries to use Qt timers the
 wrong slot will be used on some systems, but not necessarily on the
 developer's machine, which not only makes MythTV unstable but is very hard
 to debug.

 The virtual inheritence has it's own problems, and is why we needed to
 initialize all the parents separately in child classes. For example:
 {{{
     ConfigurationGroup(true, true, false, false),
     VerticalConfigurationGroup(true, true, false, false),
     TriggeredConfigurationGroup(true, true, false, false)
 }}}
 is a construct you see a lot of in MythTV settings code. This has also led
 to some odd constructs for Wizards and TriggeredConfigurationGroup
 instances. In addition Qt does not generate slots correctly for virtually
 inherited parents, so any slots in virtually inherited parents can
 missfire.

 To avoid these problems I made the Storage classes not inherit from
 QObject. Because some of the storage classes need to call Setting methods
 the Storage classes (except for TransientStorage) take a pointer to their
 associated Setting in leu of inheritence. Since the Configurable classes
 need to be able load and save themselves they too get a pointer to their
 associated Storage class. In fact, I haven't changed the immediate
 inheritence at all for most classes; so a class such as HostComboBox
 inherits from ComboBoxSetting and HostDBStorage (this used to be
 ComboBoxSetting and HostDBSetting) and passes this as the Storage and
 Setting class resp.

 There were a few exceptions, some classes inherited from three QObject
 derived classes. Usually these were
 HorizontalConfigurationGroup/VerticalConfigurationGroup and
 TriggeredConfigurationGroup OR ListBoxSetting and ConfigurationDialog. For
 the first case I modified TriggeredConfigurationGroup to treat addChild()
 instances as if the were inserted into a horizontal or vertical
 configuration group and treat the addTrigger() instances as if they were
 inserted into the triggered configuration group, so that the API didn't
 change except that you only inherit from TriggeredConfigurationGroup and
 call SetVertical(false) if you want a horizontal layout instead of the
 default vertical one. The ListBoxSetting and ConfigurationDialog classes
 were converted to  ConfigurationDialog's with a ListBoxSetting added.

 When the settings-revamp is synced to this code it should automagically
 become much more stable, which is why its tracking ticket is referenced...

 It might be possible to get rid of QObject use in the Setting derived
 classes as well, but this would involve many more changes, the storage
 classes did not use any QObject features, but only inheritted from Storage
 in order to access it's members so this change is fairly trivial, it just
 touches on a lot of code.

-- 
Ticket URL: <http://cvs.mythtv.org/trac/ticket/2533#comment:13>
MythTV <http://www.mythtv.org/>
MythTV