[142] Fixed problem with mapping overridden packet

.gitignore

@@ -1,3 +1,4 @@
+.cache
 *.py[cod]
 
 # C extensions
@@ -51,4 +52,5 @@ nosetests.xml
 # Mac cleanup
 .DS_Store
 
-cover
+cover
+.noseids

README.md

@@ -2,6 +2,8 @@
 
 StarryPy is Twisted-based plugin-driven Starbound server wrapper. It is currently in beta.
 
+# The build is currently broken while we are applying a PEP8 pull request.
+
 ## Features
 
 With the built-in plugins (which are removable):

base_plugin.py

@@ -12,7 +12,7 @@ class MapOverridePacketsMethods(type):
 
     def __new__(cls, name, bases, cls_dict):
         if name != 'BasePlugin':
-            cls_dict['overridden_packets'] = {}
+            cls_dict['overridden_methods'] = {}
             methods = (
                 key for key, value in cls_dict.iteritems()
                 if key not in cls.ignored_methods and callable(value)
@@ -23,7 +23,7 @@ def __new__(cls, name, bases, cls_dict):
                     packet_name = packet.group('packet_name').upper()
                     enum = getattr(Packets, packet_name, None)
                     if enum:
-                        cls_dict['overridden_packets'].setdefault(
+                        cls_dict['overridden_methods'].setdefault(
                             enum.value, {}
                         )[packet.group('when')] = packet_method_name
 
@@ -59,11 +59,13 @@ class BasePlugin(object):
     active = False
 
     def __init__(self, *args, **kwargs):
+        self.overridden_methods = {}
         super(BasePlugin, self).__init__(*args, **kwargs)
         if self.__class__.__name__ != 'BasePlugin':
-            for packet, when_dict in self.overridden_packets.iteritems():
+            for packet, when_dict in self.overridden_methods.iteritems():
+                self.overridden_methods.setdefault(packet, {})
                 for when, packet_name in when_dict.iteritems():
-                    self.overridden_packets[packet][when] = getattr(
+                    self.overridden_methods[packet][when] = getattr(
                         self, packet_name
                     )
 

plugin_manager.py

@@ -208,7 +208,8 @@ def load_plugins(self, plugins_to_load):
     def activate_plugins(self, plugins, dependencies):
         for plugin in (self.plugins_waiting_to_load[x] for x in plugins):
             try:
-                self.plugins[plugin.name] = plugin()
+                instance = plugin()
+                self.plugins[plugin.name] = instance
                 self.logger.debug('Instantiated plugin "%s"', plugin.name)
                 if len(plugin.depends) > 0:
                     plugin_deps = (
@@ -217,7 +218,7 @@ def activate_plugins(self, plugins, dependencies):
                     for p in plugin_deps:
                         self.plugin_classes[plugin.name].plugins[p.name] = p
                 self.plugins[plugin.name].activate()
-                self.map_plugin_packets(plugin)
+                self.map_plugin_packets(instance)
             except FatalPluginError as e:
                 self.logger.critical(
                     'A plugin reported a fatal error. Error: %s', str(e)
@@ -275,7 +276,7 @@ def map_plugin_packets(self, plugin):
         """
         Maps plugin overridden packets ready to use in do method.
         """
-        for packet_id, when_dict in plugin.overridden_packets.iteritems():
+        for packet_id, when_dict in plugin.overridden_methods.iteritems():
             for when, packet_method in when_dict.iteritems():
                 self.packets.setdefault(
                     packet_id, {}

plugins/admin_messenger/admin_messenger.py

@@ -9,7 +9,7 @@ class AdminMessenger(BasePlugin):
     """
     Adds support to message moderators/admins/owner with a @@ prefixed message.
     """
-    name = "admin_messenger"
+    name = 'admin_messenger'
     depends = ['player_manager_plugin']
 
     def activate(self):
@@ -26,48 +26,56 @@ def on_chat_sent(self, data):
             return False
         return True
 
-    def message_admins(self, message):
-        now = datetime.now()
+    def add_timestamp(self, add_normalizer=False):
         if self.config.chattimestamps:
-            timestamp = "^red;<{}> ^yellow;".format(now.strftime("%H:%M"))
+            now = datetime.now()
+            timestamp = '^red;<{}> '.format(now.strftime('%H:%M'))
+            if add_normalizer:
+                return '{}^yellow;'.format(timestamp)
+            return timestamp
         else:
-            timestamp = ""
+            return ''
+
+    def message_admins(self, message):
+        timestamp = self.add_timestamp(add_normalizer=True)
+        message = message.message[2:].decode('utf-8')
+
         for protocol in self.factory.protocols.itervalues():
             if protocol.player.access_level >= UserLevels.MODERATOR:
                 protocol.send_chat_message(
-                    "{}{}ADMIN: ^yellow;<{}^yellow;> {}{}".format(
-                        timestamp,
-                        self.config.colors["moderator"],
-                        self.protocol.player.colored_name(self.config.colors),
-                        self.config.colors["moderator"],
-                        message.message[2:].decode("utf-8")
+                    '{timestamp}{moderator_colors}'
+                    'ADMIN: ^yellow;<{player_colors}^yellow;> '
+                    '{moderator_colors}{message}'.format(
+                        timestamp=timestamp,
+                        moderator_colors=self.config.colors['moderator'],
+                        player_colors=(
+                            self.protocol.player.colored_name(
+                                self.config.colors
+                            )
+                        ),
+                        message=message
                     )
                 )
                 self.logger.info(
-                    "Received an admin message from %s. Message: %s",
-                    self.protocol.player.name,
-                    message.message[2:].decode("utf-8")
+                    'Received an admin message from %s. Message: %s',
+                    self.protocol.player.name, message
                 )
 
     @permissions(UserLevels.ADMIN)
     def broadcast_message(self, message):
-        now = datetime.now()
-        if self.config.chattimestamps:
-            timestamp = "^red;<{}> ".format(now.strftime("%H:%M"))
-        else:
-            timestamp = ""
+        timestamp = self.add_timestamp()
 
         for protocol in self.factory.protocols.itervalues():
             protocol.send_chat_message(
-                "{}{}BROADCAST: ^red;{}{}".format(
+                '{}{}BROADCAST: ^red;{}{}'.format(
                     timestamp,
-                    self.config.colors["admin"],
-                    message.message[3:].decode("utf-8").upper(),
-                    self.config.colors["default"]
+                    self.config.colors['admin'],
+                    message.message[3:].decode('utf-8').upper(),
+                    self.config.colors['default']
                 )
             )
             self.logger.info(
-                "Broadcast from %s. Message: %s",
+                'Broadcast from %s. Message: %s',
                 self.protocol.player.name,
-                message.message[3:].decode("utf-8").upper()
+                message.message[3:].decode('utf-8').upper()
             )

plugins/admin_messenger/test_admin_messenger.py

@@ -0,0 +1,195 @@
+from unittest import TestCase
+from datetime import datetime
+
+from mock import Mock, patch
+
+from plugins.admin_messenger.admin_messenger import AdminMessenger
+from plugins.core.player_manager_plugin import UserLevels
+
+
+class AdminMessengerTestCase(TestCase):
+    def test_activate(self):
+        plugin = AdminMessenger()
+        plugin.config = Mock(chat_prefix='test prefix')
+
+        with self.assertRaises(AttributeError):
+            self.plugin.prefix
+
+        plugin.activate()
+        self.assertEqual(plugin.prefix, 'test prefix')
+
+    @patch.object(AdminMessenger, 'message_admins')
+    @patch.object(AdminMessenger, 'broadcast_message')
+    @patch('plugins.admin_messenger.admin_messenger.packets')
+    def test_on_chat_sent_broadcast(
+        self, mock_packets, mock_broadcast, mock_admins
+    ):
+        mock_parse = Mock()
+        message = Mock(message='###broadcast message')
+        mock_parse.parse.return_value = message
+        mock_packets.chat_sent.return_value = mock_parse
+        mock_data = Mock(data='test data')
+        plugin = AdminMessenger()
+        plugin.config = Mock()
+        plugin.activate()
+        plugin.prefix = '#'
+
+        self.assertFalse(plugin.on_chat_sent(mock_data))
+        mock_parse.parse.assert_called_with('test data')
+        mock_broadcast.assert_called_with(message)
+
+    @patch.object(AdminMessenger, 'message_admins')
+    @patch.object(AdminMessenger, 'broadcast_message')
+    @patch('plugins.admin_messenger.admin_messenger.packets')
+    def test_on_chat_sent_message_admins(
+        self, mock_packets, mock_broadcast, mock_admins
+    ):
+        plugin = AdminMessenger()
+        plugin.config = Mock()
+        plugin.activate()
+        plugin.prefix = '#'
+        mock_parse = Mock()
+        mock_data = Mock(data='test data')
+        message = Mock(message='##admin message')
+        mock_parse.parse.return_value = message
+        mock_packets.chat_sent.return_value = mock_parse
+
+        self.assertFalse(plugin.on_chat_sent(mock_data))
+        mock_admins.assert_called_with(message)
+
+    @patch.object(AdminMessenger, 'message_admins')
+    @patch.object(AdminMessenger, 'broadcast_message')
+    @patch('plugins.admin_messenger.admin_messenger.packets')
+    def test_on_chat_sent_normal_message(
+        self, mock_packets, mock_broadcast, mock_admins
+    ):
+        mock_parse = Mock()
+        mock_data = Mock(data='test data')
+        plugin = AdminMessenger()
+        plugin.config = Mock()
+        plugin.activate()
+        plugin.prefix = '#'
+        message = Mock(message='test message')
+        mock_parse.parse.return_value = message
+
+        self.assertTrue(plugin.on_chat_sent(mock_data))
+        self.assertFalse(mock_admins.called)
+        self.assertFalse(mock_broadcast.called)
+
+    @patch('plugins.admin_messenger.admin_messenger.datetime')
+    def test_add_timestamp(self, mock_datetime):
+        current_datetime = datetime.now()
+        mock_datetime.now.return_value = current_datetime
+        plugin = AdminMessenger()
+        plugin.config = Mock()
+
+        result = plugin.add_timestamp()
+        self.assertEqual(
+            result,
+            '^red;<{}> '.format(current_datetime.strftime('%H:%M'))
+        )
+
+        result = plugin.add_timestamp(True)
+        self.assertEqual(
+            result,
+            '^red;<{}> ^yellow;'.format(current_datetime.strftime('%H:%M'))
+        )
+
+        plugin.config.chattimestamps = False
+        result = plugin.add_timestamp()
+        self.assertEqual(result, '')
+
+    @patch.object(AdminMessenger, 'add_timestamp')
+    def test_message_admins(self, mock_add_timestamp):
+        mock_logger = Mock()
+        mock_config = Mock()
+        mock_config.colors = {'moderator': 'moderator colors'}
+        mock_message = Mock()
+        mock_message.message = '##test'
+        mock_add_timestamp.return_value = 'with add_normalizer'
+        mock_just_player = Mock()
+        mock_just_player.player.access_level = UserLevels.GUEST
+        mock_player_moderator = Mock()
+        mock_player_moderator.player.access_level = UserLevels.MODERATOR
+        mock_factory = Mock()
+        mock_factory.protocols = {
+            'normal player': mock_just_player,
+            'moderator': mock_player_moderator
+        }
+        mock_protocol = Mock()
+        mock_protocol.player.colored_name.return_value = 'player colors'
+        plugin = AdminMessenger()
+        plugin.factory = mock_factory
+        plugin.config = mock_config
+        plugin.logger = mock_logger
+        plugin.protocol = mock_protocol
+
+        plugin.message_admins(mock_message)
+        mock_player_moderator.send_chat_message.assert_called_with(
+            '{}{}ADMIN: ^yellow;<{}^yellow;> {}{}'.format(
+                'with add_normalizer',
+                'moderator colors',
+                'player colors',
+                'moderator colors',
+                'test'
+            )
+        )
+        mock_logger.info.assert_called_with(
+            'Received an admin message from %s. Message: %s',
+            mock_protocol.player.name, 'test'
+        )
+        self.assertFalse(mock_just_player.send_chat_message.called)
+        mock_add_timestamp.assert_called_with(add_normalizer=True)
+
+    @patch.object(AdminMessenger, 'add_timestamp')
+    def test_broadcast_message(self, mock_add_timestamp):
+        mock_logger = Mock()
+        mock_config = Mock()
+        mock_config.colors = {
+            'admin': 'admin colors',
+            'default': 'default colors'
+        }
+        mock_message = Mock()
+        mock_message.message = '###test'
+        mock_add_timestamp.return_value = 'without add_normalizer'
+        mock_just_player = Mock()
+        mock_player_moderator = Mock()
+        mock_factory = Mock()
+        mock_factory.protocols = {
+            'normal player': mock_just_player,
+            'moderator': mock_player_moderator
+        }
+        mock_protocol = Mock()
+        mock_protocol.player.access_level = UserLevels.ADMIN
+        plugin = AdminMessenger()
+        plugin.factory = mock_factory
+        plugin.config = mock_config
+        plugin.logger = mock_logger
+        plugin.protocol = mock_protocol
+        expected_message = '{}{}BROADCAST: ^red;{}{}'.format(
+            'without add_normalizer', 'admin colors', 'TEST',
+            'default colors'
+        )
+
+        plugin.broadcast_message(mock_message)
+        mock_player_moderator.send_chat_message.assert_called_with(
+            expected_message
+        )
+        mock_just_player.send_chat_message.assert_called_with(expected_message)
+        mock_logger.info.assert_called_with(
+            'Broadcast from %s. Message: %s',
+            mock_protocol.player.name, 'TEST'
+        )
+        self.assertFalse(mock_protocol.send_chat_message.called)
+
+    def test_broadcast_message_not_admin(self):
+        mock_protocol = Mock()
+        mock_protocol.player.access_level = UserLevels.GUEST
+        plugin = AdminMessenger()
+        plugin.protocol = mock_protocol
+
+        result = plugin.broadcast_message('test')
+        self.assertFalse(result)
+        mock_protocol.send_chat_message.assert_called_with(
+            'You are not authorized to do this.'
+        )

plugins/announcer_plugin/announcer_plugin.py

@@ -2,14 +2,11 @@
 
 
 class Announcer(BasePlugin):
-    '''
+    """
     Broadcasts a message whenever a player joins or leaves the server.
-    '''
+    """
     name = 'announcer_plugin'
 
-    def activate(self):
-        super(Announcer, self).activate()
-
     def after_connect_success(self, data):
         self.factory.broadcast(
             '{} logged in.'.format(