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

[skinning] Skin timer implementation #21320

Merged
merged 1 commit into from May 23, 2022
Merged

[skinning] Skin timer implementation #21320

merged 1 commit into from May 23, 2022

Conversation

enen92
Copy link
Member

@enen92 enen92 commented Apr 27, 2022

Motivation and context

Currently time-based logic in the skinning engine is pretty limited: skins either rely on the System.IdleTime boolean condition (which is linked to input) for checking "idle" state which doesn't necessarily mean no actions on that time, dummy windows (ref) for auto closing other windows, trigger the AlarmClock builtin with skin properties (see ref) or simply delegate time-based stuff to background python scripts. All of these solutions are quite hacky and some (such as python) have a performance penalty that may not be acceptable for a default skin or low-end hardware.

I've created this with 3 main use cases in mind:

Description

(c/p from the doxygen file)

Skin timers are skin objects that dependent on time and can be fully controlled from skins either using
builtin functions or boolean conditions/expressions. One can see them
as stopwatches that can be activated and deactivated automatically depending on the value of info expressions or simply activated/deactivated manually from builtins.

The framework was created to allow skins to control the visibility of windows (or controls) depending on
the elapsed time of timers the skin define. Skin timers allow multiple use cases in skins, previously only available via the execution of python scripts (or other hacks):

  • Close a specific window after x seconds have elapsed
  • Control the visibility of a group (or triggering an animation) depending on the elapsed time of a given timer
  • Define a buffer time window that is kept activated for a short period of time (e.g. keep controls visible for x seconds after a player seek)
  • Execute timed actions (on timer stop or timer start)
  • etc

Skin timers are defined in a Timers.xml file within the xml directory of the skin. The file has the following "schema":

<timers>
    <timer>...</timer>
    <timer>...</timer>
</timers>

Examples

The following example illustrates the simplest possible skin timer. This timer is completely manual (it has to be manually started and stopped):

<timer>
    <name>mymanualtimer</name>
    <description>100% manual timer</description>
</timer>

This timer can be controlled from your skin by executing the Skin.TimerStart(mymanualtimer) builtin or
the Skin.TimerStop(mymanualtimer) builtin. You can define the visibility of skin elements based on the internal
properties of the timer, such as the fact that the timer is active/running using Skin.TimerIsRunning(mymanualtimer) or depending on the elapsed time (e.g. 5 seconds) using the expression Integer.IsGreaterOrEqual(Skin.TimerElapsedSecs(mymanualtimer),5).

The following timer is a variation of the previous timer with the particularity of being automatically stopped by the skinning engine after a maximum of elapsed 5 seconds without having to issue the Skin.TimerStop(mymanualtimer) builtin:

<timer>
    <name>mymanualautocloseabletimer</name>
    <description>100% manual autocloseable timer</description>
    <stop>Integer.IsGreaterOrEqual(Skin.TimerElapsedSecs(mymanualautocloseabletimer),5)</stop>
</timer>

This type of timer is particularly useful if you want to automatically close a specific window (or triggering a close animation) after x time has elapsed, while guaranteeing the timer is also stopped. See the example below which trigger a slide animation on a window after it has been open for 5 seconds:

<?xml version="1.0" encoding="utf-8"?>
<window type="dialog" id="1109">
	<onload>Skin.TimerStart(mymanualautocloseabletimer)</onload>
    ...
	<controls>
		<control type="group">
			<animation effect="slide" start="0,0" end="0,-80" time="300" condition="Integer.IsGreaterOrEqual(Skin.TimerElapsedSecs(mymanualautocloseabletimer),5)">Conditional</animation>
			...
        </control>
    </controls>
</window>

The following timer presents a notification (for 1 sec) whenever the timer is activated or deactivated:

<timer>
    <name>manualtimerwithactions</name>
    <description>100% manual timer with actions</description>
    <onstart>Notification(skintimer, My timer was started, 1000)</onstart>
    <onstop>Notification(skintimer, My timer was stopped, 1000)</onstop>
