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

Shell: Add option to watch/reload the brain #160

Merged
merged 9 commits into from
Sep 6, 2016
Merged

Conversation

julien-c
Copy link
Contributor

@julien-c julien-c commented Aug 30, 2016

@kirsle If you like and merge this I will make the same changes to shell.coffee

@kirsle
Copy link
Member

kirsle commented Aug 30, 2016

Does this do what you think? Try doing something like JSON.Stringify(rs, null, 2) and check the data tree, there might be duplicate triggers when you reload the bot.

A long time ago the RiveScript modules arranged data in a hierarchical object tree, like rs.topics["random"].triggers["hello bot"], so that when you reloaded the same replies again, the duplicates would overwrite their old locations in the data structure and things would just work. But now RiveScript loads the triggers into a giant array, and uses pointers into that array for other purposes (e.g. sorting the replies), but if you reload the bot brain I'd expect it to append the same set of RiveScript data to the bottom of the array and creating duplicates, and e.g. if you were to modify an existing trigger to have a different response, you might not actually get that response because the trigger was a duplicate and the original one would match instead of the new one, giving the old response.

The only good/fast way I can think to fully reload a bot that works this way would be to first clear out all the data and reparse the code from scratch.

@julien-c
Copy link
Contributor Author

@kirsle Your last proposition is actually what I'm doing. In the loadBot function I'm instantiating a whole new RiveScript object and calling loadDirectory on it.

This was following my question (and your answer) here: #156

@julien-c
Copy link
Contributor Author

PS: I've merged master back into this branch to keep it up-to-date using Github's UI, but I can properly rebase the pull request if you want.

@julien-c
Copy link
Contributor Author

julien-c commented Sep 2, 2016

@kirsle I've pushed a few more tweaks. Let me know how you feel about this. Thanks!

fs.watch(opts.brain, {recursive: false}, function() {
console.log("");
console.log('[INFO] Brain changed, reloading bot.');
rl.prompt();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this to require the user to press enter? Is it necessary? I think most watcher things just notify of the reload and continue.

Copy link
Contributor Author

@julien-c julien-c Sep 2, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope it's just to display the > input sign again in the terminal's last line. Necessary as we've been writing to the console.log. No user action needed of any sort.

@kirsle kirsle merged commit 8f297d3 into aichaos:master Sep 6, 2016
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

Successfully merging this pull request may close these issues.

2 participants