back end code review - TODO: all the TODOs we just added

client/world_view.py

@@ -3,15 +3,3 @@ def __init__(self, my_avatar, map_centred_at_me, avatar_manager):
         self.my_avatar = my_avatar
         self.map_centred_at_me = map_centred_at_me
         self.avatar_manager = avatar_manager
-
-    def get_my_avatar(self):
-        return self.my_avatar
-
-    def get_my_location(self):
-        return self.get_my_avatar().location
-
-    def get_my_health(self):
-        return self.get_my_avatar().health
-
-    def get_map_centred_at_me(self):
-        return self.map_centred_at_me

simulation/action.py

@@ -11,13 +11,12 @@ def __init__(self, direction):
         self.direction = direction
 
     def apply(self, world_state, avatar):
-        current_location = avatar.location
-        target_location = current_location + self.direction
+        target_location = avatar.location + self.direction
         if world_state.world_map.can_move_to(target_location):
             avatar.add_event(MovedEvent(avatar.location, target_location))
             avatar.location = target_location
         else:
-            avatar.add_event(MovedEvent(avatar.location, target_location))
+            avatar.add_event(FailedMoveEvent(avatar.location, target_location))
 
 
 class AttackAction(Action):
@@ -30,9 +29,8 @@ def apply(self, world_state, avatar):
         if attacked_avatars:
             for other_avatar in attacked_avatars:
                 damage_dealt = 1
-                avatar.add_event(PerformedAttackEvent(
-                    other_avatar, target_location, damage_dealt))
-                other_avatar.add_event(
-                    ReceivedAttackEvent(avatar, damage_dealt))
+                avatar.add_event(PerformedAttackEvent(other_avatar, target_location, damage_dealt))
+                other_avatar.add_event(ReceivedAttackEvent(avatar, damage_dealt))
+                # TODO: actually damage them in the world state
         else:
             avatar.add_event(FailedAttackEvent(target_location))

simulation/avatar.py

@@ -2,13 +2,18 @@
 from simulation import direction
 
 
+# This class will be implemented by the player
+Avatar = None
+
+
 class AvatarRunner(object):
     def __init__(self, initial_location, initial_code, player_id, avatar_appearance):
         self.location = initial_location
         self.events = []
         self.player_id = player_id
         self.set_code(initial_code)
         self.avatar_appearance = avatar_appearance
+        self.avatar = None
 
     def handle_turn(self, state):
         next_action = self.avatar.handle_turn(state, self.events)

simulation/direction.py

@@ -1,5 +1,6 @@
+# TODO: investigate using x and y
 class Direction:
-    def __init__(self, row=0, col=0):
+    def __init__(self, row, col):
         self.row = row
         self.col = col
 

simulation/level.py

@@ -1,26 +1,3 @@
 import random
 import numpy as np
 
-
-class Level:
-    def __init__(self, height, width, obstacle_ratio, scoring_square_ratio):
-        self.matrix_of_level = np.empty((height, width), dtype=object)
-
-        for y in xrange(height):
-            for x in xrange(width):
-                if random.random() < obstacle_ratio:
-                    self.matrix_of_level[y, x] = OBSTACLE
-                elif random.random() < scoring_square_ratio:
-                    self.matrix_of_level[y, x] = SCORE
-                else:
-                    self.matrix_of_level[y, x] = EMPTY
-
-
-class SquareType:
-    def __init__(self, name, key):
-        self.name = name
-        self.key = key
-
-EMPTY = SquareType("empty", 0)
-OBSTACLE = SquareType("obstacle", 1)
-SCORE = SquareType("score", 2)

simulation/location.py

@@ -1,5 +1,6 @@
+# TODO: investigate using x and y
 class Location(object):
-    def __init__(self, row=0, col=0):
+    def __init__(self, row, col):
         self.row = row
         self.col = col
 

simulation/test/test_turn_manager.py

@@ -5,7 +5,6 @@
 sys.path.append(os.path.abspath('.'))
 
 import unittest
-from simulation.level import Level
 
 from simulation.location import Location
 from simulation.turn_manager import TurnManager
@@ -27,9 +26,6 @@ class TestTurnManager(unittest.TestCase):
     def construct_default_avatar_appearance(self):
         return AvatarAppearance("#000", "#ddd", "#777", "#fff")
 
-    def construct_default_level(self):
-        return Level(15, 15, 0.1, 0.1)
-
     def construct_turn_manager(self, *avatars):
         self.avatar_manager = AvatarManager(avatars)
         self.world_state = WorldState(InfiniteMap(), self.avatar_manager)

