Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Game Infolabel and GUI settings update #14720

Merged
merged 4 commits into from
Oct 27, 2018
Merged

Conversation

garbear
Copy link
Member

@garbear garbear commented Oct 26, 2018

Game skinning in Kodi is now fully documented: https://forum.kodi.tv/showthread.php?tid=336876

While creating this, I noticed some inconsistencies. I made the following changes:

Infolabels

  • Dropped Listitem.Property(duration) (Savestate duration, left over from savestate manager)
  • Added RetroPlayer.VideoFilter. Possible values:
    • nearest (Nearest-neighbor, a.k.a. pixellate)
    • linear (Bilinear filtering, a.k.a. smooth blur)
    • <path to shader> - not supported yet
  • Added RetroPlayer.VideoRotation - Rotation of game in degrees CCW

GUI settings

  • Changed stretch mode from int (0 through 3) to string identifier for consistency with RetroPlayer.StretchMode
<defaultgamesettings>
    <stretchmode>normal</stretchmode>
</defaultgamesettings>

Valid identifiers are:

  • normal - Show the game at its correct aspect ratio
  • 4:3 - Stretch the game to 4:3 aspect ratio
  • fullscreen - Stretch the game to the full viewing area
  • original - Shrink the game to its original size (humorous with old games on 4K displays)

Motivation and Context

For consistency with game documentation: https://forum.kodi.tv/showthread.php?tid=336876

How Has This Been Tested?

<stretchmode> gui setting migration tested on OSX. Set the value to 1, applied the patch, observed the correct behavior - old value is ignored, new value is set to normal

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • Clean up (non-breaking change which removes non-working, unmaintained functionality)
  • Improvement (non-breaking change which improves existing functionality)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that will cause existing functionality to change)
  • Cosmetic change (non-breaking change that doesn't touch code)
  • None of the above (please explain below)

@garbear garbear changed the title Game osd doc Game Infolabel and GUI settings update Oct 26, 2018
@@ -2118,6 +2118,15 @@ const infomap videoplayer[] = {{ "title", VIDEOPLAYER_TITLE },
/// @{
/// \table_start
/// \table_h3{ Labels, Type, Description }
/// \table_row3{ <b>`VideoFilter`</b>,
/// \anchor RetroPlayer_VideoFilter

This comment was marked as spam.

/// The following values are possible:
/// - nearest (Nearest-neighbor, i.e. pixellate)
/// - linear (Bilinear, i.e. smooth blur)
/// - <path to shader> (not supported yet)

This comment was marked as spam.

This comment was marked as spam.

@@ -2118,6 +2118,15 @@ const infomap videoplayer[] = {{ "title", VIDEOPLAYER_TITLE },
/// @{
/// \table_start
/// \table_h3{ Labels, Type, Description }
/// \table_row3{ <b>`VideoFilter`</b>,

This comment was marked as spam.

@enen92
Copy link
Member

enen92 commented Oct 26, 2018

Doing a quick search on the new XML windows for games there are also references to Infolabels that are not documented, namely several options for Listitem.Property:

<videofilter>$INFO[ListItem.Property(game.videofilter)]</videofilter>
<stretchmode>$INFO[ListItem.Property(game.stretchmode)]</stretchmode>
<rotation>$INFO[ListItem.Property(game.videorotation)]</rotation>

Can you please also add those to the infolabels doxygen page like all the others:
https://codedocs.xyz/xbmc/xbmc/modules___general___list_of_gui_access.html#modules__General__List_of_gui_access_ListItem

Thanks

@garbear
Copy link
Member Author

garbear commented Oct 26, 2018

@enen92 pushed a fix, how's it look now?

Copy link
Member

@enen92 enen92 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good thanks. Only the properties are missing but that can be done later.

@garbear
Copy link
Member Author

garbear commented Oct 26, 2018

I'll do it now, instead of duplicating list of values, how do I reference a previous infolabel?

@enen92
Copy link
Member

enen92 commented Oct 26, 2018

Not sure I understand what you mean. Listitem.Property is repeated already several times for each property. You can however define just one new table entry and simplify it like below:

///   \table_row3{   <b>`ListItem.Property(Game.*)`</b>,
///                  \anchor ListItem_Property_Game
///                  _string_,
///     Bblablablablabla. Possible values
///     |     Property     |         Returned value      |
///     |:----------------:|:---------------------------:|
///     | game.foo         | Returns the value of foo    |
///     | game.bar         | Returns the value of bar    |
///   }

@garbear
Copy link
Member Author

garbear commented Oct 26, 2018

For example, the possible values for ListItem.Property(Game.VideoFilter) are the same as RetroPlayer.VideoFilter, so in the doxy for ListItem.Property(Game.VideoFilter) I want to say "See RetroPlayer.VideoFilter for a list of possible values" with a URL fragment that links to the RetroPlayer_VideoFilter anchor

@enen92
Copy link
Member

enen92 commented Oct 26, 2018

Sorry for the delay. Yes you can do something like below:

See \link RetroPlayer_VideoRotation this link \endlink for available things

@garbear
Copy link
Member Author

garbear commented Oct 26, 2018

@enen92 updated

@enen92
Copy link
Member

enen92 commented Oct 26, 2018

I'd prefer to see them grouped with all the other ListItem properties (since they refer to ListItem and not to RetroPlayer) but I guess it's a matter of taste. If you prefer to group them in the retroplayer section is fine to me too 👍

@garbear
Copy link
Member Author

garbear commented Oct 26, 2018

Grouping the listitems together would be nice, but keeping RP stuff together is less overhead for me. I'll merge when jenkins is happy.

@garbear
Copy link
Member Author

garbear commented Oct 26, 2018

jenkins build this please

@garbear garbear merged commit ff6ab0d into xbmc:master Oct 27, 2018
@garbear garbear deleted the game-osd-doc branch October 27, 2018 00:37
@Rechi Rechi added this to the Leia 18.0-beta5 milestone Oct 27, 2018
@garbear
Copy link
Member Author

garbear commented Oct 28, 2018

@enen92 I completed the documentation in the forum thread. However, I don't think the Game.* infolabels are documented in the doxygen source. Can you please copy the forum documentation into the source?

@enen92
Copy link
Member

enen92 commented Oct 28, 2018

Sure, will do

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants