Added a Llama Picture and Llama Facts Command #196
Conversation
Zycrasion
changed the title
Zycrasion
changed the title
lib/Synergy/Reactor/CatPic.pm
Outdated
| help_titles => [ 'llama pic' ], | ||
| help => '*llama pic*: get a picture of a llama', | ||
| matcher => sub ($text, @) { | ||
| return unless $text =~ /\Allama\spic?\z/i; |
There was a problem hiding this comment.
The regex here is wrong. The version of this in catpic matches "cat" followed optionally by "pic", "jpg", "gif", or "png". And then returns the optional part if it exists.
Regex can be a bit cryptic, but it's really just a very comprehensive pattern matching language, the ? character (the one at the end anyway) marks the previous entity as optional. In catpic this applies to the entire optional match section of space followed by one of the words, in this match the precious entity is the "c" of pic, so this will match either "llama pic" or "llama pi"
There was a problem hiding this comment.
Oh yeah, thanks for pointing that out. I think I have fixed it.
| $event->mark_handled; | ||
|
|
||
| my $http_request = $self->hub->http_client->GET( | ||
| "https://llama-as-a-service.vercel.app/llama_url" |
There was a problem hiding this comment.
Did you create this API just for this task? cool!
lib/Synergy/Reactor/CatPic.pm
Outdated
| }, sub ($self, $event) { | ||
| $event->mark_handled; | ||
|
|
||
| my $http_request = $self->hub->http_client->GET( |
There was a problem hiding this comment.
I would have kept the $http_future naming from the cat_pic responder here, makes it clearer that it's not just a simple client object and keeps it consistent with cat pic.
lib/Synergy/Reactor/CatPic.pm
Outdated
| @@ -191,6 +191,32 @@ sub _build_reactions ($self, @) { | |||
| return $reactions; | |||
| } | |||
|
|
|||
| # Copied cat_pic code | |||
| # Because there was no documentation | |||
| # Works on terminal, dunno if slack wil be able to display it as an image | |||
There was a problem hiding this comment.
cat pic works by assuming slack will recognise the url and fetch the target for display, so assuming what is returned is a url it should work just fine
lib/Synergy/Reactor/CatPic.pm
Outdated
| ); | ||
|
|
||
| return $http_request->on_done(sub($res) | ||
| { |
There was a problem hiding this comment.
This needs some error handling, what happens if we don't get a 200 from the remote api call? What happens if the remote API stops returning a single URL to a picture and starts returning a large html page?
lib/Synergy/Reactor/CatPic.pm
Outdated
| # Tried to test it on local slack, but the documentation was pretty bad with the config files | ||
| responder llama_pic => { | ||
| exclusive => 1, # No idea what this is. | ||
| targeted => 1, # Or this |
There was a problem hiding this comment.
exclusive is a means to prevent a string being matched by multiple responders, targeted is to do with if you need to target the bot with @botName or PM etc. Someone who has worked on synergy before could answer those questions, but copying what is in catpic is fine here.
|
i think i fixed everything |
lib/Synergy/Reactor/CatPic.pm
Outdated
|
|
||
| return $http_request->on_done(sub($res) | ||
| { | ||
| $event->reply($res->content) |
lib/Synergy/Reactor/CatPic.pm
Outdated
| return $http_future->on_done(sub($res) | ||
| { | ||
| if ($res->code == 200) | ||
| { |
There was a problem hiding this comment.
just a small nit pick, the new code uses a different brace style to the rest of the code.
opening brace on the same line as the related keyword, one space before brace
|
All brackets Should Be Fixed now |
rjbs
force-pushed
the
main
branch
4 times, most recently
from
f23a1b1
to
4022b0d
Compare
rjbs
force-pushed
the
main
branch
9 times, most recently
from
32f29ba
to
39ade23
Compare
rjbs
force-pushed
the
main
branch
4 times, most recently
from
aafd26b
to
eee1d40
Compare
Uses a webservice to get random llama picture.
Llama webservice written by me
also added a llama facts command