simulation/turn_manager.py

@@ -3,6 +3,7 @@
 
 
 class WorldStateProvider:
+    """TODO: think about changing to snapshot rather than lock?"""
     def __init__(self):
         self._world_state = None
         self._lock = Lock()
@@ -30,15 +31,16 @@ def _update_environment(self):
         pass
 
     def run_turn(self):
-        world_state = world_state_provider.lock_and_get_world()
+        try:
+            world_state = world_state_provider.lock_and_get_world()
 
-        self._update_environment()
+            self._update_environment()
 
-        actions = [(p, p.handle_turn(world_state.get_state_for(p))) for p in world_state.avatar_manager.avatarsById.values()]
-        for avatar, action in actions:
-            action.apply(world_state, avatar)
-
-        world_state_provider.release_lock()
+            actions = [(p, p.handle_turn(world_state.get_state_for(p))) for p in world_state.avatar_manager.avatarsById.values()]
+            for avatar, action in actions:
+                action.apply(world_state, avatar)
+        finally:
+            world_state_provider.release_lock()
 
     def run_game(self):
         while True:

simulation/world_map.py

@@ -1,28 +1,56 @@
+import random
 import numpy as np
-from simulation import level
 from simulation.direction import Direction
 
 
+class SquareType:
+    def __init__(self, name, key):
+        self.name = name
+        self.key = key
+
+EMPTY = SquareType("empty", 0)
+OBSTACLE = SquareType("obstacle", 1)
+SCORE = SquareType("score", 2)
+
+
+def generate_map(height, width, obstacle_ratio, scoring_square_ratio):
+    matrix_of_level = np.empty((height, width), dtype=object)
+
+    for y in xrange(height):
+        for x in xrange(width):
+            if random.random() < obstacle_ratio:
+                matrix_of_level[y, x] = OBSTACLE
+            elif random.random() < scoring_square_ratio:
+                matrix_of_level[y, x] = SCORE
+            else:
+                matrix_of_level[y, x] = EMPTY
+
+    return WorldMap(matrix_of_level)
+
+
+# TODO: investigte switch from numpy to 2d lists to avoid making users know numpy and having to install it
 class WorldMap(object):
-    def __init__(self, level):
-        self.grid = level
+    def __init__(self, grid):
+        self.grid = grid
 
+    # TODO: cope with negative coords (here and possibly in other places
     def can_move_to(self, target_location):
         num_rows, num_cols = self.grid.shape
         return (
             (0 <= target_location.row < num_rows)
             and (0 <= target_location.col < num_cols)
-            and self.grid[target_location.row, target_location.col] != level.OBSTACLE
+            and self.grid[target_location.row, target_location.col] != OBSTACLE
         )
 
+    # TODO: switch to always deal in fixed coord space rather than floating origin
     def get_world_view_centred_at(self, view_location, distance_to_edge):
         """
                        world map = self.grid
         +-----------------------------------------------+
         |                                               |
         |                                               |
         |                                               |
-        |                + map_corner                   |
+        |                + view_map_corner              |
         |                |                              |
         |                v                              |
         |                     view map                  |   |
@@ -45,23 +73,23 @@ def get_world_view_centred_at(self, view_location, distance_to_edge):
 
         """
         num_grid_rows, num_grid_cols = self.grid.shape
-        num_view_rows, num_view_cols = 2 * distance_to_edge + 1, 2 * distance_to_edge + 1
+        view_diameter = 2 * distance_to_edge + 1
 
         view_map_corner = view_location - Direction(distance_to_edge, distance_to_edge)
 
         # Non-cropped indices
         row_start = view_map_corner.row
-        row_exclusive_end = row_start + num_view_rows
+        row_exclusive_end = row_start + view_diameter
 
         col_start = view_map_corner.col
-        col_exclusive_end = col_start + num_view_cols
+        col_exclusive_end = col_start + view_diameter
 
         # Cropped indices
         cropped_row_start = max(0, row_start)
-        cropped_row_exclusive_end = min(num_grid_rows, row_start + num_view_rows)
+        cropped_row_exclusive_end = min(num_grid_rows, row_start + view_diameter)
 
         cropped_col_start = max(0, col_start)
