Small "Grid Battle" console game

This is my first attempt to make a game on my own. I just want to see if anyone has any comments. Am I doing things, at least, sort of alright? Is there a better way I could deal with the input?

```
import sys, platform, os
import time, random, math

class Entity(object):
'''Basic entity object
Has position and character'''
def __init__(self, name, x, y, hp, char, state):
self.name = name
self.x = x
self.y = y
self.hp = hp
self.char = char
self.state = state

def move(self, direction, board):
'''To update entity position'''

if direction == 'left' and board.is_valid_move(self, (self.x, self.y - 1)):
self.y -= 1
elif direction == 'right' and board.is_valid_move(self, (self.x, self.y + 1)):
self.y += 1
elif direction == 'up' and board.is_valid_move(self, (self.x - 1, self.y)):
self.x -= 1
elif direction == 'down' and board.is_valid_move(self, (self.x + 1, self.y)):
self.x += 1

def attack(self, target):
target.hp -= 1

def distance(self, target, pos=()):
'''Get distance from self to target'''
if not pos:
x, y = self.x, self.y
else:
x, y, = pos
return math.sqrt((x - target.x)2 + (y - target.y)2)

def move_toward(self, target, board, pos=()):
'''Moves entity towards target'''
move_dict = {
'right' : self.distance(target, (self.x, self.y + 1)),
'left' : self.distance(target, (self.x, self.y - 1)),
'up' : self.distance(target, (self.x - 1, self.y)),
'down' : self.distance(target, (self.x + 1, self.y))
}

small_index = move_dict.values().index(min(move_dict.values()))
small_key = move_dict.keys()[small_index]

self.move(small_key, board)

def change_state(self, state):
'''Change Entity state. Only for death right now.'''
self.state = state

What you have here looks good. It's clean and readable for the most part. Seems to work well, but I have some thoughts anyway.

In your Entity object, you have a state object for characters being alive or dead. Maybe you plan to extend this later, but right now it looks like it'd make more sense to have a value that's just self.alive = True. Then you can set it to False later, to indicate death. If you're doing this, there's no need to requite a parameter. You can just always set the initial value as True:

def __init__(self, name, x, y, hp, char):
    self.alive = True

If you do extend it, then I would firstly use constants as more reliable than strings:

class STATE:
    ALIVE = "alive"
    DEAD = "dead"

and I would still default it to being STATE.ALIVE, but you can use optional function paramaters to allow the user to supply it when they want, like this:

def __init__(self, name, x, y, hp, char, state=STATE.ALIVE):
    self.state = state

Why is your move described this way:

def move(self, direction, board):
    '''To update entity position'''

That indicates it's specific to an enemy, not a general Entity. Since the player is also an Entity this makes little sense. Instead, the Enemy class should inherit from Entity and add this Enemy specific function.

Also move doesn't in any way indicate if no movement occurs. Shouldn't you have an else case that somehow indicates movement was unnsuccesful? Eg. else: return False could then be used to check if movement actually occured. As it stands, if the enemy's move isn't valid it will just remain still.

In attack you hardcode 1 as the damage that target takes, but what if you want to add difficulty that will change this? You should make a strength attribute and use it here so that each individual Entity can have its own strength.

In distance you could shorten your if else with a ternary. A ternary's basically an expression that will resolve to one of two values based on a boolean evaluation. Here's how yours could look:

def distance(self, target, pos=()):
    '''Get distance from self to target'''

    x, y = pos if pos else (self.x, self.y)
    return math.sqrt((x - target.x)**2 + (y - target.y)**2)

If pos evaluates as True then it will use pos, if it's False then self.x, self.y are used.

move_toward is quite a confusing set up. I think you made it overly complicated by making it a dictionary. Instead, make a list of 2 value tuples. If you make the first value be the result of self.distance then you can just sort this list afterwards and the first (ie. lowest) value will be the one you move towards. Here's what I mean:

def move_toward(self, target, board, pos=()):
    '''Moves entity towards target'''
    possible_moves = (
        (self.distance(target, (self.x, self.y + 1)), 'right'),
        (self.distance(target, (self.x, self.y - 1)), 'left'),
        (self.distance(target, (self.x - 1, self.y)), 'up'),
        (self.distance(target, (self.x + 1, self.y)), 'down')
    )
    # Sort and take the first, ie. lowest, value.
    small_key = sorted(possible_moves)[0]

    self.move(small_key, board)

Whether you end up making state be a boolean, string or saved constant, there's little point in makign a function for this. It might be needed if you have complicated machinations in mind (eg. if an Entity can be poisoned but it's contextual and needs to be calculated). But at the moment it's just a simple attribute setting function. Python doesn't bother with those, let people set the value directly with player.state = "dead".

update doesn't need pass in your first if block, it doesn't do anything. Also, since you evaluate the same expression twice, consider nesting them instead:

def update(self, board):
    '''Update Entity, check hp, xp, ect.'''

    if self.hp <= 0:
        if self.state == 'dead':
            time.sleep(1)
        else:
            self.change_state('dead')
            self.char = '%'

In Board, using width and height would be easier to understand names than x_size and y_size. Also you can save some space in __init__ by making your inner list with multiplication:

self.board = [[char] * x_size for _ in range(y_size)]

I'd also suggest having '-' be a class constant as you refer to it multiple times later.

You should rearrange how you print your board. For a start you could make a __str__ function, then you could just call print(Board()) and have it print properly to screen. To make that work, instead of printing in the function you'll need to build a string and return it. But that doesn't require much modification as I'll show below.

But also, you have the board characters being updated in their own separate function even though you're unlikely to ever need to do that separate to a print call. Your mileage may vary, but personally I'd put them together in one function.

There's no reas

StackExchange Code Review Q#110371, answer score: 5