From 431e4fc4fe0a97ada0dc937828ddde12b9a2e67c Mon Sep 17 00:00:00 2001
From: Joseph Daniel <gjdaniel1999@gmail.com>
Date: Wed, 14 Aug 2019 21:54:27 +0530
Subject: [PATCH 1/9] Show msg from ValueError

---
 piqueserver/commands.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/piqueserver/commands.py b/piqueserver/commands.py
index 53242a0ce..6afd9731c 100644
--- a/piqueserver/commands.py
+++ b/piqueserver/commands.py
@@ -427,8 +427,8 @@ def _handle_command(connection, command, parameters):
         msg = str(e)
     except PermissionDenied as e:
         msg = 'You can\'t do that: {}'.format(str(e))
-    except ValueError:
-        msg = 'Invalid parameters'
+    except ValueError as e:
+        msg = str(e) if e.args else "Invalid parameters"
 
     return format_command_error(command_func, msg)
 

From d7bf9f1802893a0f07f74e6c0fe301d18176691c Mon Sep 17 00:00:00 2001
From: Joseph Daniel <gjdaniel1999@gmail.com>
Date: Wed, 14 Aug 2019 22:12:56 +0530
Subject: [PATCH 2/9] Add ValueError msgs for /fog

---
 piqueserver/core_commands/game.py | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/piqueserver/core_commands/game.py b/piqueserver/core_commands/game.py
index 7e1b87700..b4f9265bd 100644
--- a/piqueserver/core_commands/game.py
+++ b/piqueserver/core_commands/game.py
@@ -246,22 +246,24 @@ def fog(connection, *args):
         hex_code = args[0][1:]
 
         if (len(hex_code) != 3) and (len(hex_code) != 6):
-            raise ValueError()
+            raise ValueError("Invalid hex code length")
 
         if len(hex_code) == 3: # it's a short hex code, turn it into a full one
             hex_code = (hex_code[0]*2) + (hex_code[1]*2) + (hex_code[2]*2)
 
         valid_characters = re.compile('[a-fA-F0-9]')
         for char in hex_code:
-            if valid_characters.match(char) == None:
-                raise ValueError()
+            if valid_characters.match(char) is None:
+                raise ValueError("Invalid hex code characters")
 
         r = int(hex_code[0:2], base=16)
         g = int(hex_code[2:4], base=16)
         b = int(hex_code[4:6], base=16)
     else:
-        raise ValueError()
+        raise ValueError("Neither RGB or hex code provided")
 
+    old_fog_color = connection.protocol.fog_color
     connection.protocol.set_fog_color((r, g, b))
-    # TODO: Warn when the fog was set to the same color as before
+    if old_fog_color == (r, g, b):
+        return 'Fog color changed successfully\nWarning: fog color set to same color as before'
     return 'Fog color changed successfully'

From 42f3de34dc440c9867cdbe55a3fa765a39bd9fd0 Mon Sep 17 00:00:00 2001
From: Joseph Daniel <gjdaniel1999@gmail.com>
Date: Wed, 14 Aug 2019 22:43:47 +0530
Subject: [PATCH 3/9] Add target_player decorator

---
 piqueserver/commands.py            | 29 ++++++++++++++++++++++++++---
 tests/piqueserver/test_commands.py | 28 +++++++++++++++++++++++++++-
 2 files changed, 53 insertions(+), 4 deletions(-)

diff --git a/piqueserver/commands.py b/piqueserver/commands.py
index 6afd9731c..cbf7d60fe 100644
--- a/piqueserver/commands.py
+++ b/piqueserver/commands.py
@@ -268,10 +268,33 @@ def _decorated(connection, *args, **kwargs):
         func(connection, *args, **kwargs)
     return _decorated
 
