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

Support promise returns #43

Closed
ocordeiro opened this issue Sep 15, 2015 · 6 comments
Closed

Support promise returns #43

ocordeiro opened this issue Sep 15, 2015 · 6 comments

Comments

@ocordeiro
Copy link

Hello, this update make rive script support promises
Ex.:

+ test promise
- <call>promise</call>

> object promise javascript
    var q = require("q");
    var def = q.defer();
    setTimeout(function(){
     def.resolve('Promise');
    }, 1000);
    return def.promise;
< object 

brain.js
https://gist.github.com/alamcordeiro/47d81412941ef3be8be0

New Brain.prototype.asyncCall function

@kirsle
Copy link
Member

kirsle commented Sep 16, 2015

I made a diff of your gist to visualize the change necessary for this. Key places in your gist where the code is changed:

  • In reply() here
  • Remove <call> processing from processTags() here
  • And move the <call> processing to the new asyncCall() function here

What is the use case for this? Can you get similar functionality in an out-of-band way? (i.e. implement promises in your app code that wraps around the call to reply()?) Objects are designed to return verbatim text, which is inserted in the reply in place of the <call> tag.

Another idea is to make a custom language handler (see rivescript/lang/javascript.js here for an example), where you would write > object test promise in your RS code and it returns a promise rather than a text reply.

I'd be reluctant to implement this change because it uses a feature specific to a programming language (promises to JavaScript) which aren't equally available in other languages. See RiveScript Goals and Scope. However, if you create a pull request that implements this change in a way that it is backwards compatible (and not active by default, but can be used by someone who wants it) then I'll accept it.

For example: if an object macro returns a text type object, substitute the text into the original reply like before. If it returns a promise object, then resolve the promise. The RiveScript library should continue to be usable with no changes for existing users of the library who don't use or need promises. If promises can't be implemented in a way that still works as before, make a separate function call instead of reply(), like:

rs.replyAsync(user, msg).then(function(reply){
    ...
});
--- lib/brain.js    2015-09-15 16:50:19.854015626 -0700
+++ brain-promise.js    2015-09-15 16:50:15.396005396 -0700
@@ -1,6 +1,5 @@
 "use strict";
 var Brain, inherit_utils, utils;
-
 utils = require("./utils");

 inherit_utils = require("./inheritance");
@@ -22,7 +21,8 @@
   };

   Brain.prototype.reply = function(user, msg, scope) {
-    var begin, reply;
+    var self, begin, reply;
+    self = this;
     this.say("Asked to reply to [" + user + "] " + msg);
     this._currentUser = user;
     msg = this.formatMessage(msg);
@@ -38,12 +38,17 @@
     } else {
       reply = this._getReply(user, msg, "normal", 0, scope);
     }
-    this.master._users[user].__history__.input.pop();
-    this.master._users[user].__history__.input.unshift(msg);
-    this.master._users[user].__history__.reply.pop();
-    this.master._users[user].__history__.reply.unshift(reply);
-    this._currentUser = void 0;
-    return reply;
+    reply = this.asyncCall(reply, scope, 0);
+    return new Promise(function(resolve) {
+      reply.then(function(reply){
+        self.master._users[user].__history__.input.pop();
+        self.master._users[user].__history__.input.unshift(msg);
+        self.master._users[user].__history__.reply.pop();
+        self.master._users[user].__history__.reply.unshift(reply);
+        self._currentUser = void 0;
+        resolve(reply);
+      });
+    });
   };

   Brain.prototype._getReply = function(user, msg, context, step, scope) {
@@ -601,33 +606,46 @@
     }
     reply = reply.replace(/\{__call__\}/g, "<call>");
     reply = reply.replace(/\{\/__call__\}/g, "</call>");
-    match = reply.match(/<call>(.+?)<\/call>/i);
-    giveup = 0;
-    while (match) {
+    return reply;
+  };
+
+  Brain.prototype.asyncCall = function(reply, scope, giveup) {
+    var self, match, text, parts, obj, args, output, lang;
+    self = this;
       giveup++;
+    return new Promise(function(resolve) {
       if (giveup >= 50) {
-        this.warn("Infinite loop looking for call tag!");
-        break;
+        self.warn("Infinite loop looking for call tag!");
+        resolve(reply);
+        return;
+      }
+      reply = reply.replace(/\{__call__\}/g, "<call>");
+      reply = reply.replace(/\{\/__call__\}/g, "</call>");
+      match = reply.match(/<call>(.+?)<\/call>/i);
+      if(!match){
+        resolve(reply);
       }
       text = utils.strip(match[1]);
       parts = text.split(/\s+/);
       obj = parts.shift();
       args = parts;
-      output = "";
-      if (this.master._objlangs[obj]) {
-        lang = this.master._objlangs[obj];
-        if (this.master._handlers[lang]) {
-          output = this.master._handlers[lang].call(this.master, obj, args, scope);
+      if (self.master._objlangs[obj]) {
+        lang = self.master._objlangs[obj];
+        if (self.master._handlers[lang]) {
+          output = self.master._handlers[lang].call(self.master, obj, args, scope);
         } else {
-          output = "[ERR: No Object Handler]";
+          resolve("[ERR: No Object Handler]");
+          return;
         }
       } else {
-        output = "[ERR: Object Not Found]";
+        resolve("[ERR: Object Not Found]");
+        return;
       }
-      reply = reply.replace(new RegExp("<call>" + utils.quotemeta(match[1]) + "</call>", "i"), output);
-      match = reply.match(/<call>(.+?)<\/call>/i);
-    }
-    return reply;
+      Promise.resolve(output).then(function(cb) {
+        reply = reply.replace(new RegExp("<call>" + utils.quotemeta(match[1]) + "</call>", "i"), cb);
+        resolve(self.asyncCall(reply, scope, giveup));
+      });
+    });
   };

   Brain.prototype.substitute = function(msg, type) {

@ocordeiro
Copy link
Author

Thank you very much for the explanation.
Its definately better to use other method called "replyAsync",it also ensures backward compatibility.
I will see a better way to implement this...

@kirsle
Copy link
Member

kirsle commented Sep 16, 2015

I added an example for how a RiveScript bot could send an asynchronous message from an object macro, without using promises. It's in the git repo now at the eg/examples/async-object directory.

Link: https://github.com/aichaos/rivescript-js/tree/master/eg/async-object

The example uses setTimeout() as its asynchronous call but any async node function should work.

@dcsan
Copy link
Contributor

dcsan commented Sep 16, 2015

that format is very helpful with examples in the directory...

@ocordeiro
Copy link
Author

Very good. I had not thought of that :D
What if I define a parameter called "async" in configurations to enable return promise support?

var bot = new RiveScript({async: true});

Using async:true method "reply()" return a promise
Using async:false or async is undefined. method reply() return a string (reply)
It also ensures backward compatibility.

@ocordeiro
Copy link
Author

"Why I am switching to promises"
http://spion.github.io/posts/why-i-am-switching-to-promises.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants