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

[python] Add xbmcgui.DialogBusy() #10699

Merged
merged 3 commits into from Nov 2, 2016

Conversation

@phil65
Copy link
Member

commented Oct 13, 2016

Description

Adds a new dialog class to the python interface.

Motivation and Context

Lot of python scripters are making use of our busy dialog although there is no proper interface to do that. So their solution is to use executebuiltin("ActivateWindow(busydialog)") and executebuiltin("Dialog.Close(busydialog)") in order to open / close it, which feels very workaround-ish.
Because of that, I added busydialog handling to our xbmcgui class, with a similar interface as our progress dialog.
When doin this I also found out that our busydialog core code supports a progress bar. That one can be used via python as well now, and I also added one to estuary in a separate commit.

How Has This Been Tested?

With some simple test scripts

Screenshots (if appropriate):

screenshot041

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the Code guidelines of this project
  • My change requires a change to the documentation, either Doxygen or wiki
  • I have updated the documentation accordingly
  • I have read the CONTRIBUTING document
  • I have added tests to cover my change
  • All new and existing tests passed
@MartijnKaijser

This comment has been minimized.

Copy link
Member

commented Oct 13, 2016

jenkins build this please

@phil65

This comment has been minimized.

Copy link
Member Author

commented Oct 13, 2016

@tamland for review, please.

throw WindowException("Error: Window is NULL, this is not possible :-)");

dlg = pDialog;
open = true;

This comment has been minimized.

Copy link
@tamland

tamland Oct 15, 2016

Member

this open flag doesn't seem to do anything..

This comment has been minimized.

Copy link
@phil65

phil65 Oct 17, 2016

Author Member

it gets checked in deallocating() (it´s done this way for DialogProgress, too.)
Can I drop it?

void DialogBusy::update(int percent) const
{
DelayedCallGuard dcguard(languageHook);
auto pDialog = dlg;

This comment has been minimized.

Copy link
@tamland

tamland Oct 15, 2016

Member

does nothing

This comment has been minimized.

Copy link
@phil65

phil65 Oct 17, 2016

Author Member

true. will fix.

///
/// @param percent integer - percent complete. (0:100)
///
/// @note If percent == 0, the progressbar will be hidden.

This comment has been minimized.

Copy link
@tamland

tamland Oct 15, 2016

Member

Sounds like a bug. It should be possible to have a progress bar at 0%

This comment has been minimized.

Copy link
@MartijnKaijser

MartijnKaijser Oct 15, 2016

Member

Currently there's indeed no progress bar when 0% You see this as well when installing Addons from repo

This comment has been minimized.

Copy link
@phil65

phil65 Oct 17, 2016

Author Member

Yup, I just did it in same way as progress dialog. I agree that it makes sense to change this behaviour for both.

This comment has been minimized.

Copy link
@phil65

phil65 Oct 17, 2016

Author Member

Should I add a commit to change behaviour to this PR?

/// ..
/// dialog.close()
/// ..
/// ~~~~~~~~~~~~~

This comment has been minimized.

Copy link
@tamland

tamland Oct 15, 2016

Member

I think you can drop all of these "examples". They are pretty useless!:)

This comment has been minimized.

Copy link
@phil65

phil65 Oct 17, 2016

Author Member

Ok, will only keep the one for initialization then. ( .create() )

@phil65

This comment has been minimized.

Copy link
Member Author

commented Oct 17, 2016

@MartijnKaijser Should I add the @python_v17 stuff just to the class description or to every single method of the class?

/// ~~~~~~~~~~~~~
///
update(...);
update(...);

This comment has been minimized.

Copy link
@garbear

garbear Oct 17, 2016

Member

whitespace oops

@MartijnKaijser

This comment has been minimized.

Copy link
Member

commented Oct 17, 2016

Each one please as that's clearest

@MartijnKaijser

This comment has been minimized.

Copy link
Member

commented Oct 26, 2016

@tamland ping for review if you have a moment

@phil65 phil65 force-pushed the phil65:python_busydialog branch 2 times, most recently from 380d3c5 to 3bf2fd2 Oct 28, 2016

@tamland

This comment has been minimized.

Copy link
Member

commented Oct 31, 2016

Need to update the documentation on the percent parameter, otherwise it looks good.

@phil65 phil65 force-pushed the phil65:python_busydialog branch 3 times, most recently from daaa5d3 to fbfb7f5 Nov 2, 2016

@phil65 phil65 force-pushed the phil65:python_busydialog branch from fbfb7f5 to 8c02804 Nov 2, 2016

@phil65

This comment has been minimized.

Copy link
Member Author

commented Nov 2, 2016

updated.

@MartijnKaijser

This comment has been minimized.

Copy link
Member

commented Nov 2, 2016

Jenkins build this please

@MartijnKaijser MartijnKaijser added this to the Krypton 17.0-beta6 milestone Nov 2, 2016

@MartijnKaijser MartijnKaijser merged commit c05fc78 into xbmc:master Nov 2, 2016

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
jenkins4kodi You did a great job. Have a cookie.
Details

@phil65 phil65 deleted the phil65:python_busydialog branch Dec 11, 2016

@FernetMenta

This comment has been minimized.

Copy link
Member

commented May 29, 2018

@phil65 calling busy dialog from python is nonsense. the busy dialog is made to keep the GUI going while the calling thread is blocked.

calling dialogBusy from python leads to having multiple busy dialogs open at the same time:
https://trac.kodi.tv/ticket/17896

further: opening a modal dialog via a message is wrong anyway. leads to having the busy dialog open but obscured by some other dialog.

@FernetMenta

This comment has been minimized.

Copy link
Member

commented May 29, 2018

I will deprecte this and log a message when this is called.

@Razzeee

This comment has been minimized.

Copy link
Member

commented May 29, 2018

Please also make sure to block Dialog.Close(busydialog) and ActivateWindow(busydialog) then.

@FernetMenta

This comment has been minimized.

Copy link
Member

commented May 30, 2018

Please also make sure to block Dialog.Close(busydialog) and ActivateWindow(busydialog) then.

already had ActivateWindow(busydialog), added closedialog now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.