</timer>

The following timer is an example of a completely automatic timer. The timer is automatically activated or deactivated based on the value
of boolean info expressions. In this particular example, the timer is automatically started whenever the Player is playing a file (if not already running). It is stopped if
there is no file being played (and of course if previously running). Since the timer can be activated/deactivated multiple times, reset="true" ensures the timer is
always reset to 0 on each start operation. Whenever the timer is started or stopped, notifications are issued.

<timer>
    <name>myautomatictimer</name>
    <description>Player state checker</description>
    <start reset="true">Player.Playing</start>
    <stop>!Player.Playing</stop>
    <onstart>Notification(skintimer, Player has started playing a file, 1000)</onstart>
    <onstop>Notification(skintimer, Player is no longer playing a file, 1000)</onstop>
</timer>

In certain situations you might want to reset your timer without having to stop and start. For instance, if you want to stop the timer after 4 seconds
but have the timer resetting to 0 seconds if the user provides some input to Kodi. For such cases the <reset/> condition can be used:

<timer>
    <name>windowtimer</name>
    <description>Reset on idle</description>
    <start reset="true">Window.IsActive(mywindow)</start>
    <reset>Window.IsActive(mywindow) + !System.IdleTime(1) + Integer.IsGreaterOrEqual(Skin.TimerElapsedSecs(windowtimer), 1)</reset>
    <stop>!Window.IsActive(mywindow) + Integer.IsGreaterOrEqual(Skin.TimerElapsedSecs(windowtimer), 4)</stop>
    <onstop>Dialog.Close(mywindow)</onstop>
</timer>

Available tags

Tag Description
name The unique name of the timer. The name is used as the id of the timer, hence needs to be unique. (required)
description The description of the timer, a helper string. (optional)
start An info bool expression that the skinning engine should use to automatically start the timer (optional)
reset An info bool expression that the skinning engine should use to automatically reset the timer (optional)
stop An info bool expression that the skinning engine should use to automatically stop the timer (optional)
onstart A builtin function that the skinning engine should execute when the timer is started (optional)
onstop A builtin function that the skinning engine should execute when the timer is stopped (optional)

How has this been tested?

Runtime tested with a skin timer implementation equivalent to #20816:

Timers.xml:

<?xml version="1.0" encoding="UTF-8"?>
<timers>
    <timer>
        <name>topbaroverlaytimer</name>
        <description>Timer valid for 5 seconds while the Player is Playing</description>
        <stop>[!Player.Paused | Integer.IsGreaterOrEqual(Skin.TimerElapsedSecs(topbaroverlaytimer),5)]</stop>
        <onstart>Notification(skintimer, timer started, 1000)</onstart>
        <onstop>Notification(skintimer, timer stopped, 1000)</onstop>
    </timer>
</timers>

Window:

<?xml version="1.0" encoding="utf-8"?>
<window type="dialog" id="1109">
	<onload>Skin.TimerStart(topbaroverlaytimer)</onload>
	...
	<controls>
		<control type="group">
			<animation effect="slide" start="0,0" end="0,-80" time="300" condition="Player.Paused + Integer.IsGreaterOrEqual(Skin.TimerElapsedSecs(topbaroverlaytimer),5)">Conditional</animation>
                        ...
                </control>
	</controls>
</window>

I plan to implement the other 2 use-cases shortly. Since they will stress test the implementation and they may require changes to this one I suggest only to merge this once they are finished. Code review is appreciated though whenever you find time.

What is the effect on users?

Skinners should now have a more powerful way of defining time-based logic in skins.

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)

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

