Big Mythgame patch

[Chris Lack <psycho_driver@…>]

Added new metadata fields to gamemetadata: developer, esrb, metarating, rating, players - The developer field is self explanatory and I personally find this a more useful sorting feature than publisher. The 'esrb' field adds support for parental rating values. 'rating' is a value that can be assigned to a game by the end user. 'metarating' is a value that can be populated by a metadata grabber. <not yet implemented: For sorting, if 'rating' is set, it will be used, otherwise the 'metarating' value will be used.> 'players' allows for sorting games based on the number of players they support.

Updated db version to 1019 to reflect changes.

Changed game tree from one metadata field per level to allow for tiered sorting based on up to nine metadata filters.

Implemented more functionality based around the 'display' metadata field, so games can be arbitrarily hidden. Added 'show/hide hidden' options to game tree menu in order to access games previously marked as hidden.

Added some additional functionality to the Giant Bomb metadata grabber. Also added image browser to choose images to assign to games. The most useful change was to work around a shortcoming of their API and allow displaying system type next to results. This makes deciding which result to use much less of a gamble. Also by removing the path in the art selector dialog you can unset a screenshot or coverart image. As a result of some of the changed functionality, it was necessary to add a 'publishers' field to the main myth metadata class. I imagine this could be a useful addition for some other media types. It would be fairly easy to remove this portion of the code if this change would be too intrusive.

Changed functionality of in-menu game scanner to not delete metadata for games that aren't present if the setup option requesting a prompt before removal has been set. Automatic removal is bad for people who use removable media for games.

Updated both of the default themes to allow editing of the new metadata values. The 4:3 editor is getting a bit cramped, but it all fit. I also implemented multiline description editors in both dialogs. Some adjustment may be desired by someone with a better eye for aesthetics than myself.

I am not a C++ programmer by trade, or even hobby. I'm pretty handy with C, so, it's not completely foreign, but I may have done something wrong in here without realizing it. It's all working quite well so far for me. Any pointers/suggestions/fixes are welcome.

Chris Lack psycho_driver@…

[robertm]

A lot of good thoughts here, but a lot of things that need addressing, too. This isn't a complete list, but it's what I see on a first read-through.

1) As you mention, this patch is big. Way, way too big. I will need these broken out into functional parts to be able to apply. Start with just the additions to the metadata, about which I have the following comments:

• First, a general comment. You should strive for consistency of operation between MythGame? and MythVideo?, Watch recordings, etc. in both terminology and functionality. Some of the following comments go towards that.

• ESRB is a North American entity, and thus doesn't apply to much of the world. I would prefer a term like "Certification" which is what is used in MythVideo? to indicate age ratings. Should also work to not use ESRB specific ratings in the as those are not used outside of the US, Canada, and Mexico.

• Instead of "metarating" use "userrating" which is what this general concept is called everywhere else. I would not bother to break up the metadata rating and the user rating as two distinct values. This isn't done anywhere else we get ratings from the metadata/listings provider/etc. Just fill an empty value with the metadata rating and let the user edit it as they prefer afterwards. Drop "rating."

• Developer and Players are fine.

2) Next thing you might break down/issue to address: Some of the metadata image handling changes here are outright wrong or really problematic-- You can't just stack up image selection windows, three per looked up item, on a metadata return. The original behavior is correct-- in a regular metadata lookup, the code needs to pull in the first item in the list and download it. If you want to give the user the opportunity to select from the list (and I agree that we should) then you need to implement specific buttons to do so like in MythVideo? (Find online artwork, etc). A regular metadata lookup shouldn't incur multiple stacked selection dialogs without any obvious text prompt-- it's inconsistent and unclear. Also, you've added a hardcoded setting of the host for image download to the master backend. This is incorrect. MythGame? doesn't support games in storage groups and doesn't support imagery in storage groups either. This is just going to download each image type to the storage group on the master backend, but does nothing to make them work when actually *loading* the images (and thus would only work if the SGs as defined on the MBE were actually also mounted on the various frontends). This part either needs to go or needs a serious rethink.

3) The Players UI widget should be a spinbox, not a textedit. Since you only want to accept integer input, you can enforce it by using the right widget. The rating UI widget should be a spinbox too, with a defined top and bottom, and sane increments in between rather than a buttonlist. Again here, you should follow the exampled set by MythVideo?, MythMusic, etc.

4) Once the above are addressed, we can add the theme changes so that the new metadata becomes visible/accessible

5) At this point you can probably safely add the changes to the tree building/filtering.

6) In the next patch, move on to the scanning behavior. Not removing if the prompt is enabled is fine, but you need to actually prompt if you're going to obey the setting, not just do nothing. You should add a prompt here if you want to see this behavior realized.

7) You can add the changes to the base metadata parsing classes and grabber scripts as a separate patch too. I have not looked at this one but I'll warn in advance that it's probably the one I'll take the most critical eye to, as these classes are used in a number of spots and I've worked to keep them clean.

As a final, unnumbered item, I think there's probably a lot here that if I read more carefully I will have some concerns with, but those things become a lot easier to pick out when the patch isn't 90 KB. Example: Why set all the artwork to NULL is file.size() == 1 ? This is broken in at least a couple of cases I can think of. Look for logical flaws and issues like this as you groom each piece for inclusion.

If you would like to start breaking this apart, we can start reviewing the various pieces for inclusion one by one.

[Chris Lack <psycho_driver@…>]

Thanks for the input. I'll see what I can do about breaking things into smaller chunks and implementing some of the changes you've suggested. It may be slow going as I work a lot of hours and have a baby and toddler, so my free time is at a bit of a premium.

[psycho_driver@…]

My concern with a single rating value is that if a user changes the rating, and then does a metadata refresh which pulls a rating value, the user-assigned value will be overwritten. I suppose one way of avoiding this would be to not overwrite any non-zero rating value with new metadata. A downside to this would be that the rating would remain static after an initial pull, even if it had shifted significantly on the source.

The downside to overwriting a rating supplied by the end-user would be the annoyance of having to re-set it.

The only downside to maintaining separate values would be that additional database field needed, and the work it will cause me figuring out a graceful way to sort the data based on the two values.

[robertm]

This ticket should not be out of infoneeded-- since the requested changes have never been submitted.