Discoknights game structure

I'm building a game with Pygame, a simple turn-based strategy game where on each turn a character can move and/or perform an action. I have a somewhat good structure for the game on the background, and pygame provides the gui for that. Below is the code of the main file. I think the structure of main.py is quite terrible at the moment. The question is then, how do I alter that (including the main loop) to a better one. Also, what can I do to improve the performance?

Here's everything else if you need to take a look at the background logic.

```
def map_to_screen(x,y, offset_x=0, offset_y=0):
screen_x = (x - y) * (tile_w / 2) + offset_x
screen_y = (x + y) * (tile_h / 2) + offset_y
return screen_x, screen_y

def screen_to_map(x,y, offset_x=0, offset_y=0):
x -= ( offset_x + tile_w / 2 )
y -= offset_y
x = x / 2
map_x = (y + x)/(tile_h)
map_y = (y - x)/(tile_h)
if map_x 10:
opacity = 255 - 12 * count
else:
opacity = 255

#surface.blit(map_surface, (map_offset_x + map_fix_x, map_offset_y))
blit_alpha(surface, text_surface, location, opacity)

count += 1
if count > 20:
count = 0
return count, None

return count, text_surface

def get_effect_text(action):
if action.type == Action.HEAL:
text = "+" + str(action.strength)
color = (10, 200, 10)
else:
text = "-" + str(action.strength)
color = (200, 10, 10)

text_surface = med_font.render(text, False, color)
return text_surface

def blit_alpha(target, source, location, opacity):
'''Blits opaque element while keeping per pixel alpha in other parts of the surface.'''
x = location[0]
y = location[1]
temp = pygame.Surface((source.get_width(), source.get_height())).convert_alpha()
temp.blit(target, (-x, -y))
temp.blit(source, (0, 0))
temp.set_alpha(opacity)
target.blit(temp, location)

def blit_map(surface):
return

There's way too much code here (842 lines!) to try to give it a meaningful review. So I'll just make some general observations.

-
You could organize much of this code into methods on the classes you have. For example, the function render_characters_and_objects draws characters and objects. You could move the code for rendering a character into a render method on the Character class, and move the code for drawing an object into a render method on the MapObject class.

-
Basically, you should scrutinize every piece of code and ask the question, "would this code be clearer if it were turned into a method on a class?"

-
Code that deals with positions (like the map_to_screen function) should use position objects (possibly your Coordinates class?) instead of two numbers x and y.

-
Global variables make code hard to understand. For example, a lot of the code you posted uses on a global variable called m belonging to the Map class. These functions need to be methods on that class.

-
Again, you should scrutinize every global variable and ask the question, "does this variable really belong to an instance of some class?" For example, surely map_w, map_h, map_offset_x and map_offset_y ought to be properties of the Map instance? Could the sprites array lookups be avoided by storing a character's sprites in a property on the Character instance?

-
I wouldn't bother with collecting dirty rectangles. This adds a bunch of complexity that you don't need. Keep it simple and redraw the whole screen every frame.

-
A typical organization of game code is to give each object an update or tick method, which updates the object's internal state for one time interval, and a draw or render method that displays the object on the screen. Then in each game step the main loop just calls the update method on every object, and then the render method on every object. All the details of how each object updates and renders can be encapsulated within the object itself.

-
For example, at the moment you have the walking logic in its own loop: this means, I think, that only one character can walk at a time. It would be better for each character to have a state property; when this is set to WALKING then their update method causes them to take the next step towards the target and their render method picks the appropriate animation frame. Then you wouldn't need a separate game loop to handle the walking logic.

StackExchange Code Review Q#87533, answer score: 4