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

Something very strange happened #178

Closed
Kuchiriel opened this issue Oct 6, 2016 · 2 comments · Fixed by #181
Closed

Something very strange happened #178

Kuchiriel opened this issue Oct 6, 2016 · 2 comments · Fixed by #181

Comments

@Kuchiriel
Copy link

Kuchiriel commented Oct 6, 2016

I just followed that example:

 > begin
  // Change the reply formatting based on the bot's mood
  + request
  * <bot mood> == happy => {sentence}{ok}{/sentence}
  * <bot mood> == angry => {uppercase}{ok}{/uppercase}
  * <bot mood> == sad   => {lowercase}{ok}{/lowercase}...
  - {ok}
< begin

But when the bot get "angry", he start to repeat everything I say with strange things around it.

[You]: hello
[Bot]:  {__CALL__} KIA {__CALL_ARG__}HELLO{/__CALL_ARG__}{/__CALL__}

I'm using a async Bot.

This only happens in the web js interpreter, with node, everything happens normally.

@kirsle kirsle added this to the v1.17.0 milestone Oct 6, 2016
@kirsle
Copy link
Member

kirsle commented Oct 6, 2016

That's interesting and I think I know exactly why.

In #78 (and related fixes in #83), the <call> tag was made to support running asynchronously by returning a promise. This changed the way the <call> tags are processed to work in two different steps and caused a couple other problems such as #111, because at the end of the processTags function there's still left over {__call__} and {__call_arg__} tags that had to be postponed until the very last minute (when their object macro promises resolve) before finally substituting the result in their place.

Because these "temporary" {__call__} type tags are left behind until the very last minute, the "begin block" ends up processing the {uppercase}...{/uppercase} tag surrounding the "reply" from the bot and it ends up capitalizing the tags like {__CALL__}. When the object macro finally resolves (after the uppercasing had already been done), it can't find its temporary tags to replace them because it's doing a case sensitive search, so the result from the macro ends up getting discarded and you get the leftover {__CALL__} etc. tags.

This only happens in the web js interpreter, with node, everything happens normally.

I'd expect it to be reproduceable on both environments, are they running the same versions of rivescript-js?


The fix for this will probably be to fix these regexps to be case insensitive (it looks from the source that the {__call_arg__} already is, but not the {__call__}).

However, the underlying thing the code was trying to do in the first place wouldn't work with the output of the object macro. The uppercasing would be done before the object macro resolves and is substituted into the reply, so the actual text returned by the macro would still be its original case (any other text in the reply outside the <call> tag, though, would be uppercased). It might also wreak havoc on the arguments to the macro, as the args would all be made uppercase before being sent into the object macro. 😉

That example RiveScript code works as expected on all the other versions, and on JS without replyAsync(). RiveScript was originally implemented in Perl (no async), and ported to Java and Python (no async) before JavaScript, where it also had no async support originally until #78, so adding async support to it came with a whole list of caveats like this.

I'll get this one fixed for v1.17.0

@Kuchiriel
Copy link
Author

Kuchiriel commented Oct 6, 2016

Thanks for the detailed explanation, I really appreciate your attention and commitment with RiveScript.

Yeah all that async world is kinda strange for me.

I'll wait for the v1.17.0, by now I'll just think another way to make bot appear more angry. XD

I'd expect it to be reproduceable on both environments, are they running the same versions of rivescript-js?

That one is Node
Loading RiveScript Module Version 1.16.0

That one is Running from the Web
`Engine: Loaded RiveScript Version 1.16.0``

As you can see, same version, but that one in Node isn't async.

I forgot to say a thing, I communicate my async web interpreter with a node interpreter using rest, and I pair the variables to keep things working, also I got a file for every username, using one of the persistance examples of the project.

Basicly I have only the necessary and minimal replies in the web end, and a wildcard will call a reply from the node server everytime user types or say something that do not include in this minimal web set. In that server there is more complicated triggers that feed web end, and other interactions vectors.

Because of that nature I had to use async and deal with promises, I don't know if that kind of error occurs only if the bot is async, or because of something more. The fact is that I use in the web something like:

+ *
- <call>kia <star></call>
rs.setSubroutine('kia', function(rs, args) {

console.log("Engine: Match not found on client, sending to the backend.")

var data = {
username: username,
message: args.join(" "),
vars: JSON.stringify(rs.getUservars(username))
}
var error = () => {
console.log(data)
}
  return new rs.Promise(function(resolve, reject) { 
    $.ajax({
    type: "POST",
    url: "/bot",
    data: data,
    success: function(response) {
        rs.setUservars(username, response.vars)
        resolve(response.reply)
    },
    error: error
    })
    });
})

Keep up the awesome work!

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

Successfully merging a pull request may close this issue.

2 participants