xbmc/addons/Skin.cpp Outdated Show resolved Hide resolved
xbmc/addons/Skin.cpp Outdated Show resolved Hide resolved
xbmc/addons/Skin.cpp Outdated Show resolved Hide resolved
xbmc/addons/Skin.cpp Outdated Show resolved Hide resolved
xbmc/addons/gui/skin/SkinTimers.cpp Outdated Show resolved Hide resolved
xbmc/addons/gui/skin/SkinTimers.cpp Outdated Show resolved Hide resolved
xbmc/addons/gui/skin/SkinTimers.cpp Outdated Show resolved Hide resolved
xbmc/addons/gui/skin/SkinTimers.cpp Outdated Show resolved Hide resolved
xbmc/addons/gui/skin/SkinTimers.cpp Outdated Show resolved Hide resolved
xbmc/addons/gui/skin/SkinTimers.cpp Outdated Show resolved Hide resolved
@enen92 enen92 force-pushed the skintimers branch 4 times, most recently from cc8702f to 224671f Compare April 29, 2022 12:07
@enen92
Copy link
Member Author

enen92 commented Apr 29, 2022

Ok, I think I've adjusted all the comments. Thanks much!

@enen92 enen92 closed this Apr 29, 2022
@enen92 enen92 reopened this Apr 29, 2022
@enen92 enen92 force-pushed the skintimers branch 2 times, most recently from 02c2fe8 to c8e5a9c Compare May 3, 2022 22:40
@enen92 enen92 closed this May 4, 2022
@enen92 enen92 reopened this May 4, 2022
@jenkins4kodi jenkins4kodi added the Rebase needed PR that does not apply/merge cleanly to current base branch label May 4, 2022
@jenkins4kodi jenkins4kodi removed the Rebase needed PR that does not apply/merge cleanly to current base branch label May 5, 2022
@jjd-uk
Copy link
Member

jjd-uk commented May 7, 2022

Based on the Estuary PR's #21328 and #21355 the changes here seem to be working as expected. Can't review if the C++ changes are handled in the best way however.

@jenkins4kodi jenkins4kodi removed the Rebase needed PR that does not apply/merge cleanly to current base branch label May 9, 2022
@enen92 enen92 closed this May 9, 2022
@enen92 enen92 reopened this May 9, 2022
@enen92
Copy link
Member Author

enen92 commented May 20, 2022

All three use cases mentioned in the PR post have been implemented and validated the implementation.
Would be great to have reviews/approvals on this one so it can go in for alpha2

@basilgello
Copy link
Collaborator

Looks good to me from the brief look at the code. I will abstain from approval though until I make a Debian build.

@jjd-uk
Copy link
Member

jjd-uk commented May 21, 2022

@enen92 I'll try and find the time later to look at that doxygen file, if not later then tomorrow at latest.

FYI once this and your related Estuary PR's are in.

I've got this ready to PR

image

So ability to use or not use skin timer to autoclose seekbar when paused, and be able to configure the time after which autoclose takes effect.

I've also been able to work after much trial & error how to have a configurable DisplayAfterSeek replacement using skin timers, which is the setting Seekbar autoclose after seeking time (seconds)

@enen92
Copy link
Member Author

enen92 commented May 21, 2022

Great! Too bad in the skin settings window we don't have access to setting levels (afaik). I am a bit afraid in that screenshots we clutter settings and add too much complexity.
No problem about time, take all you need.

@jjd-uk
Copy link
Member

jjd-uk commented May 21, 2022

The additional complexity is why I'll leave it until your Estaury PR's are in, and then offer this as an enhancement. So nothing is held up if bike shedding starts on what I've done. If nothing else it will showcase to skinners the other possibilities such as doing a different displayafterseek replacement implementation.

@enen92
Copy link
Member Author

enen92 commented May 22, 2022

Merging by tomorrow night if no more feedback/requests

@enen92
Copy link
Member Author

enen92 commented May 23, 2022

Today is yesterday's tomorrow night :)
Merging.

@enen92 enen92 merged commit 586b89e into xbmc:master May 23, 2022
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

6 participants