[v1.0.0] conversations script does not work as expected

hailiang-wang commented 7 years ago

Such a conversations script does not work.

> topic:keep preview_words (preview)
+ __preview
- {@__preview_question_kickoff}

    + ~yes
    % __preview_question_kickoff
    - Great, let's play! 

    + ~no
    % __preview_question_kickoff
    - No? Alright, let's play a differnt game! 

    + (*)
    % __preview_question_kickoff
    - OK, let's play! 

+ __preview_question_kickoff
- Do you want to play word games? 
- Let's play word games

< topic

When running in bot instance, SS:GetReply prints out -

[2016-11-16 23:44:26.117] [DEBUG] chain.engine - channel mobileapp fromUserId 54731 fromUsername n123
[2016-11-16 23:44:26.126] [INFO] chain.engine - ssbot receive << mobileapp 54731 @User# a20kG8c91U said: yes
[2016-11-16 23:44:26.127] [DEBUG] brain.engine - reply userId a20kG8c91U message yes
  SS:GetReply Found pending topics/conversations: [{"id":"582c7ec5d827b4420c260cb7","type":"REPLY"},{"name":"preview_words","type":"TOPIC","id":"582c7ec4d827b
4420c260c7a"},{"name":"preview_words","score":0,"type":"TOPIC","id":"582c7ec4d827b4420c260c7a"},{"name":"sayhello","score":0,"type":"TOPIC","id":"582c7ec4d827
b4420c260c6a"}] +17s
  SS:GetReply Conversation reply thread:  { _id: 582c7ec5d827b4420c260cb7,
  reply: 'You\'ll love this game, want to play? Yes?',
  parent: '582c7ec5d827b4420c260cb4',
  __v: 0,
  gambits: [],
  filter: '',
  keep: false,
  id: 'UNsBdn5b' } +2ms
  SS:GetReply Matching gambits are:  +92ms
  SS:GetReply Continue searching: null +1ms
  SS:GetReply Set of matches:  +0ms
  SS:GetReply afterHandle { replyId: null,
  props: {},
  clearConversation: false,
  topicName: null,
  minMatchSet: [],
  string: '',
  subReplies: [],
  stars: null,
  continueMatching: null } +0ms
[2016-11-16 23:44:26.267] [INFO] chain.engine - ssbot reply   >> mobileapp 54731 @User# a20kG8c91U said:
[2016-11-16 23:44:26.269] [DEBUG] io.engine - _createMessageOutbound output {"type":"textMessage","textMessage":"@message_incomprehension@","isSendMessageOutb
ound":true}
[2016-11-16 23:44:26.290] [DEBUG] io.engine - _createMessageOutbound id@ FH1VoAdWVA

Expected Behavior

Current Behavior

Possible Solution

Steps to Reproduce (for bugs)

1. 2. 3. 4.

Context

Your Environment

Version used:
Environment name and version (e.g. PHP 5.4 on nginx 1.9.1):
Server type and version:
Operating System and version:
Link to your project:

hailiang-wang commented 7 years ago

After checking the database, I think it is because even in the GetReply, it hits a reply/conversation thread. But in this reply object, it gets no gambits, it should be something wrong with the ss-parser or importData.

hailiang-wang commented 7 years ago

After checking data.json, the conversation is not built into data.json.

Due to some reason, the ~yes, ~no, *() are not mapped with filter __preview_question_kickoff**. Actually, their gambit has none filter.

nsaraiya commented 7 years ago

https://github.com/superscriptjs/superscript/wiki/Conversations mentions:

Please note that:

In conversations like this, we always try to match forward, but once there is no more matches we don't walk back up the tree, we reset back to the Random Topic. Also, * in conversation threads have the same weight as topic level * wildcards.

So can you try to define

+ __preview_question_kickoff
- Do you want to play word games?
- Let's play word games at the top of your ss file?

Just a thought - the position might be causing a problem for you.

bensalilijames commented 7 years ago

There’s a couple of things going on here:

1) Before v1, you would have been able to do this:

+ Hi there
- Goodbye cruel world

+ I am happy
% Goodbye
- Alright

So despite the fact that Goodbye should really be Goodbye *, it will still match as part of a conversation. My thinking was that if you need a star, you should really be explicit about this. So the breaking change made was the match the entirety of the string, from start to finish.

Now, goodbye no longer matches goodbye cruel world unless you specifically put a star in.

In your example, this means that:

% __preview_question_kickoff

Should become

% {@__preview_question_kickoff}

So it exactly matches the reply.

2) There’s an issue itself with matching conversation threads. There’s a bug in the new processTags logic which, upon seeing a redirect of some kind, overrides the root ID with the new reply. Commenting out that line fixes this case, but breaks others, so I’ll be doing a little bit of work to fix this in the next couple days.

bensalilijames commented 7 years ago

This should now work. It now tracks all the matched IDs in a reply instead of just the last one, so all the relevant conversations are checked, even if they're part of a redirect of some kind. Cheers!

hailiang-wang commented 7 years ago

👍

superscriptjs / superscript