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

Fixes to get the Bot working and then some more... #26

Closed
wants to merge 31 commits into from

Conversation

ElvenSpellmaker
Copy link
Collaborator

I forked your code and tried to run the bot but it complained about passing arguments to commands/listeners that didn't have constructors, so I have made a fix for it which attempts to use arguments and if that fails then it tries to construct without arguments.
If the config. file has arguments but the command/listener doesn't accept them then it will warn the user that the command doesn't accept the supplied arguments.

I also fixed the keywors mistake to keywords because it failed to start due to it being misspelt.
I then noticed that FutileFreedom also fixed the second bug but you haven't committed his pull request yet (I cloned yours and it still had the error), found here: #25

Thanks. =)

@matejvelikonja
Copy link
Collaborator

Good job. I'll take a look at commit when I'll have time and let you know how is it. For merging, we'll have to wait for @pogosheep.

…ng too "trustworthy" and allowing commands to get the nick/username/hostname

of the sender.

An attempt to remove colons from the beginning of fields that don't need them such that Listeners don't have to manually remove them.

Also implemented a serialise and remember (un-serialise) structure so that Commands and Listeners may save their state and recall this state later.
This needs further expansion so that

Expansion ideas:
Master recognition system such that serialise/remember/(quit/timeout/restart) can't be abused.
Command/Listener enable/disable system.
@ElvenSpellmaker
Copy link
Collaborator Author

I've just pushed up more adds some functionality to the bot that it didn't have as well as fixing an issue mentioned here: #16
I patched that by making it ignore PRIVMSG messages for server commands which is a good enough patch imo..

Also added is serialisation and remembering (un-serialisation) which for now is a manual process (initiated by !serialise and !remember assuming those plug-ins are enabled, hence the new config. file).

One thing I noticed is that if the bot is messaged personally it responds to itself rather than to the user who messaged them.
The say method in Bot will need to be fixed to respond back to the right person.

@matejvelikonja
Copy link
Collaborator

Hey, could you use spaces for indentation instead of tabs. Code looks a bit hard to read if we mix that two together.

Could you explain a bit more about this two new commands?

@ElvenSpellmaker
Copy link
Collaborator Author

Sorry, I would've though github would convert tabs to spaces automatically. (I'm a bit new to all this)
I'll change it now.

The serialisation function serves as a way for Commands and Listeners to store data to a file, or database on demand so that it may be recalled later into the memory of the bot.
If you wanted to patch your bot but not lose working data that a command or listener may use, it might be nice to write it to a file/db. Several bots I have seen implement this type of manual functionality, rather than thrashing a file or db after every command/event.
My code provides a way of calling all Commands and Listeners and if they implement a serialise() or remember() method then it will call them when the user uses !serialise or !remember respectively.

If you write a new Command or Listener, you only need to implement serialise and remember to have this store/load functionality.

Probably would be wise for the Bot class to have a field which is set by the config. file for a directory to store files into.

(Also I fixed the bot messaging itself instead of the user who messaged it which I mentioned previously).

@matejvelikonja
Copy link
Collaborator

@ElvenSpellmaker i just did a quick review and it looks great. It would be awesome now that we would have a command or listener to use this.
do you have an idea for a command or listener to use this?

…s for me as

I don't know how to set-up a channel with a password.

I need to update the README soon as well seeing as a *lot* has been changed since I forked.
Instead of just a plain array of channels, like "#PHPTest" you now have a hash:

"#PHPTest => '', #cs => 'foobarbaz'"

PHPTest is a channel without a password and cs is a channel with the password foobarbaz.
Consider to alleviate potential JOIN spam, joining all JOIN messages into one join message:

e.g.

JOIN #PHPTest
JOIN #cs foobarbaz
|
V
JOIN #PHPtest,#cs ,foobarbaz
It also switches the NICK and USER commands around as you are *meant* to send NICK before USER.
@ElvenSpellmaker
Copy link
Collaborator Author

I have done a few more commits today to try and make the bot handle passwords for both servers and channels, as described in these reports:
#20
#28

If people can validate they work I'd be very grateful!

I haven't implemented nickname passwords yet. (Such as messaging nickserv), I may attempt this later.

Hopefully this time it works...
…ore generic one.

Check for "End of /MOTD command" instead of "Welcome" as not every MOTD has Welcome in it!
@ElvenSpellmaker
Copy link
Collaborator Author

(I have no idea how I have closed the pull request twice...)

Update: Changed a naïve way of looking if you are welcome, I assume if you can see the MOTD then you are welcome, probably isn't always the case, but it's better than looking for "Welcome".
(If you look at the log in #28 then you can see "Welcome" isn't in the MOTD and so the bot just hangs.

@ElvenSpellmaker
Copy link
Collaborator Author

@matejvelikonja Do you have write access to Pogosheep's repo?

If so, can't you merge my changes?

@matejvelikonja
Copy link
Collaborator

yes, I have it now. @pogosheep doesn't have enough time to do this, so he gave me permissions.

will merge this as soon as I test it and figure out how we will proceed with development. lot's of developers started to contribute, and we will have to make some rules about that. like start writing tests, code standards etc. we have to write maintainable and well tested code.

i also have to set up irc channel, so we can stay in touch easier.

@matejvelikonja
Copy link
Collaborator

@ElvenSpellmaker can you merge current version of pogosheep code to your one? otherwise i cant accept PR.

@ElvenSpellmaker
Copy link
Collaborator Author

I will when I get back home in a couple of hours.

@ElvenSpellmaker
Copy link
Collaborator Author

I will also hopefully upload the few plugins I have written and will update the readme.

@ElvenSpellmaker
Copy link
Collaborator Author

Also where is the IRC channel so I can join please?

@matejvelikonja
Copy link
Collaborator

look at the config file :)

ElvenSpellmaker and others added 19 commits March 13, 2013 19:53
Merging with pogo's code.

Conflicts:
	Classes/Library/IRC/Bot.php
… bit anyway).

Changed a few more things, allows commands to send to private messages and other channels,
plus a few more bugfixes.
Added several methods to bot.php: getListeners(), getListener() and
getCommand()

Change-Id: I1179281401cdaa2a4dcc50e647ec8462219bd265
Added common code to a super class
classes' method

Change-Id: I6f934eb4499fa5ba3710f64e3eb1b36bfcf64d25
removed say() method in Listener Base
Change-Id: I5e96b72cc8ac3a714ab728c1c362963000919d79
…ngPogosCode

Merging Hosh's changes.

Conflicts:
	Classes/Library/IRC/Base.php
	Classes/Library/IRC/Bot.php
… the process so I've merged here.

A few other things too.

Note: The new structure still needs some more work, Commands and Listeners now have far too many ways of accessing the same information.
@ElvenSpellmaker
Copy link
Collaborator Author

Seeing as this has many, many merge conflicts and is horrendously old I will close this.

I am writing a new version of all these changes over on my fork if anyone would like to check that out.

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