-
Notifications
You must be signed in to change notification settings - Fork 72
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
Event callbacks and example bot #27
base: master
Are you sure you want to change the base?
Conversation
|
||
def redirect(self, user, target): | ||
"""Redirect within current topic. Returns reply from the redirected trigger.""" | ||
return self._getreply(user, target) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this function here? To skip the processing of the >begin
block? It doesn't seem to be called anywhere else internally except the unit tests. When redirects are actually processed it still goes straight for self._getreply()
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The main use case for this whole PR is being able to fully restore engine state, including setting last visited topic and replaying the last redirect if necessary. I'm actually using this in my bots. I think it's useful to add explicit API methods for setting topics and doing redirects.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you update the documentation for this function and clarify its use case? I don't want users to accidentally start using this instead of reply()
when they find out it kinda works the same way.
For example, things to highlight in the documentation:
- The
target
parameter assumes that all message processing has already been done (e.g. substitutions, no punctuation, etc) - or just clarify that it must be formatted the same as how an@redirect
would be in RiveScript source (also, maybe passing thetarget
throughcheck_syntax("@", target)
internally would be a good idea to save users from themselves). - The begin block is not processed.
- User input/reply history is not modified.
current_user()
is not set nor cleared.
Or basically drive home the point that this function is for specific use cases and that most people probably don't need to worry about it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the use case for the topic callback? Under the hood, topics are just user variables (e.g. |
It's handy for tracking engine state (e.g. for persistence) - I attached an example bot. We might want to persist each topic change and in-topic redirects to be able to fully restore the session later. Of course you can get and store all engine state (vars, topic, etc.) after each reply, but I think the callback-based approach is much cleaner -- topic changes don't occur on every reply. My first implementation of this behavior was via function calls, but it made the script a total mess and it was only time consuming and error-prone (adding a |
from collections import deque | ||
|
||
# Configure readline all interactive prompts | ||
import readline |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The readline
module doesn't seem to be available on Win32. There is pyreadline which implements readline in pure Python which can work for Windows users.
This could maybe be replaced with some try/except logic to try the built-in readline
first, then pyreadline
for Windows, then give an error telling Windows users to pip install pyreadline
(so that it doesn't have to go in the requirements.txt for users who don't need it).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Event callbacks
Enabling event-based handling of session state changes, instead of getting them and comparing after each reply.
Example bot - managing user session data using event callbacks
More info in
eg/sessions/example.py
.NOTE: Maybe this could be added as a feature to RiveScript, but I've kept it separate for now as I'm not sure what's the status/roadmap for current multi-user functionality. I'm not using it in my bots which operate in multithreaded environment. I'm just keeping one RS instance per user and managing sessions outside. I think session management should be abstracted away and independent of scripting.
Additional public functions
get_topic(user)
,set_topic(user, topic)
,redirect(user, target)
Other changes
rivescript.py
so that all interactive prompts (includingintaractive.py
) are more terminal-friendlycollections.deque