+def target_player(func: Callable):
+    """This decorator converts first argument of a command to a `piqueserver.FeatureConnection`.
+       It's intended for commands which accept single argument for target player eg. /fly [player].
+       It implicitly uses invoker as target if no arguments are provided.
+       It uses first argument are player name or id for targetting.
+       It forces non-player invokers to provide player argument.
 
-# TODO: all of these utility functions should be seperated from the actual
-# implementation of the commands
-
+    >>> @command()
+    ... @target_player
+    ... def fly(connection, target):
+    ...     target.fly = True
+    ...     pass
+    """
+    @functools.wraps(func)
+    def _decorated(connection, *args, **kwargs):
+        is_player = connection in connection.protocol.players.values()
+        # implicitly set target to invoker if no args
+        if len(args) == 0 and is_player:
+            args = [connection]
+        # try and use first arg as player name or id to target
+        elif len(args) > 0:
+            args = [get_player(connection.protocol, args[0]), *args[1:]]
+        # console or irc invokers are required to provide a target
+        else:
+            raise ValueError("Target player is required")
+        func(connection, *args, **kwargs)
+    return _decorated
 
 def get_player(protocol, value: str, spectators=True):
     """
diff --git a/tests/piqueserver/test_commands.py b/tests/piqueserver/test_commands.py
index e94b1eb9a..6a5538ff9 100644
--- a/tests/piqueserver/test_commands.py
+++ b/tests/piqueserver/test_commands.py
@@ -1,5 +1,6 @@
 import unittest
-from piqueserver.commands import command, _alias_map
+from unittest.mock import Mock
+from piqueserver.commands import command, _alias_map, target_player
 
 
 class TestCommandDecorator(unittest.TestCase):
@@ -28,3 +29,28 @@ def test_command_alias(self):
         def test():
             pass
         self.assertEqual(_alias_map['n'], 'name')
+
+
+class TestTargetPlayerDecorator(unittest.TestCase):
+    def test_no_arg_non_player(self):
+        @command()
+        @target_player
+        def test(connection, player):
+            pass
+
+        connection = Mock()
+        connection.protocol.players.values = lambda: []
+
+        with self.assertRaises(ValueError):
+            test(connection, player=None)
+
+    def test_no_arg_player(self):
+        @command()
+        @target_player
+        def test(connection, player):
+            self.assertEqual(connection, player)
+
+        connection = Mock()
+        connection.protocol.players.values = lambda: [connection]
+
+        test(connection)

From 3483cb9cf35b8504acae84951dc608bfd9dce28b Mon Sep 17 00:00:00 2001
From: Joseph Daniel <gjdaniel1999@gmail.com>
Date: Fri, 16 Aug 2019 20:07:58 +0530
Subject: [PATCH 4/9] Port commands to use target_player Commands affected:
 /switch /ping /ip, /invisible, /godbuild /unstick, /where, /fly /client,
 /heal /givestrike /rapid

---
 piqueserver/core_commands/game.py       | 19 +++++---------
 piqueserver/core_commands/info.py       | 18 +++++--------
 piqueserver/core_commands/moderation.py | 34 +++++++------------------
 piqueserver/core_commands/movement.py   | 32 ++++++++---------------
 piqueserver/core_commands/player.py     | 22 +++++++---------
 piqueserver/scripts/airstrike2.py       | 13 +++-------
 piqueserver/scripts/rapid.py            | 11 +++-----
 7 files changed, 49 insertions(+), 100 deletions(-)

diff --git a/piqueserver/core_commands/game.py b/piqueserver/core_commands/game.py
index b4f9265bd..4c9aa69c1 100644
--- a/piqueserver/core_commands/game.py
+++ b/piqueserver/core_commands/game.py
@@ -3,7 +3,8 @@
 from twisted.internet import reactor
 from piqueserver.config import cast_duration
 from pyspades.common import prettify_timespan
-from piqueserver.commands import command, CommandError, get_player, get_team, get_truthy
+from piqueserver.commands import (
+    command, CommandError, get_player, get_team, get_truthy, target_player)
 
 
 @command('time')
@@ -70,26 +71,18 @@ def unlock(connection, value):
 
 
 @command(admin_only=True)
-def switch(connection, player=None, team=None):
+@target_player
+def switch(connection, player):
     """
     Switch teams either for yourself or for a given player
     /switch [player]
     """
     protocol = connection.protocol
-    if player is not None:
-        player = get_player(protocol, player)
-    elif connection in protocol.players.values():
-        player = connection
-    else:
-        raise ValueError()
     if player.team.spectator:
         player.send_chat(
             "The switch command can't be used on a spectating player.")
         return
-    if team is None:
-        new_team = player.team.other
-    else:
-        new_team = get_team(connection, team)
+    new_team = player.team.other
     if player.invisible:
         old_team = player.team
         player.team = new_team
@@ -248,7 +241,7 @@ def fog(connection, *args):
         if (len(hex_code) != 3) and (len(hex_code) != 6):
             raise ValueError("Invalid hex code length")
 
-        if len(hex_code) == 3: # it's a short hex code, turn it into a full one
+        if len(hex_code) == 3:  # it's a short hex code, turn it into a full one
             hex_code = (hex_code[0]*2) + (hex_code[1]*2) + (hex_code[2]*2)
 
         valid_characters = re.compile('[a-fA-F0-9]')
diff --git a/piqueserver/core_commands/info.py b/piqueserver/core_commands/info.py
index efc124537..9a3520c6f 100644
--- a/piqueserver/core_commands/info.py
+++ b/piqueserver/core_commands/info.py
@@ -1,4 +1,5 @@
-from piqueserver.commands import command, _commands, has_permission, get_player, get_command_help, player_only
+from piqueserver.commands import (
+    command, _commands, has_permission, get_player, get_command_help, player_only, target_player)
 
 
 @command()
@@ -13,21 +14,16 @@ def streak(connection):
 
 
 @command()
-def ping(connection, value=None):
+@target_player
+def ping(connection, player):
     """
     Tell your current ping (time for your actions to be received by the server)
     /ping
     """
-    if value is None:
-        if connection not in connection.protocol.players.values():
-            raise ValueError()
-        player = connection
-    else:
-        player = get_player(connection.protocol, value)
     ping = player.latency
-    if value is None:
-        return 'Your ping is %s ms. Lower ping is better!' % ping
-    return "%s's ping is %s ms" % (player.name, ping)
+    if connection == player:
+        return 'Your ping is {} ms. Lower ping is better!'.format(player.latency)
+    return "{}'s ping is {} ms".format(player.name, player.latency)
 
 
 @command()
diff --git a/piqueserver/core_commands/moderation.py b/piqueserver/core_commands/moderation.py
index f12af0768..4180ecf2e 100644
--- a/piqueserver/core_commands/moderation.py
+++ b/piqueserver/core_commands/moderation.py
@@ -1,16 +1,15 @@
 from random import choice
 from twisted.internet import reactor
+# aparently, we need to send packets in this file. For now, I give in.
 from pyspades.contained import CreatePlayer, SetTool, KillAction, InputData, SetColor, WeaponInput
 from pyspades.constants import (GRENADE_KILL, FALL_KILL, NETWORK_FPS)
 from pyspades.common import (
     prettify_timespan,
     make_color)
-from piqueserver.commands import command, CommandError, get_player, join_arguments
+from piqueserver.commands import (
+    command, CommandError, get_player, join_arguments, target_player)
 from piqueserver.utils import timeparse
 
-# aparently, we need to send packets in this file. For now, I give in.
-
-
 
 def has_digits(s: str) -> bool:
     return any(char.isdigit() for char in s)
@@ -201,17 +200,12 @@ def unmute(connection, value):
 
 
 @command(admin_only=True)
-def ip(connection, value=None):
+@target_player
+def ip(connection, player):
     """
     Get the IP of a user
     /ip [player]
     """
-    if value is None:
-        if connection not in connection.protocol.players.values():
-            raise ValueError()
-        player = connection
-    else:
-        player = get_player(connection.protocol, value)
     return 'The IP of %s is %s' % (player.name, player.address[0])
 
 
@@ -237,18 +231,13 @@ def who_was(connection, value):
 
 
 @command('invisible', 'invis', 'inv', admin_only=True)
-def invisible(connection, player=None):
+@target_player
+def invisible(connection, player):
     """
     Turn invisible
     /invisible [player]
     """
     protocol = connection.protocol
-    if player is not None:
-        player = get_player(protocol, player)
-    elif connection in protocol.players.values():
-        player = connection
-    else:
-        raise ValueError()
     # TODO: move this logic to a more suitable place
     player.invisible = not player.invisible
     player.filter_visibility_data = player.invisible
@@ -364,18 +353,13 @@ def god(connection, player=None):
 
 
 @command('godbuild', admin_only=True)
-def god_build(connection, player=None):
+@target_player
+def god_build(connection, player):
     """
     Place blocks that can be destroyed only by players with godmode activated
     /godbuild [player]
     """
     protocol = connection.protocol
-    if player is not None:
-        player = get_player(protocol, player)
-    elif connection in protocol.players.values():
-        player = connection
-    else:
-        raise ValueError()
 
     player.god_build = not player.god_build
 
diff --git a/piqueserver/core_commands/movement.py b/piqueserver/core_commands/movement.py
index 7957dc0a4..855339751 100644
--- a/piqueserver/core_commands/movement.py
+++ b/piqueserver/core_commands/movement.py
@@ -1,18 +1,15 @@
 from pyspades.common import (coordinates, to_coordinates)
 from piqueserver.commands import (command, CommandError, get_player,
-                                  PermissionDenied, player_only)
+                                  PermissionDenied, player_only, target_player)
 
 
 @command(admin_only=True)
-def unstick(connection, player=None):
+@target_player
+def unstick(connection, player):
     """
     Unstick yourself or another player and inform everyone on the server of it
     /unstick [player]
     """
-    if player is not None:
-        player = get_player(connection.protocol, player)
-    else:
-        player = connection
     connection.protocol.send_chat("%s unstuck %s" %
                                   (connection.name, player.name), irc=True)
     player.set_location_safe(player.get_location())
@@ -66,7 +63,8 @@ def do_move(connection, args, silent=False):
     elif arg_count == 3 or arg_count == 4:
         x = min(max(0, int(args[initial_index])), 511)
         y = min(max(0, int(args[initial_index + 1])), 511)
-        z = min(max(0, int(args[initial_index + 2])), connection.protocol.map.get_height(x, y) - 2)
+        z = min(max(0, int(args[initial_index + 2])),
+                connection.protocol.map.get_height(x, y) - 2)
         position = '%d %d %d' % (x, y, z)
     else:
         raise ValueError('Wrong number of parameters!')
@@ -104,18 +102,15 @@ def do_move(connection, args, silent=False):
 
 
 @command(admin_only=True)
-def where(connection, player=None):
+@target_player
+def where(connection, player):
     """
     Tell you the coordinates of yourself or of a given player
     /where [player]
     """
-    if player is not None:
-        connection = get_player(connection.protocol, player)
-    elif connection not in connection.protocol.players.values():
-        raise ValueError()
-    x, y, z = connection.get_location()
+    x, y, z = player.get_location()
     return '%s is in %s (%s, %s, %s)' % (
-        connection.name, to_coordinates(x, y), int(x), int(y), int(z))
+        player.name, to_coordinates(x, y), int(x), int(y), int(z))
 
 
 @command('teleport', 'tp', admin_only=True)
@@ -162,19 +157,14 @@ def tpsilent(connection, player1, player2=None):
 
 
 @command(admin_only=True)
-def fly(connection, player=None):
+@target_player
+def fly(connection, player):
     """
     Enable flight
     /fly [player]
     Hold control and press space ;)
     """
     protocol = connection.protocol
-    if player is not None:
-        player = get_player(protocol, player)
-    elif connection in protocol.players.values():
-        player = connection
-    else:
-        raise ValueError()
     player.fly = not player.fly
 
     message = 'now flying' if player.fly else 'no longer flying'
diff --git a/piqueserver/core_commands/player.py b/piqueserver/core_commands/player.py
index a46fdde5a..7d31e8efd 100644
--- a/piqueserver/core_commands/player.py
+++ b/piqueserver/core_commands/player.py
@@ -1,16 +1,15 @@
-from piqueserver.commands import command, get_player, PermissionDenied, player_only
+from piqueserver.commands import command, get_player, PermissionDenied, player_only, target_player
 
 @command("client", "cli")
-def client(connection, target=None):
+@target_player
+def client(connection, player):
     """
     Tell you information about your client or the client of a given player
     /client [player]
     """
-    if not target:
-        player = connection
+    if connection == player:
         who_is = "You are"
     else:
-        player = get_player(connection.protocol, target)
         who_is = player.name + " is"
 
     return "{} connected with {}".format(who_is, player.client_string)
@@ -65,19 +64,16 @@ def kill(connection, value=None):
 
 
 @command(admin_only=True)
-def heal(connection, player=None):
+@target_player
+def heal(connection, player):
     """
     Heal and refill yourself or a given player and inform everyone on the server of this action
     /heal [player]
     """
-    if player is not None:
-        player = get_player(connection.protocol, player, False)
-        message = '%s was healed by %s' % (player.name, connection.name)
-    else:
-        if connection not in connection.protocol.players.values():
-            raise ValueError()
-        player = connection
+    if player == connection:
         message = '%s was healed' % (connection.name)
+    else:
+        message = '%s was healed by %s' % (player.name, connection.name)
     player.refill()
     connection.protocol.send_chat(message, irc=True)
 
diff --git a/piqueserver/scripts/airstrike2.py b/piqueserver/scripts/airstrike2.py
index bc8cf18a7..814e3f5e9 100644
--- a/piqueserver/scripts/airstrike2.py
+++ b/piqueserver/scripts/airstrike2.py
@@ -17,7 +17,8 @@
 from pyspades.common import to_coordinates, Vertex3
 from pyspades.world import Grenade
 from pyspades.constants import UPDATE_FREQUENCY, WEAPON_TOOL
-from piqueserver.commands import command, admin, get_player, player_only
+from piqueserver.commands import (
+    command, admin, get_player, player_only, target_player)
 
 STREAK_REQUIREMENT = 8
 
@@ -59,14 +60,8 @@ def airstrike(connection, *args):
 
 
 @command('givestrike', admin_only=True)
-def give_strike(connection, player=None):
-    protocol = connection.protocol
-    if player is not None:
-        player = get_player(protocol, player)
-    elif connection in protocol.players.values():
-        player = connection
-    else:
-        raise ValueError()
+@target_player
+def give_strike(connection, player):
     player.airstrike = True
     player.send_chat(S_READY)
 
diff --git a/piqueserver/scripts/rapid.py b/piqueserver/scripts/rapid.py
index e704c3c69..97b3da706 100644
--- a/piqueserver/scripts/rapid.py
+++ b/piqueserver/scripts/rapid.py
@@ -16,7 +16,7 @@
 from twisted.internet.reactor import callLater
 from twisted.internet.task import LoopingCall
 from pyspades import contained as loaders
-from piqueserver.commands import command, get_player
+from piqueserver.commands import command, get_player, target_player
 
 ALWAYS_RAPID = False
 RAPID_INTERVAL = 0.08
@@ -24,14 +24,9 @@
 
 
 @command('rapid', admin_only=True)
-def toggle_rapid(connection, player=None):
+@target_player
+def toggle_rapid(connection, player):
     protocol = connection.protocol
-    if player is not None:
-        player = get_player(protocol, player)
-    elif connection in protocol.players.values():
-        player = connection
-    else:
-        raise ValueError()
 
     player.rapid = rapid = not player.rapid
     player.rapid_hack_detect = not rapid

From dea3f7075950c9e2006448fadf73a1c45ffc7d79 Mon Sep 17 00:00:00 2001
From: Joseph Daniel <gjdaniel1999@gmail.com>
Date: Fri, 16 Aug 2019 20:35:47 +0530
Subject: [PATCH 5/9] Add more ValueError msgs

---
 piqueserver/core_commands/movement.py | 4 ++--
 piqueserver/scripts/afk.py            | 2 +-
 piqueserver/scripts/analyze.py        | 2 +-
 piqueserver/scripts/badmin.py         | 2 +-
 piqueserver/scripts/daycycle.py       | 8 ++++----
 piqueserver/scripts/timedmute.py      | 2 +-
 piqueserver/scripts/votekick.py       | 2 +-
 pyspades/common.pyx                   | 6 +++---
 8 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/piqueserver/core_commands/movement.py b/piqueserver/core_commands/movement.py
index 855339751..737ce91b2 100644
--- a/piqueserver/core_commands/movement.py
+++ b/piqueserver/core_commands/movement.py
@@ -73,7 +73,7 @@ def do_move(connection, args, silent=False):
     if arg_count == 1 or arg_count == 3:
         # must be run by a player in this case because moving self
         if connection not in connection.protocol.players.values():
-            raise ValueError()
+            raise ValueError("Both player and target player are required")
         player = connection.name
     # player specified
     elif arg_count == 2 or arg_count == 4:
@@ -133,7 +133,7 @@ def teleport(connection, player1, player2=None, silent=False):
             return 'No administrator rights!'
     else:
         if connection not in connection.protocol.players.values():
-            raise ValueError()
+            raise ValueError("Both player and target player are required")
         player, target = connection, player1
         silent = silent or player.invisible
         message = '%s ' + \
diff --git a/piqueserver/scripts/afk.py b/piqueserver/scripts/afk.py
index f308ba3dc..fb0032e84 100644
--- a/piqueserver/scripts/afk.py
+++ b/piqueserver/scripts/afk.py
@@ -38,7 +38,7 @@ def kick_afk(connection, minutes, amount=None):
     protocol = connection.protocol
     minutes = int(minutes)
     if minutes < 1:
-        raise ValueError()
+        raise ValueError("Minutes cannot be < 1")
     to_kick = []
     seconds = minutes * 60.0
     minutes_s = prettify_timespan(seconds)
diff --git a/piqueserver/scripts/analyze.py b/piqueserver/scripts/analyze.py
index 78cfe3e6f..2edba8241 100644
--- a/piqueserver/scripts/analyze.py
+++ b/piqueserver/scripts/analyze.py
@@ -33,7 +33,7 @@ def analyze_shot(connection, player=None):
 
     player = get_player(protocol, player)
     if player not in protocol.players:
-        raise ValueError()  # FIXME: proper error
+        raise ValueError("Target player is required")
 
     if connection.name in protocol.analyzers:
         if player.name == protocol.analyzers[connection.name]:
diff --git a/piqueserver/scripts/badmin.py b/piqueserver/scripts/badmin.py
index 0cbd9abb9..189034171 100644
--- a/piqueserver/scripts/badmin.py
+++ b/piqueserver/scripts/badmin.py
@@ -101,7 +101,7 @@ def score_grief(connection, player, time=None):  # 302 = blue (0), #303 = green
     color = connection not in connection.protocol.players.values() and connection.colors
     minutes = float(time or 2)
     if minutes < 0.0:
-        raise ValueError()
+        raise ValueError("Minutes cannot be < 0")
     time = reactor.seconds() - minutes * 60.0
     blocks_removed = player.blocks_removed or []
     blocks = [b[1] for b in blocks_removed if b[0] >= time]
diff --git a/piqueserver/scripts/daycycle.py b/piqueserver/scripts/daycycle.py
index c1b150c0f..0b1ce07ed 100644
--- a/piqueserver/scripts/daycycle.py
+++ b/piqueserver/scripts/daycycle.py
@@ -47,10 +47,10 @@ def day_time(connection, value=None):
     if value is not None:
         if not connection.admin:
             return S_NO_RIGHTS
-        value = float(value)
-        if value < 0.0:
-            raise ValueError()
-        connection.protocol.current_time = value
+        time = float(value)
+        if time < 0.0:
+            raise ValueError("Time cannot be < 0")
+        connection.protocol.current_time = time
         connection.protocol.update_day_color()
     f, i = modf(connection.protocol.current_time)
     return S_TIME_OF_DAY.format(hours=int(i), minutes=int(f * 60))
diff --git a/piqueserver/scripts/timedmute.py b/piqueserver/scripts/timedmute.py
index dbdd93869..5ea7bf9b5 100644
--- a/piqueserver/scripts/timedmute.py
+++ b/piqueserver/scripts/timedmute.py
@@ -28,7 +28,7 @@ def timed_mute(connection, *args):
     player = get_player(protocol, nick)
 
     if time < 0:
-        raise ValueError()
+        raise ValueError("Time cannot be < 0")
 
     if not player.mute:
         TimedMute(player, time, reason)
diff --git a/piqueserver/scripts/votekick.py b/piqueserver/scripts/votekick.py
index 0c60abe3a..a7bea598d 100644
--- a/piqueserver/scripts/votekick.py
+++ b/piqueserver/scripts/votekick.py
@@ -98,7 +98,7 @@ def start_votekick(connection, *args):
             # player requested votekick info
             protocol.votekick.send_chat_update(player)
             return
-        raise ValueError()
+        raise ValueError("Target player is required")
 
     value = args[0]
     victim = get_player(protocol, value)
diff --git a/pyspades/common.pyx b/pyspades/common.pyx
index 3188df4c0..ef8c978c7 100644
--- a/pyspades/common.pyx
+++ b/pyspades/common.pyx
@@ -33,13 +33,13 @@ EPSILON = 0.0000001
 
 def coordinates(data):
     if data is None:
-        raise ValueError()
+        raise ValueError("data is None")
     if len(data) != 2:
-        raise ValueError()
+        raise ValueError("invalid data length")
     x = (ord(data[0].lower()) - ord('a')) * 64
     y = (int(data[1]) - 1) * 64
     if x < 0 or x >= 512 or y < 0 or y >= 512:
-        raise ValueError()
+        raise ValueError("coordinates are out-of-bounds")
     return x, y
 
 def to_coordinates(x, y):

From 51755daa8c3b73f674dffd2a04dd1fe0866c796c Mon Sep 17 00:00:00 2001
From: Joseph Daniel <gjdaniel1999@gmail.com>
Date: Fri, 16 Aug 2019 20:37:36 +0530
Subject: [PATCH 6/9] Port more commands to use target_player /from /paint
 /unlink

---
 piqueserver/scripts/geoip.py      | 19 ++++++-------------
 piqueserver/scripts/paint.py      |  9 ++-------
 piqueserver/scripts/runningman.py | 12 +++---------
 3 files changed, 11 insertions(+), 29 deletions(-)

diff --git a/piqueserver/scripts/geoip.py b/piqueserver/scripts/geoip.py
index 1231d1aa7..926e39c9c 100644
--- a/piqueserver/scripts/geoip.py
+++ b/piqueserver/scripts/geoip.py
@@ -14,7 +14,7 @@
 '''
 
 import os
-from piqueserver.commands import command, restrict, get_player
+from piqueserver.commands import (command, restrict, get_player, target_player)
 from piqueserver.config import config
 
 # optional commands
@@ -31,17 +31,9 @@
     )
 finally:
 
-    @restrict('admin')
-    @command('from')
-    def where_from(connection, value=None):
-        # Get player IP address
-        if value is None:
-            if connection not in connection.protocol.players.values():
-                raise ValueError()
-            player = connection
-        else:
-            player = get_player(connection.protocol, value)
-
+    @command('from', admin_only=True)
+    @target_player
+    def where_from(connection, player):
         # Query database
         try:
             record = database.city(player.address[0])
@@ -49,7 +41,8 @@ def where_from(connection, value=None):
             return 'Player location could not be determined.'
 
         # Extract info
-        raw_items = (record.city, *reversed(record.subdivisions), record.country)
+        raw_items = (record.city, *reversed(record.subdivisions),
+                     record.country)
 
         items = (raw_item.name for raw_item in raw_items if raw_item.name is not None)
 
diff --git a/piqueserver/scripts/paint.py b/piqueserver/scripts/paint.py
index 0b1dded2c..51c24770a 100644
--- a/piqueserver/scripts/paint.py
+++ b/piqueserver/scripts/paint.py
@@ -14,20 +14,15 @@
 from pyspades.contained import BlockAction
 from pyspades.common import Vertex3
 from pyspades.constants import *
-from piqueserver.commands import command, admin, get_player
+from piqueserver.commands import command, admin, get_player, target_player
 
 PAINT_RAY_LENGTH = 32.0
 
 
 @command(admin_only=True)
+@target_player
 def paint(connection, player=None):
     protocol = connection.protocol
-    if player is not None:
-        player = get_player(protocol, player)
-    elif connection in protocol.players.values():
-        player = connection
-    else:
-        raise ValueError()
 
     player.painting = not player.painting
 
diff --git a/piqueserver/scripts/runningman.py b/piqueserver/scripts/runningman.py
index 7d938b4e5..c009686d3 100644
--- a/piqueserver/scripts/runningman.py
+++ b/piqueserver/scripts/runningman.py
@@ -18,7 +18,7 @@
 from pyspades.collision import distance_3d_vector
 from pyspades.constants import *
 from pyspades.common import Vertex3
-from piqueserver.commands import command, admin, get_player
+from piqueserver.commands import (command, admin, get_player, target_player)
 
 ENABLED_AT_START = False
 
@@ -61,18 +61,12 @@ def relink(connection):
 
 
 @command(admin_only=True)
+@target_player
 def unlink(connection, player=None):
     protocol = connection.protocol
     if not protocol.running_man:
         return S_NOT_ENABLED
 
-    if player is not None:
-        player = get_player(protocol, player)
-    elif connection in protocol.players.values():
-        player = connection
-    else:
-        raise ValueError()
-
     player.drop_link()
     player.linkable = not player.linkable
     player.send_chat(S_LINKABLE_SELF if player.linkable else S_UNLINKABLE_SELF)
@@ -154,7 +148,7 @@ def can_be_linked_to(self, player):
 
         def get_new_link(self):
             available = list(filter(self.can_be_linked_to,
-                                     self.team.get_players()))
+                                    self.team.get_players()))
             if not available:
                 return
             self.drop_link(force_message=True)

From 4a021206ca4fb66ca61b614bbba21b1746f2ee4b Mon Sep 17 00:00:00 2001
From: Joseph Daniel <gjdaniel1999@gmail.com>
Date: Sat, 17 Aug 2019 10:41:44 +0530
Subject: [PATCH 7/9] Use `is` for identity checks

---
 piqueserver/core_commands/info.py   | 2 +-
 piqueserver/core_commands/player.py | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/piqueserver/core_commands/info.py b/piqueserver/core_commands/info.py
index 9a3520c6f..b17de556c 100644
--- a/piqueserver/core_commands/info.py
+++ b/piqueserver/core_commands/info.py
@@ -21,7 +21,7 @@ def ping(connection, player):
     /ping
     """
     ping = player.latency
