mozbot does not automatically make itself a bot.

RESOLVED FIXED in 2.6

Status

Webtools
Mozbot
P1
minor
RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: Darren VanBuren, Assigned: wolf)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [has-attachment][r+])

Attachments

(1 attachment, 5 obsolete attachments)

(Reporter)

Description

10 years ago
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
(Reporter)

Comment 1

10 years ago
It's all platforms / OSs.
OS: Mac OS X → All
Hardware: PC → All
(Assignee)

Updated

10 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Updated

10 years ago
Assignee: nobody → bugtrap
Target Milestone: --- → 2.6
/umode +B probably isn't supported by all IRC servers. Does mozbot already have affordances for server-specific features?
(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.
What if +B means "bad person that should be klined"?
(Assignee)

Comment 5

10 years ago
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. 
(Reporter)

Comment 6

10 years ago
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?
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.
(Reporter)

Comment 8

10 years ago
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.
(Assignee)

Comment 9

10 years ago
Created attachment 320934 [details] [diff] [review]
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 on attachment 320934 [details] [diff] [review]
Patch v0 (From Firebot): Just do a /umode on startup

Can we move +B to the config file?
(Reporter)

Comment 11

10 years ago
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.
(Reporter)

Comment 12

10 years ago
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?

(Assignee)

Updated

10 years ago
Whiteboard: [has-attachment][in-firebot]
(Assignee)

Updated

10 years ago
Whiteboard: [has-attachment][in-firebot] → [has-attachment]
(Assignee)

Comment 13

10 years ago
(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.
Severity: normal → minor
Priority: -- → P2
Whiteboard: [has-attachment] → [has-attachment][needs update]
(Assignee)

Comment 14

10 years ago
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...
Attachment #320934 - Attachment is obsolete: true
(Assignee)

Updated

10 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 15

10 years ago
Created attachment 347389 [details] [diff] [review]
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 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.
Attachment #347389 - Flags: review-
(Assignee)

Comment 17

10 years ago
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.
Attachment #347389 - Attachment description: Set user-defined umode at startup, v1 → WIP. Set user-defined umode at startup, v0.1
Attachment #347389 - Flags: review-
(Assignee)

Comment 18

10 years ago
Created attachment 348101 [details] [diff] [review]
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.
Attachment #347389 - Attachment is obsolete: true
Attachment #348101 - Flags: review?(reed)
Comment on attachment 348101 [details] [diff] [review]
Set user-defined umode at startup, v1

-u0p ? -u8p at least, please...
(Assignee)

Comment 20

10 years ago
Created attachment 348106 [details] [diff] [review]
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.
Attachment #348101 - Attachment is obsolete: true
Attachment #348106 - Flags: review?(reed)
Attachment #348101 - Flags: review?(reed)
(Assignee)

Updated

10 years ago
Priority: P2 → P1
Whiteboard: [has-attachment][needs update] → [has-attachment][r?]
Attachment #348106 - Flags: review?(reed) → review-
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.
(Assignee)

Comment 22

10 years ago
Created attachment 352994 [details] [diff] [review]
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.
Attachment #352994 - Flags: review?(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.
Attachment #352994 - Flags: review?(reed) → review-
(Assignee)

Comment 24

10 years ago
Created attachment 352998 [details] [diff] [review]
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. ;-) )
Attachment #348106 - Attachment is obsolete: true
Attachment #352994 - Attachment is obsolete: true
Attachment #352998 - Flags: review?(reed)
Comment on attachment 352998 [details] [diff] [review]
Set user-defined umode at startup, v2.1

r=me
Attachment #352998 - Flags: review?(reed) → review+
(Assignee)

Comment 26

10 years ago
Checking in mozbot.pl;
/cvsroot/mozilla/webtools/mozbot/mozbot.pl,v  <--  mozbot.pl
new revision: 2.59; previous revision: 2.58
done
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Whiteboard: [has-attachment][r?] → [has-attachment][r+]
You need to log in before you can comment on or make changes to this bug.