superscriptjs / superscript

A dialogue engine for creating chat bots
http://superscriptjs.com
MIT License
1.65k stars 209 forks source link

Filter functions not working #316

Closed SephVelut closed 7 years ago

SephVelut commented 7 years ago

On branch v1.0.0

SS:FilterFunction Reply filter function found: ^sigh("true") +19ms
  SS:Utils Running plugin function with name: sigh +1ms
  SS:Utils Calling plugin function: sigh with args: true,function (err, filterReply) {
                if (err) {
                  console.error(err);
                  return resolve(false);
                }

                if (filterReply === 'true' || filterReply === true) {
                  return resolve(true);
                }
                return resolve(false);
              } +8ms
 SS:FilterFunction Reply filter function found: ^sigh("false") +1ms
  SS:Utils Running plugin function with name: sigh +0ms
  SS:Utils Calling plugin function: sigh with args: false,function (err, filterReply) {
                if (err) {
                  console.error(err);
                  return resolve(false);
                }

                if (filterReply === 'true' || filterReply === true) {
                  return resolve(true);
                }
                return resolve(false);
              } +1ms
  SS:FilterSeen filterRepliesBySeen +1ms []
  SS:GetReply Bucket of selected replies:  +0ms []
  SS:GetReply Pick Scheme: +1ms random
  SS:GetReply Set of matches:  +2ms
  SS:GetReply afterHandle +5ms { replyId: null,
  replyIds: null,
  props: {},
  clearConversation: false,
  topicName: null,
  debug: [],
  string: '',
  subReplies: [],
  stars: null,
  continueMatching: null }
  SS:Message Message received was empty, callback immediately +0ms
Could not write log to file with path: /home/sephvelut/app/bot/logs/ffff127001_trans.txt.log
  SS:User Updating History +2ms
  SS:SuperScript Update and Reply to user '::ffff:127.0.0.1' +0ms
  SS:SuperScript [ Final Reply - '::ffff:127.0.0.1']- '' +1ms
+ test
- ok

+ qq
- {^sigh("true")} asd
- {^sigh("false")} poi
exports.sigh = function(bool, cb) {
        cb(null, false);
};

The filter function does get fired off, but the bot never replies with anything.

bensalilijames commented 7 years ago

Yeah, that script should work. Out of interest are you using the source or the published npm 1.0.0-alphax version?

Also is it possible that the reply gets exhausted because it doesn't have the {keep} flag on, or does this happen when you say qq as the first trigger after start up?

silentrob commented 7 years ago

Interesting, we have a test for this and It seems to still be passing (in V1.0) Im wondering if the polarity of the boolean is inverted and making it hard to grok.

bensalilijames commented 7 years ago

Er, I didn't look closely enough.

exports.sigh = function(bool, cb) {
  cb(null, false);
};

This always returns false so it'll always be filtered out. (I think? @silentrob)

silentrob commented 7 years ago
+ my name is <name>
- {^hasName("false")} ^save("name",<cap1>) Nice to meet you, <cap1>.
- {^hasName("true")} I know, you already told me your name.
const hasName = function hasName(bool, cb) {
  this.user.getVar('name', (e, name) => {
    if (name !== null) {
      cb(null, (bool === 'true'));
    } else {
      // We have no name
      cb(null, (bool === 'false'));
    }
  });
};
SephVelut commented 7 years ago

I installed "superscript": "alpha" on npm. I have the custom function using false deliberately. I just want to see a reply true or false, but I'm not seeing anything. I'm using the telnet client and loading the plugins directory with pluginsPath set in options.

bensalilijames commented 7 years ago

I think the custom function should return true if you want to see the reply, and false if you want to filter it out, though I may be going crazy. Maybe it should be the other way around?

SephVelut commented 7 years ago

Okay I tried with true and it replies. I always thought filters worked liked a conditional with true and false alternate replies? At least that was the previous behavior. How would I reply or call another custom function on a false a condition?

silentrob commented 7 years ago

I do agree that is miss leading.. by definition filter functions should be filter based on a true condition / value.

bensalilijames commented 7 years ago

Just for clarity: in the plugin function that you write as a filter, you'll want to either return true if you want the reply to be used (not filtered), or false if you want it to not be used (filtered).

const myPlugin = myPlugin(some, args, here, callback) {
  if (someCondition) {
    // filter the reply
    return callback(null, false)
  }
  return callback(null, true)
}
SephVelut commented 7 years ago

I agree the filter functionality should pass only on true. Giving false to the callback isn't necessary in that case; the filter will only pass on true.

So that leaves conditional replies. Anyway to make this happen?

+ *
- {condition1} a
- {condition2} b
- {condition3} ^saveSomething(<cap1>)
- {condition4} ^doSomething() c
bensalilijames commented 7 years ago

You could either create four different plugin functions, or if they share common functionality, one plugin function that accepts different arguments:

+ *
- {^myNameIsBen()} a
- {^myNameIsRob()} b
- {^myNameIsAlice()} ^saveSomething(<cap1>)
- {^myNameIsDavid()} ^doSomething() c

or (preferably):

+ *
- {^myName("Ben")} a
- {^myName("Rob")} b
- {^myName("Alice")} ^saveSomething(<cap1>)
- {^myName("David")} ^doSomething() c
SephVelut commented 7 years ago

The second, single function is preferable but consider if I want to {^myName(<cap1>)} because I don't know what the input will be. Wouldn't filters breakdown at this point for conditionals?

+ *
- {^myName(<cap1>)} a
- {^myName(<cap1>)} b
- {^myName(<cap1>)} ^saveSomething(<cap1>)
- {^myName(<cap1>)} ^doSomething() c

Wouldn't work. You would have to use separate functions, which is less than ideal

{^myNameIsBen(<cap1>)}
{^myNameIsRob(<cap1>)}
{^myNameIsDingo(<cap1>)}
silentrob commented 7 years ago

We have processed the trigger and have all the captured input, so passing <cap1> in should work.

SephVelut commented 7 years ago

What I mean is

+ *
- {^myName(<cap1>)} a
- {^myName(<cap1>)} b
- {^myName(<cap1>)} ^saveSomething(<cap1>)
- {^myName(<cap1>)} ^doSomething() c

which filter gets to pass? What I would really like to do is

+ *
- {<cap1> == "Rob"} a
- {<cap1> == "Ben"} b
- {<cap1> == "Jake"} c

I suppose you could

+ *
- {^myNameIs(<cap1>, "Rob")} a
- {^myNameIs(<cap1>, "Ben")} b
- {^myNameIs(<cap1>, "Jake")} c

But it feels like it could be more 'built in'. Thanks for the help btw.

silentrob commented 7 years ago

For the sake of completion, we do have another system that might work.. but it is for managing conversation state/flow.

+ __start__
- match here {id=123, bool=true,   str="string"  }

%% (bool == true)
  + boo ya
  - YES

%% (id == 123)
- winning
bensalilijames commented 7 years ago

Yeah, that last one is probably what I'd do right now! :)

We could introduce some shorthand for simple comparisons like the second example. Probably lower down the priority list though.