432165 - mozbot does not automatically make itself a bot.

Status: RESOLVED FIXED

(Webtools Graveyard :: Mozbot, defect, P1)

Description

[Darren VanBuren]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9pre) Gecko/2008043016 Firefox/3.0pre
Build Identifier: CVS

After Mozbot connects to the server, it should run /umode +B.

Reproducible: Always

Steps to Reproduce:
1.Run Mozbot.pl

Actual Results:  
Mozbot doesn't do /umode +B

Expected Results:  
/umode +B is performed

Comment 1

[Darren VanBuren]

It's all platforms / OSs.

Comment 2

[:Gavin Sharp [email: [e-mail]]]

/umode +B probably isn't supported by all IRC servers. Does mozbot already have affordances for server-specific features?

Comment 3

[Reed Loden [:reed]]

(In reply to comment #2)
> /umode +B probably isn't supported by all IRC servers. Does mozbot already have
> affordances for server-specific features?

Shouldn't matter. IRC servers that don't support +B should ignore it.

Comment 4

[:Gavin Sharp [email: [e-mail]]]

What if +B means "bad person that should be klined"?

Comment 5

[Wolf [:wolf]]

I'm not aware of any IRCd's where +B has that meaning. Comment #2 is correct though. +B seems to be only supported by Unreal.  For Ultimate, its +b  and it does have a +B which seems to be "Server Bot", so I'd think it'd just complain and not accept it. Hybrid seems to have no real definition for it.

Sucks that IRC modes aren't more standardized. 

Comment 6

[Darren VanBuren]

I was going to take this. But I was at school on my teacher's laptop and Bugzilla said midair collision when I tried to make a comment, thanks to Wolf.

So now that I'm home I would have worked on this but I guess I don't get the chance.

And +B shouldn't mean "bad person that should be klined", and even if it did, you probably couldn't set it on yourself.

Wolf: What's your current workaround for firebot?

Comment 7

[:Gavin Sharp [email: [e-mail]]]

My comment wasn't a serious objection, I was just raising the point that unconditionally setting umode +B might have undesired effects depending on IRCd in use.

Comment 8

[Darren VanBuren]

i know. Maybe we have to find a way to get the IRCd's name from the server. Don't most IRC servers say something to the client about it's version? If not, can't we do an IRC command to ask the server, and parse that for Unreal, or Ultimate and do a /umode for the correct one.

Comment 9

[Wolf [:wolf]]

Attachment: Patch v0 (From Firebot): Just do a /umode on startup

This is straight from firebot's mozbot.pl. He just sets mode +B on himself during startup.
As has been pointed out previously, this probably should be configurable to change which mode he sets on himself (and be disabled completely) and should probably live in one of the modules and not the root itself though.

Comment 10

[Reed Loden [:reed]]

Comment on attachment 320934 [details] [diff] [review]
Patch v0 (From Firebot): Just do a /umode on startup

Can we move +B to the config file?

Comment 11

[Darren VanBuren]

That would be nice. We should put a variable like umodes in the Admin module, and if the variable was empty, we would not do a /umode.

Comment 12

[Darren VanBuren]

okay, I got the patch to work and now okswbot is marked as a bot.
Do we want to close this or should we leave it open until we get a better, more customizable patch?

Comment 13

[Wolf [:wolf]]

(In reply to comment #12)
> Do we want to close this or should we leave it open until we get a better, more
> customizable patch?

The latter. It should be a pref in the admin module (combining comment 10 and 11).

This is pretty simple, i'll probably do it soon. If somebody else wants to do it first though, go right ahead.

Comment 14

[Wolf [:wolf]]

Comment on attachment 320934 [details] [diff] [review]
Patch v0 (From Firebot): Just do a /umode on startup

This is now bitrotted and no longer applies to tip.

Updated patch forthcoming...

Comment 15

[Wolf [:wolf]]

Attachment: WIP. Set user-defined umode at startup, v0.1

Removed hardcoded mode. Now its the umode variable in the Admin module.
Simply set the variable then tell the bot to cycle for it to take effect.

Comment 16

[Reed Loden [:reed]]

Comment on attachment 347389 [details] [diff] [review]
WIP. Set user-defined umode at startup, v0.1

>+    # set custom usermode for the bot
>+    &debug("Setting bot usermode...");
>+    $self->mode($self->nick, $umode);

You should only do this if $umode isn't empty.

Comment 17

[Wolf [:wolf]]

Comment on attachment 347389 [details] [diff] [review]
WIP. Set user-defined umode at startup, v0.1

Indeed, thugh in testing, it didn't seem to do anything if null already.

Also want to try to remove the requirement to have to tell the bot to /cycle to apply the mode, before this lands.

Comment 18

[Wolf [:wolf]]

Attachment: Set user-defined umode at startup, v1

Fixed drive-by review comment.
Added immediate setting of the umode to eliminate the need to cycle the bot for changes to take effect, since in general, mozbot has no other variables that require it.

Comment 19

[Reed Loden [:reed]]

Comment on attachment 348101 [details] [diff] [review]
Set user-defined umode at startup, v1

-u0p ? -u8p at least, please...

Comment 20

[Wolf [:wolf]]

Attachment: Set user-defined umode at startup, v1r2

Oops. Was looking at just the changed lines in a terminal and forgot to add context when setting the output file.

Comment 21

[Reed Loden [:reed]]

Comment on attachment 348106 [details] [diff] [review]
Set user-defined umode at startup, v1r2

>+my $umode = "";

Make this '' to match the others.

>+    # set custom usermode for the bot
>+	if ($umode) {
>+	    &debug("Setting bot usermode...");
>+	    $self->mode($self->nick, $umode);
>+	}

I'm not sure how that even works. http://mxr.mozilla.org/mozilla/source/webtools/mozbot/mozbot.pl#2039 shows mode() taking four args with the first one being an event. Maybe a setUmode() like setNick() would be better?

This is too far down the chain. You should be setting the umode pretty quickly after setting the nick in case a umode gives the bot power in order to follow through with what a module needs. Maybe an on_set_umode() with a tie to welcome (numeric 001) would be appropriate for this? Also, your indentation definitely needs fixing.

>-    }
>+	} elsif ($variable eq 'umode') {
>+		$self->mode($event, $nicks[$nick], $value, '');
>+        $self->say($event, "Attempted to change current umode to '$value'.");
>+	}

