Opened 13 years ago

Closed 12 years ago

Last modified 12 years ago

#2533 closed task (wontfix)

setting-revamp : review + test/fix + merge

Reported by: danielk Owned by: danielk
Priority: trivial Milestone: unknown
Component: mythtv Version: head
Severity: low Keywords:
Cc: Ticket locked: no

Description

This is a tracking ticket for getting the setting-revamp code up to snuff and merged.

Change History (16)

comment:1 Changed 13 years ago by danielk

Owner: changed from Isaac Richards to danielk
Severity: mediumlow
Version: 0.20head

comment:2 Changed 13 years ago by danielk

Type: defecttask

comment:3 Changed 13 years ago by danielk

(In [11536]) Refs #2533. Cleanup of videosource.{cpp,h}, and its reverb through other classes.

comment:4 Changed 13 years ago by danielk

(In [11537]) Refs #2533. Cleanup of scanwizardhelpers.h.

comment:5 Changed 13 years ago by danielk

(In [11538]) Refs #2533. Cleanup of mythwidgets.{cpp,h}.

comment:6 Changed 13 years ago by danielk

(In [11544]) Refs #2533. Removes accidental change to tv_play.cpp

comment:7 Changed 13 years ago by danielk

(In [11545]) Refs #2533. First pass of cleanup of settings.{cpp,h} in settings-revamp

comment:8 Changed 13 years ago by danielk

(In [11546]) Refs #2533. Cleanup of dbsettings.{cpp,h} in settings-revamp

comment:9 Changed 13 years ago by danielk

(In [11547]) Refs #2533. Cleanup of new xml files in settings-revamp

comment:10 Changed 13 years ago by danielk

(In [11551]) Refs #2533. First pass of cleanup of globalsettings.{cpp,h} in settings-revamp

comment:11 Changed 13 years ago by danielk

Priority: minortrivial

I've done one review+cleanup pass over all the significantly modified files in the settings-revamp branch. Most of the broken parts have TODO's attached.

An exception is the settings.h classes which will need to have multiple QObject inheritence and virtual QObject inheritence removed. This has to be fixed whether this branch is ever merged or not.

I have not verified that there are no missing configuration controls in globalsettings.{cpp,h} by comparing it closely with the current version, but I added back any controls I noticed were missing.

Also, the keybinding code in this branch is broken.

Finally, the expert mode shows shows you the expert options for all screens when you pull it up for any screen, with no filtering available. Because of this, I've disabled the expert options for now and placed them on the last advanced pane[s] of each configuration screen.

This isn't really usable at the moment, but it is probably worth it to get it working rather than re-implementing the idea. However, I'm putting this on my "to merge" queue after the mythtv-vid and multiuser branches.

comment:12 Changed 13 years ago by danielk

(In [11681]) Refs #2316. Refs #2533. Cleanup of the transient mythtv settings.

Some were defined in dbsettings.cpp and some in settings.h. Some were duplicates. This moves them to settings.h and standardizes the naming.

comment:13 Changed 13 years ago 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.

comment:14 Changed 13 years ago by Jarod Wilson

(In [13237]) - Backport of trunk changesets [11681], [11682] and [11683]

to make digital cable channel scanning truly actually work (successfully tested by myself w/an AirStar? HD-5000 and HDHomeRun, and by Janne w/DVB-T and C)

comment:15 Changed 12 years ago by danielk

Resolution: wontfix
Status: newclosed

too much has changed for this to make sense to merge anymore.

comment:16 Changed 12 years ago by danielk

(In [17831]) Refs #2533. Abandoned effort.

Note: See TracTickets for help on using tickets.