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.

Sign up for GitHub

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

Jump to bottom

Improve resumepoints #652

Merged
merged 2 commits into from
Dec 22, 2019
Merged

Improve resumepoints #652

merged 2 commits into from
Dec 22, 2019

Conversation

mediaminister
Copy link
Collaborator

@mediaminister mediaminister commented Dec 19, 2019 •
edited
Loading

This pull request includes:

  • Reorganize resumepoints.py
  • Wait for the resumepoints to be updated before loading the menus

@mediaminister mediaminister mentioned this pull request Dec 19, 2019
Closed
@mediaminister mediaminister force-pushed the cache branch 6 times, most recently from b729ab6 to d9fd46b Compare December 19, 2019 12:32
@dagwieers dagwieers added the enhancement New feature or request label Dec 19, 2019
@dagwieers dagwieers added this to the v2.3.0 milestone Dec 19, 2019
@mediaminister mediaminister marked this pull request as ready for review December 19, 2019 14:09
@mediaminister mediaminister force-pushed the cache branch 5 times, most recently from 78ff2c6 to 029305d Compare December 19, 2019 16:39
@mediaminister mediaminister changed the title Improve resumepoints WIP: Improve resumepoints Dec 19, 2019
@mediaminister mediaminister changed the title WIP: Improve resumepoints Improve resumepoints Dec 19, 2019
@mediaminister mediaminister force-pushed the cache branch 12 times, most recently from d971ad3 to 7487a83 Compare December 20, 2019 14:23
@mediaminister mediaminister force-pushed the cache branch 7 times, most recently from b01feef to 8c14f12 Compare December 20, 2019 19:11
@mediaminister mediaminister force-pushed the cache branch 9 times, most recently from 13d18af to 3f835b1 Compare December 21, 2019 14:23
@dagwieers
Copy link
Collaborator

If this is ready, it can be merged. The implementation is a lot better.

My only concern is one that other add-ons do not make but is related to the use of properties on Window 10000. IMO it may be better to use a VRT NU related Window instead rather than the Main window. But I have not looked into the details myself yet.

The use of Windows ID 10000 could lead to conflicts with other add-ons (and could require namespacing our properties with e.g. vrtnu_), on the other hand using a VRT NU specific Window ID could potentially lose its information when switching to Kodi windows. (I don't know how Kodi behaves in this regard)

But this is a minor concern given that all add-ons I know are using Windows ID 10000.

@mediaminister
Copy link
Collaborator Author

I added vrtnu_ namespacing to the resumepoints properties. I'm going to merge this.

@mediaminister mediaminister merged commit 0785c6b into add-ons:master Dec 22, 2019
@mediaminister mediaminister deleted the cache branch May 22, 2020 08:52
@mediaminister mediaminister mentioned this pull request Oct 15, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dagwieers dagwieers dagwieers left review comments

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
v2.3.0
Development

Successfully merging this pull request may close these issues.
2 participants
@mediaminister @dagwieers