-        cropped_col_exclusive_end = min(num_grid_cols, col_start + num_view_cols)
+        cropped_col_exclusive_end = min(num_grid_cols, col_start + view_diameter)
 
         assert 0 <= cropped_row_start < cropped_row_exclusive_end <= num_grid_rows
         assert 0 <= cropped_col_start < cropped_col_exclusive_end <= num_grid_cols

simulation/world_state.py

@@ -9,6 +9,6 @@ def __init__(self, world_map, avatar_manager):
     def get_state_for(self, avatar):
         return WorldView(avatar, self.world_map, self.avatar_manager)
 
+    # TODO: move to avatar_manager
     def get_avatars_at(self, location):
-        return [p for p in self.avatar_manager.avatars
-                if p.location == location]
+        return [a for a in self.avatar_manager.avatars if a.location == location]

ui/players/api_watch.py

@@ -1,8 +1,10 @@
-import random
 from django.http import JsonResponse
 from simulation.turn_manager import world_state_provider
 
 
+# TODO: rename to something more djangoey or merge with views.py
+
+
 def get_world_parameters(request):
     world = world_state_provider.lock_and_get_world()
     try:

ui/players/models.py

@@ -7,5 +7,6 @@ class Player(models.Model):
     code = models.TextField()
 
 
+# TODO: switch code back  to Avatar from Player to support players undoing their code changes
 class Avatar(models.Model):
     player = models.ForeignKey(User)

ui/players/static/js/program.js

@@ -49,11 +49,11 @@ $( document ).ready(function() {
             data: {code: editor.getValue(), csrfmiddlewaretoken: $('#saveForm input[name=csrfmiddlewaretoken]').val()},
             success: function(data) {
                 setButtonsEnabled(false);
+                $('#alerts').hide();
             },
             error: function(jqXHR, textStatus, errorThrown) {
                 showAlert('An error has occurred whilst saving: ' + errorThrown);
-
-            }       
+            }
         });
     });
 });

ui/players/templates/players/watch.html

@@ -22,8 +22,7 @@
 
     function refreshWorld() {
         console.log("Refreshing world...");
-        jQuery.ajax("/api/watch/world", {
-            ____timeout : 500,
+        $.ajax("/api/watch/world", {
             success : function(data) {
                 CONTROLS.initialiseWorld(data.width, data.height, data.layout);
                 console.log("World refresh successful.");
@@ -39,12 +38,10 @@
     }
 
     function refreshState() {
-        jQuery.ajax("/api/watch/state", {
-            complete : function(jqXHR, textStatus) {
-                setTimeout(refreshState, 100);
-            },
+        $.ajax("/api/watch/state", {
             success : function(data) {
                 CONTROLS.setState(data.players, data.items);
+                setTimeout(refreshState, 100);
             },
             error : function(jqXHR, textStatus, errorThrown) {
                 console.error("State refresh failed.", jqXHR, textStatus, errorThrown);

ui/players/views.py

@@ -4,14 +4,13 @@
 from django.contrib.auth import authenticate, login
 from threading import Thread
 from simulation.avatar_manager import AvatarManager
-from simulation.level import Level
 from simulation.turn_manager import TurnManager
 from simulation.turn_manager import world_state_provider
-from simulation.world_map import WorldMap
+from simulation import world_map
 from simulation.world_state import WorldState
 import logging
 from django.contrib.auth.forms import UserCreationForm
-from players.models import Player
+from models import Player
 
 
 INITIAL_CODE = '''from simulation.action import MoveAction
@@ -23,9 +22,10 @@ def handle_turn(self, world_state, events):
         import random
         directions = (direction.EAST, direction.SOUTH, direction.WEST, direction.NORTH)
         return MoveAction(random.choice(directions))
-
 '''
 
+# TODO: move all views that just render a template over to using django generic views
+
 logger = logging.getLogger("views")
 
 
@@ -49,10 +49,9 @@ def register(request):
 
 def run_game():
     print "Running game..."
-    level = Level(15, 15, 0.1, 0.1)
-    world_map = WorldMap(level.matrix_of_level)
+    my_map = world_map.generate_map(15, 15, 0.1, 0.1)
     player_manager = AvatarManager([])
-    world_state = WorldState(world_map, player_manager)
+    world_state = WorldState(my_map, player_manager)
     turn_manager = TurnManager(world_state)
 
     turn_manager.run_game()
@@ -69,6 +68,7 @@ def program(request):
     return render(request, 'players/program.html')
 
 
+# TODO: on exception when submitting code, catch it and return response with details (so players can debug their code)
 @login_required
 def code(request):
     if request.method == 'POST':