Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Msg target can be changed by fast messages to the bot. #42

Closed
pschwartz opened this Issue · 4 comments

2 participants

@pschwartz

There is a current issue with the msg target that allows the target to change if multiple messages coming before a plugin writes to the irc bot.

This is due to target being a member of the IRC class.

I think the best method to correct this is to encapsulate the message building/sending to a class external of the IRC class and use the object built to send the message.

To ease this the class should include a Factory to allow rapid creation of message in place.

@pschwartz pschwartz referenced this issue from a commit in pschwartz/pyhole
@pschwartz pschwartz gh-42 Moved to encapsulated Message class for transport.
This will require a change to all plugins to no longer call
self.irc.notice() or self.irc.reply() directly.

The new correct method is to import:

from pyhole.core import Message

and in function that would call reply:
_msg = Message.getMessage(**kwargs)
_msg.reply = "Message to return to irc"
_msg.dispatch(self.irc)

TODO(pschwartz): Add irc connection to Factory so user does not need to
pass it to dispatch().

TODO(pschwartz): Update current plugins to use new api.

TODO(pschwartz): further clean up client code to move irc hooks (client
related not plugin) to seperate modules allowing better plugin actions
based on irc hook.
7b99696
@blamarvt

From pschwartz@7b99696

This will require a change to all plugins to no longer call
self.irc.notice() or self.irc.reply() directly.

The new correct method is to import:

from pyhole.core import Message

and in function that would call reply:
_msg = Message.getMessage(**kwargs)
_msg.reply = "Message to return to irc"
_msg.dispatch(self.irc)

From the plugin developer's perspective what does this buy me except for having to do more work? I'd like to think there is a solution to this problem that doesn't involve changing what I'd consider to be a simple interface (self.irc.reply(...)).

@pschwartz

I should have been a little clearer on this. There is only actually 1 line of code difference.

Here his a very simple function for a command !test.

@plugin.hook_add_command("test")
@utils.spawn
def do_display_test(self, params, **kwargs):
    reply = "<--- This is a test --->"
    self.irc.reply(reply)

Here is the same function with the new message object.

@plugin.hook_add_command("test")
@utils.spawn
def do_display_test(self, params=None, **kwargs):
    _msg = Message.getMessage(**kwargs)
    _msg.reply = "<--- This is a test --->"
    _msg.dispatch(self.irc)

The difference in these is a single helper function for Message.getMessage() that retrieves the full_message which is the message object from kwargs.

I am already removing the need for passing the irc object to dispatch so that will reduce code there.

There are 2 main benefits to doing it this way over reworking all the code to pass a target to the plugin that could then be passed back to set where the message needs to go.

  1. Every message is a separate object preventing any possibility of the message going to an unintended target.

  2. Plugins no longer worry about the access to an irc object in order to send a message. The irc object will be accessible from the message object which will allow a simple .dispatch() to send any type of message back to the irc server. This will allow the plugins to be agnostic to the type of message they received or send.

@pschwartz

To further comment on the above. It might be possible to update the plugin api that is generating the function calls to include a message object outside of kwargs allowing for quicker direct access to the object.

@pschwartz pschwartz referenced this issue from a commit in pschwartz/pyhole
@pschwartz pschwartz gh-42 Reworked how message is instantiated and used.
Changed part of the plugin api to call plugins with:

``` func(msg, param, **kwargs) ```
d884a74
@pschwartz pschwartz referenced this issue from a commit in pschwartz/pyhole
@pschwartz pschwartz gh-42 plugins pep8 pass. 9cf7edd
@pschwartz pschwartz referenced this issue from a commit in pschwartz/pyhole
@pschwartz pschwartz gh-42 Moved to encapsulated Message class for transport.
This will require a change to all plugins to no longer call
self.irc.notice() or self.irc.reply() directly.

The new correct method is to import:

from pyhole.core import Message

and in function that would call reply:
_msg = Message.getMessage(**kwargs)
_msg.reply = "Message to return to irc"
_msg.dispatch(self.irc)

TODO(pschwartz): Add irc connection to Factory so user does not need to
pass it to dispatch().

TODO(pschwartz): Update current plugins to use new api.

TODO(pschwartz): further clean up client code to move irc hooks (client
related not plugin) to seperate modules allowing better plugin actions
based on irc hook.
c3f5f15
@pschwartz pschwartz referenced this issue from a commit in pschwartz/pyhole
@pschwartz pschwartz gh-42 Reworked how message is instantiated and used.
Changed part of the plugin api to call plugins with:

``` func(msg, param, **kwargs) ```
df31b6d
@pschwartz pschwartz referenced this issue from a commit in pschwartz/pyhole
@pschwartz pschwartz gh-42 plugins pep8 pass. a21fde7
@pschwartz

This has been open for a long time after the code has been merged. Closing.

@pschwartz pschwartz closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.