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

[guiinfo]Fix Musicplayer.position(n).xyz and Musicplayer.offset(n).UserRating info labels #16657

Merged
merged 2 commits into from Sep 25, 2019

Conversation

@DaveTBlake
Copy link
Member

DaveTBlake commented Sep 22, 2019

Fix Musicplayer.position(n).xyz that stopped working for all labels xyz after #13754 the major refactoring of CGUIInfoManager .

Fix Musicplayer.offset(n).UserRating which was accidentally excluded by a range check on musicplayer infolables that had not been updated when new labels added.

See https://forum.kodi.tv/showthread.php?tid=330696&pid=2846886#pid2846886 for skinner reports of this issue.

Also reorder the defines to group bools separately from labels (after colluding with @ronie )

@scott967 perhaps you have time to test too.

@DaveTBlake DaveTBlake added this to the Matrix 19.0-alpha 1 milestone Sep 22, 2019
@DaveTBlake DaveTBlake requested a review from ronie Sep 22, 2019
@DaveTBlake DaveTBlake changed the title Fix Musicplayer.position(n).xyz and Musicplayer.offset(n).UserRating info labels [guiinfo]Fix Musicplayer.position(n).xyz and Musicplayer.offset(n).UserRating info labels Sep 22, 2019
xbmc/GUIInfoManager.cpp Outdated Show resolved Hide resolved
xbmc/guilib/guiinfo/GUIInfoLabels.h Outdated Show resolved Hide resolved
@ronie

This comment has been minimized.

Copy link
Member

ronie commented Sep 22, 2019

i've runtime tested it and apart from my comments it's working fine.

@ksooo

This comment has been minimized.

Copy link
Member

ksooo commented Sep 22, 2019

i've runtime tested it and apart from my comments it's working fine.

okay, thanks.

@DaveTBlake DaveTBlake force-pushed the DaveTBlake:PositionInfolabelFix branch from 640574f to 3c71d68 Sep 23, 2019
@DaveTBlake

This comment has been minimized.

Copy link
Member Author

DaveTBlake commented Sep 23, 2019

For anyone looking back later:
No stack overflow happens, as CMusicGUIInfo::GetLabel only calls CMusicGUIInfo::GetPlaylistInfo when data1 is non-zero (hence the change of data1 to 2 for absolute position), and GetPlaylistInfo calls GetLabel with a new CGUIInfo hence data1 is 0.

@DaveTBlake DaveTBlake force-pushed the DaveTBlake:PositionInfolabelFix branch from 3c71d68 to 5316053 Sep 23, 2019
…et and position for range checking separately from others e.g. infobools. Define range.
@DaveTBlake DaveTBlake force-pushed the DaveTBlake:PositionInfolabelFix branch from 5316053 to e6b3a06 Sep 23, 2019
@ksooo
ksooo approved these changes Sep 23, 2019
Copy link
Member

ksooo left a comment

lgtm

@ksooo ksooo requested a review from ronie Sep 25, 2019
@ksooo

This comment has been minimized.

Copy link
Member

ksooo commented Sep 25, 2019

@ronie could you please approve. Your requested changes were incorporated meanwhile.

@ronie
ronie approved these changes Sep 25, 2019
@ronie

This comment has been minimized.

Copy link
Member

ronie commented Sep 25, 2019

done

@ksooo ksooo merged commit f27354e into xbmc:master Sep 25, 2019
1 check passed
1 check passed
default You're awesome. Have a cookie
Details
@DaveTBlake DaveTBlake deleted the DaveTBlake:PositionInfolabelFix branch Sep 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.