-    if connection == player:
+    if connection is player:
         return 'Your ping is {} ms. Lower ping is better!'.format(player.latency)
     return "{}'s ping is {} ms".format(player.name, player.latency)
 
diff --git a/piqueserver/core_commands/player.py b/piqueserver/core_commands/player.py
index 7d31e8efd..02bf09d9c 100644
--- a/piqueserver/core_commands/player.py
+++ b/piqueserver/core_commands/player.py
@@ -7,7 +7,7 @@ def client(connection, player):
     Tell you information about your client or the client of a given player
     /client [player]
     """
-    if connection == player:
+    if connection is player:
         who_is = "You are"
     else:
         who_is = player.name + " is"

From 3c766cb0e071803e49394f398f1132f5911944cc Mon Sep 17 00:00:00 2001
From: Joseph Daniel <gjdaniel1999@gmail.com>
Date: Sat, 17 Aug 2019 10:58:04 +0530
Subject: [PATCH 8/9] Add missing return inside target_player wrapper

---
 piqueserver/commands.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/piqueserver/commands.py b/piqueserver/commands.py
index cbf7d60fe..a0da5f010 100644
--- a/piqueserver/commands.py
+++ b/piqueserver/commands.py
@@ -293,7 +293,7 @@ def _decorated(connection, *args, **kwargs):
         # console or irc invokers are required to provide a target
         else:
             raise ValueError("Target player is required")
-        func(connection, *args, **kwargs)
+        return func(connection, *args, **kwargs)
     return _decorated
 
 def get_player(protocol, value: str, spectators=True):

From f55303e8e35dd80647cec05639fad3c9b4d9bcce Mon Sep 17 00:00:00 2001
From: Joseph Daniel <gjdaniel1999@gmail.com>
Date: Sat, 17 Aug 2019 11:05:19 +0530
Subject: [PATCH 9/9] Use `is` for identity checks

---
 piqueserver/core_commands/player.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/piqueserver/core_commands/player.py b/piqueserver/core_commands/player.py
index 02bf09d9c..fef8801ce 100644
--- a/piqueserver/core_commands/player.py
+++ b/piqueserver/core_commands/player.py
@@ -70,7 +70,7 @@ def heal(connection, player):
     Heal and refill yourself or a given player and inform everyone on the server of this action
     /heal [player]
     """
-    if player == connection:
+    if connection is player:
         message = '%s was healed' % (connection.name)
     else:
         message = '%s was healed by %s' % (player.name, connection.name)