Skip to content

Commit 7473e2d

Browse files
JarbasAlclaude
andcommitted
feat: session-aware game-skill + OCP playback
Per-session playback state in OVOSCommonPlaybackSkill (state events emit via message.reply so they route to the originating session) and game_skill on the context-gated layers (probing skill_will_match with its session). OCPCommonPlayback deprecation is handled in #423. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent e4ef6f7 commit 7473e2d

4 files changed

Lines changed: 155 additions & 57 deletions

File tree

ovos_workshop/skills/common_play.py

Lines changed: 52 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717
from typing import List, Callable, Optional, Dict
1818

1919
from ovos_bus_client import Message
20+
from ovos_bus_client.message import dig_for_message
21+
from ovos_bus_client.session import SessionManager
2022
from ovos_config.locations import get_xdg_cache_save_path
2123
from ovos_utils import camel_case_split
2224
from ovos_utils.log import LOG
@@ -109,15 +111,44 @@ def __init__(self, *args,
109111
self.__prev_handler = prev_handler
110112
self.__resume_handler = resume_handler
111113
self._stop_event = Event()
112-
self._playing = Event()
113-
self._paused = Event()
114+
# OCP is session aware: track playing/paused state per session_id so
115+
# several sessions can play (and pause/stop) independently
116+
self._playing_sessions = set()
117+
self._paused_sessions = set()
114118
# TODO new default icon
115119
self.skill_icon = skill_icon or ""
116120

117121
self.ocp_matchers: Dict[str, AhocorasickNER] = {}
118122
self._ocp_ents: Dict[str, List[str]] = {}
119123
super().__init__(*args, **kwargs)
120124

125+
# --- per-session playback state (OCP is session aware) ------------------
126+
@staticmethod
127+
def get_session_id(message: Optional[Message] = None) -> str:
128+
"""Resolve the session id for the current/given message."""
129+
message = message or dig_for_message()
130+
return SessionManager.get(message).session_id
131+
132+
def is_playing_in(self, session_id: str) -> bool:
133+
return session_id in self._playing_sessions
134+
135+
def is_paused_in(self, session_id: str) -> bool:
136+
return session_id in self._paused_sessions
137+
138+
@property
139+
def playing_sessions(self) -> List[str]:
140+
return list(self._playing_sessions)
141+
142+
@property
143+
def is_playing(self) -> bool:
144+
"""True if the *current* session is playing (back-compat property)."""
145+
return self.is_playing_in(self.get_session_id())
146+
147+
@property
148+
def is_paused(self) -> bool:
149+
"""True if the *current* session is paused (back-compat property)."""
150+
return self.is_paused_in(self.get_session_id())
151+
121152
def _read_skill_name_voc(self):
122153
"""
123154
Load skill name aliases from a vocabulary file or generate them from the class name if not found.
@@ -456,14 +487,15 @@ def __handle_ocp_play(self, message):
456487
457488
If a playback handler is registered, it is called with the message if accepted, and the player state is set to PLAYING. Logs an error if no playback handler is implemented.
458489
"""
459-
self._playing.set()
460-
self._paused.clear()
490+
sid = self.get_session_id(message)
491+
self._playing_sessions.add(sid)
492+
self._paused_sessions.discard(sid)
461493
if self.__playback_handler:
462494
params = signature(self.__playback_handler).parameters
463495
kwargs = {"message": message} if "message" in params else {}
464496
self.__playback_handler(**kwargs)
465-
self.bus.emit(Message("ovos.common_play.player.state",
466-
{"state": PlayerState.PLAYING}))
497+
self.bus.emit(message.reply("ovos.common_play.player.state",
498+
{"state": PlayerState.PLAYING}))
467499
else:
468500
LOG.error(f"Playback requested but {self.skill_id} handler not "
469501
"implemented")
@@ -474,13 +506,14 @@ def __handle_ocp_pause(self, message):
474506
475507
If no pause handler is implemented, logs an error.
476508
"""
477-
self._paused.set()
509+
sid = self.get_session_id(message)
510+
self._paused_sessions.add(sid)
478511
if self.__pause_handler:
479512
params = signature(self.__pause_handler).parameters
480513
kwargs = {"message": message} if "message" in params else {}
481514
if self.__pause_handler(**kwargs):
482-
self.bus.emit(Message("ovos.common_play.player.state",
483-
{"state": PlayerState.PAUSED}))
515+
self.bus.emit(message.reply("ovos.common_play.player.state",
516+
{"state": PlayerState.PAUSED}))
484517
else:
485518
LOG.error(f"Pause requested but {self.skill_id} handler not "
486519
"implemented")
@@ -489,13 +522,14 @@ def __handle_ocp_resume(self, message):
489522
"""
490523
Handles OCP resume requests by invoking the registered resume handler and updating the player state to PLAYING if successful. Logs an error if no resume handler is implemented.
491524
"""
492-
self._paused.clear()
525+
sid = self.get_session_id(message)
526+
self._paused_sessions.discard(sid)
493527
if self.__resume_handler:
494528
params = signature(self.__resume_handler).parameters
495529
kwargs = {"message": message} if "message" in params else {}
496530
if self.__resume_handler(**kwargs):
497-
self.bus.emit(Message("ovos.common_play.player.state",
498-
{"state": PlayerState.PLAYING}))
531+
self.bus.emit(message.reply("ovos.common_play.player.state",
532+
{"state": PlayerState.PLAYING}))
499533
else:
500534
LOG.error(f"Resume requested but {self.skill_id} handler not "
501535
"implemented")
@@ -520,13 +554,14 @@ def __handle_ocp_prev(self, message):
520554

521555
def __handle_ocp_stop(self, message):
522556
# for skills managing their own playback
523-
if self._playing.is_set():
524-
self._paused.clear()
557+
sid = self.get_session_id(message)
558+
if sid in self._playing_sessions:
559+
self._paused_sessions.discard(sid)
525560
self.stop()
526561
self.gui.release()
527-
self.bus.emit(Message("ovos.common_play.player.state",
528-
{"state": PlayerState.STOPPED}))
529-
self._playing.clear()
562+
self.bus.emit(message.reply("ovos.common_play.player.state",
563+
{"state": PlayerState.STOPPED}))
564+
self._playing_sessions.discard(sid)
530565

531566
def __handle_stop_search(self, message):
532567
self._stop_event.set()

ovos_workshop/skills/game_skill.py

Lines changed: 78 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,17 @@
22
from typing import Optional, Dict, Iterable
33

44
from ovos_bus_client.message import Message
5+
from ovos_bus_client.session import SessionManager
56
from ovos_bus_client.util import get_message_lang
67
from ovos_utils.ocp import MediaType, MediaEntry, PlaybackType, Playlist, PlayerState
78
from ovos_utils.parse import match_one, MatchStrategy
89

910
from ovos_workshop.decorators import ocp_featured_media, ocp_search
1011
from ovos_workshop.skills.common_play import OVOSCommonPlaybackSkill
12+
from ovos_workshop.skills.converse import ConversationalSkill
1113

1214

13-
class OVOSGameSkill(OVOSCommonPlaybackSkill):
15+
class OVOSGameSkill(OVOSCommonPlaybackSkill, ConversationalSkill):
1416
""" To integrate with the OpenVoiceOS Common Playback framework
1517
1618
"play" intent is shared with media and managed by OCP pipeline
@@ -82,13 +84,8 @@ def _ocp_search(self, phrase: str, media_type: MediaType) -> Iterable[MediaEntry
8284
entry.match_confidence = conf
8385
yield entry
8486

85-
@property
86-
def is_playing(self) -> bool:
87-
return self._playing.is_set()
88-
89-
@property
90-
def is_paused(self) -> bool:
91-
return self._paused.is_set()
87+
# is_playing / is_paused (and per-session variants) are provided by
88+
# OVOSCommonPlaybackSkill and are session aware.
9289

9390
@abc.abstractmethod
9491
def on_play_game(self):
@@ -119,13 +116,14 @@ def on_load_game(self):
119116

120117
def stop_game(self):
121118
"""to be called by skills if they want to stop game programatically"""
122-
if self.is_playing:
123-
self._paused.clear()
119+
sid = self.get_session_id()
120+
if self.is_playing_in(sid):
121+
self._paused_sessions.discard(sid)
124122
self.gui.release()
125123
self.log.debug("changing OCP state: PlayerState.STOPPED ")
126124
self.bus.emit(Message("ovos.common_play.player.state",
127125
{"state": PlayerState.STOPPED}))
128-
self._playing.clear()
126+
self._playing_sessions.discard(sid)
129127
self.on_stop_game()
130128
return True
131129
return False
@@ -134,17 +132,35 @@ def stop(self) -> bool:
134132
"""NOTE: not meant to be called by the skill, this is a callback"""
135133
return self.stop_game()
136134

135+
# pipelines that are NOT a real intent match (they should not count when a
136+
# game asks "would an intent be selected for this utterance?")
137+
_NON_INTENT_PIPELINES = ("converse", "fallback", "common_query", "stop")
138+
137139
def calc_intent(self, utterance: str, lang: str, timeout=1.0) -> Optional[Dict[str, str]]:
138-
"""helper to check what intent would be selected by ovos-core"""
140+
"""helper to check what intent would be selected by ovos-core
141+
142+
NOTE: converse, common_query, stop and fallbacks are intentionally
143+
ignored here - this reports only a genuine intent-parser match (adapt /
144+
padatious / padacioso), which is what gates whether a layer intent will
145+
fire for the utterance.
146+
"""
139147
# let's see what intent ovos-core will assign to the utterance
140-
# NOTE: converse, common_query and fallbacks are not included in this check
141148
response = self.bus.wait_for_response(Message("intent.service.intent.get",
142149
{"utterance": utterance, "lang": lang}),
143150
"intent.service.intent.reply",
144151
timeout=timeout)
145152
if not response:
146153
return None
147-
return response.data["intent"]
154+
intent = response.data.get("intent")
155+
if not intent:
156+
return None
157+
# the full pipeline runs converse first; a game is "active" so converse
158+
# would always claim the utterance. Ignore non-intent-parser pipelines so
159+
# this reflects whether a *layer intent* would actually match.
160+
pipeline = (intent.get("intent_service") or "").lower()
161+
if any(p in pipeline for p in self._NON_INTENT_PIPELINES):
162+
return None
163+
return intent
148164

149165

150166
class ConversationalGameSkill(OVOSGameSkill):
@@ -158,16 +174,20 @@ def on_load_game(self):
158174
self.speak_dialog("cant_load_game")
159175

160176
def on_pause_game(self):
161-
"""called by ocp_pipeline on 'pause' if game is being played"""
162-
self._paused.set()
177+
"""called by ocp_pipeline on 'pause' if game is being played
178+
179+
NOTE: the per-session paused state is already set by the OCP framework
180+
before this handler runs."""
163181
self.acknowledge()
164182
# individual skills can change default value if desired
165183
if self.settings.get("pause_dialog", False):
166184
self.speak_dialog("game_pause")
167185

168186
def on_resume_game(self):
169-
"""called by ocp_pipeline on 'resume/unpause' if game is being played and paused"""
170-
self._paused.clear()
187+
"""called by ocp_pipeline on 'resume/unpause' if game is being played and paused
188+
189+
NOTE: the per-session paused state is already cleared by the OCP
190+
framework before this handler runs."""
171191
self.acknowledge()
172192
# individual skills can change default value if desired
173193
if self.settings.get("pause_dialog", False):
@@ -183,10 +203,14 @@ def on_stop_game(self):
183203
auto-save may be implemented here"""
184204

185205
@abc.abstractmethod
186-
def on_game_command(self, utterance: str, lang: str):
206+
def on_game_command(self, utterance: str, lang: str,
207+
message: Optional[Message] = None):
187208
"""pipe user input that wasnt caught by intents to the game
188209
do any intent matching or normalization here
189210
don't forget to self.speak the game output too!
211+
212+
@param message: the utterance Message (carries the session); use it to
213+
key per-session game state so several sessions can play at once.
190214
"""
191215

192216
def on_abandon_game(self):
@@ -198,6 +222,34 @@ def on_abandon_game(self):
198222
on_game_stop will be called after this handler"""
199223

200224
# converse
225+
def can_converse(self, message: Message) -> bool:
226+
"""A game wants to converse while it is being played (or paused), EXCEPT
227+
when one of its own context-gated layer intents would match the
228+
utterance - those must be handled by the intent pipeline (adapt), not
229+
swallowed by converse.
230+
231+
This lets the converse pipeline route only the free-form input that no
232+
layer intent matched into `on_game_command`.
233+
"""
234+
if not (self.is_playing or self.is_paused):
235+
return False
236+
try:
237+
utterance = message.data.get("utterances", [""])[0]
238+
lang = get_message_lang(message)
239+
# The converse pipeline runs *before* adapt, so while a game is
240+
# active converse would otherwise always claim the utterance and
241+
# prevent layer intents from matching. Probe the intent service
242+
# (read-only) excluding the converse stage (to avoid re-entering
243+
# this very check); if one of our context-gated layer intents would
244+
# match, let adapt handle it instead of swallowing it here.
245+
if utterance and self.skill_will_match(
246+
utterance, lang, exclude_pipeline=["ovos-converse-pipeline-plugin"],
247+
session=SessionManager.get(message)):
248+
return False
249+
except Exception as e:
250+
self.log.debug(f"can_converse intent-gate failed: {e}")
251+
return True
252+
201253
def skill_will_trigger(self, utterance: str, lang: str, skill_id: Optional[str] = None, timeout=0.8) -> bool:
202254
"""helper to check if this skill would be selected by ovos-core with the given utterance
203255
@@ -230,15 +282,18 @@ def _async_cmd(self, message: Message):
230282
utterance = message.data["utterances"][0]
231283
lang = get_message_lang(message)
232284
self.log.debug(f"Piping utterance to game: {utterance}")
233-
self.on_game_command(utterance, lang)
285+
self.on_game_command(utterance, lang, message)
234286

235287
def converse(self, message: Message) -> bool:
236288
try:
237289
utterance = message.data["utterances"][0]
238290
lang = get_message_lang(message)
239-
# let the user implemented intents do the job if they can handle the utterance
240-
# otherwise pipe utterance to the game handler
241-
if self.skill_will_trigger(utterance, lang):
291+
# let the user implemented (context-gated) intents do the job if they
292+
# can handle the utterance, otherwise pipe utterance to the game
293+
# handler. Probe excludes converse to avoid re-entrancy.
294+
if self.skill_will_match(utterance, lang,
295+
exclude_pipeline=["ovos-converse-pipeline-plugin"],
296+
session=SessionManager.get(message)):
242297
self.log.debug("Skill intent will trigger, don't pipe utterance to game")
243298
return False
244299

test/unittests/skills/test_common_play_extended.py

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -89,13 +89,16 @@ def test_skill_icon_default_empty(self) -> None:
8989
def test_search_handlers_initially_empty(self) -> None:
9090
self.assertIsInstance(self.skill._search_handlers, list)
9191

92-
def test_playing_event_not_set(self) -> None:
93-
"""_playing event is not set on init (not actively playing)."""
94-
self.assertFalse(self.skill._playing.is_set())
95-
96-
def test_paused_event_not_set(self) -> None:
97-
"""_paused event is not set on init."""
98-
self.assertFalse(self.skill._paused.is_set())
92+
def test_no_playing_sessions_on_init(self) -> None:
93+
"""No session is marked as playing on init (not actively playing)."""
94+
self.assertEqual(self.skill._playing_sessions, set())
95+
self.assertEqual(self.skill.playing_sessions, [])
96+
self.assertFalse(self.skill.is_playing)
97+
98+
def test_no_paused_sessions_on_init(self) -> None:
99+
"""No session is marked as paused on init."""
100+
self.assertEqual(self.skill._paused_sessions, set())
101+
self.assertFalse(self.skill.is_paused)
99102

100103
def test_register_media_type(self) -> None:
101104
"""register_media_type adds a new type to supported_media."""

test/unittests/skills/test_game_skill_extended.py

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -27,19 +27,24 @@ class ConcreteGameSkill(OVOSGameSkill):
2727
"""Minimal concrete subclass implementing all abstract methods."""
2828

2929
def on_play_game(self):
30-
self._playing.set()
30+
sid = self.get_session_id()
31+
self._playing_sessions.add(sid)
32+
self._paused_sessions.discard(sid)
3133

3234
def on_pause_game(self):
33-
self._paused.set()
34-
self._playing.clear()
35+
sid = self.get_session_id()
36+
self._paused_sessions.add(sid)
37+
self._playing_sessions.discard(sid)
3538

3639
def on_resume_game(self):
37-
self._paused.clear()
38-
self._playing.set()
40+
sid = self.get_session_id()
41+
self._paused_sessions.discard(sid)
42+
self._playing_sessions.add(sid)
3943

4044
def on_stop_game(self):
41-
self._playing.clear()
42-
self._paused.clear()
45+
sid = self.get_session_id()
46+
self._playing_sessions.discard(sid)
47+
self._paused_sessions.discard(sid)
4348

4449
def on_save_game(self):
4550
pass
@@ -158,13 +163,13 @@ def test_stop_game_when_not_playing_returns_false(self) -> None:
158163

159164
def test_stop_game_when_playing_returns_true(self) -> None:
160165
"""stop_game returns True when game is playing."""
161-
self.skill._playing.set()
166+
self.skill._playing_sessions.add(self.skill.get_session_id())
162167
result = self.skill.stop_game()
163168
self.assertTrue(result)
164169

165170
def test_stop_game_clears_playing(self) -> None:
166-
"""stop_game clears the _playing event."""
167-
self.skill._playing.set()
171+
"""stop_game clears the per-session playing state."""
172+
self.skill._playing_sessions.add(self.skill.get_session_id())
168173
self.skill.stop_game()
169174
self.assertFalse(self.skill.is_playing)
170175

0 commit comments

Comments
 (0)