Fix your indentation. As mentioned above, a setUmode() might be better.

Comment 22

[Wolf [:wolf]]

Attachment: Set user-defined umode at startup, v2

(In reply to comment #21)
> (From update of attachment 348106 [details] [diff] [review])
> >+my $umode = "";
> 
> Make this '' to match the others.

Done. Actually, dropped it entirely, since my $umode; works for the same purpose.
> 
> >+    # set custom usermode for the bot
> >+	if ($umode) {
> >+	    &debug("Setting bot usermode...");
> >+	    $self->mode($self->nick, $umode);
> >+	}
> 
> I'm not sure how that even works.
> http://mxr.mozilla.org/mozilla/source/webtools/mozbot/mozbot.pl#2039 shows
> mode() taking four args with the first one being an event. Maybe a setUmode()
> like setNick() would be better?

It works, because the mode() you're quoting there is part of the mozbot API, called by modules (such as admin, which is what the later code is part of.) This call is for Net::IRC's method itself, which has different arguments, since this is during startup.

> 
> This is too far down the chain. You should be setting the umode pretty quickly
> after setting the nick in case a umode gives the bot power in order to follow
> through with what a module needs. Maybe an on_set_umode() with a tie to welcome
> (numeric 001) would be appropriate for this? Also, your indentation definitely
> needs fixing.

Done, though it doesn't appear possible to have 2 add_handlers() to the same event, so instead of calling when welcome, I added it to the tail of on_set_nick which means it'll work for both startup and if the bots nick changes.

> 
> >-    }
> >+	} elsif ($variable eq 'umode') {
> >+		$self->mode($event, $nicks[$nick], $value, '');
> >+        $self->say($event, "Attempted to change current umode to '$value'.");
> >+	}
> 
> Fix your indentation. As mentioned above, a setUmode() might be better.

Fixed, I hope. Tabs to spaces, thought I had my editor set, must've lost it at some point.  Not much point to adding a setUmode, as mode() does the same thing, it can't be shared between on_set_umode and here since this is a module and that isn't, and I don't currently want to add more clutter to the bot API functions.

Comment 23

[Reed Loden [:reed]]

Comment on attachment 352994 [details] [diff] [review]
Set user-defined umode at startup, v2

>+    #Set the umode for the new nick...
>+    on_set_umode($self, $event);

So, this is better, but setting the umode shouldn't be tied to nick changing, as they have nothing to do with each other. Since you can only tie 001 (welcome numeric) to one function, why not tie it to a on_connect() function (or whatever you want to name it) that calls both on_set_nick() and on_set_umode()? That would allow you to have multiple things occurring when 001 is sent.

Comment 24

[Wolf [:wolf]]

Attachment: Set user-defined umode at startup, v2.1

(In reply to comment #23)
> (From update of attachment 352994 [details] [diff] [review])
> >+    #Set the umode for the new nick...
> >+    on_set_umode($self, $event);
> 
> So, this is better, but setting the umode shouldn't be tied to nick changing,
> as they have nothing to do with each other. Since you can only tie 001 (welcome
> numeric) to one function, why not tie it to a on_connect() function (or
> whatever you want to name it) that calls both on_set_nick() and on_set_umode()?
> That would allow you to have multiple things occurring when 001 is sent.

Done, named it on_welcome, since on_connect is definitely taken (incidentally, it was where this code started in the first patch you reviewed. ;-) )

Comment 25

[Reed Loden [:reed]]

Comment on attachment 352998 [details] [diff] [review]
Set user-defined umode at startup, v2.1

r=me

Comment 26

[Wolf [:wolf]]

Checking in mozbot.pl;
/cvsroot/mozilla/webtools/mozbot/mozbot.pl,v  <--  mozbot.pl
new revision: 2.59; previous revision: 2.58
done