Setting the scene for a role-playing game

I made some games in Pygame, but I am still new to making games.
I made this RPG game, and this code is just a part of the game. The game executes different Pygame files for different levels of the game as it can be seen in this part of the code (this is for random battles):

if (mainCharacterCoordinates.colliderect(battlefield) and (keys_pressed[pygame.K_DOWN] or keys_pressed[pygame.K_UP] or keys_pressed[pygame.K_RIGHT] or keys_pressed[pygame.K_LEFT])):
    f = open('x.txt', 'w')
    f.write(str(x))
    f.close()

    f = open('y.txt', 'w')
    f.write(str(y))
    f.close()
    execfile('battlefield.py')
    Exit = True

This is a screenshot of the game:

My concern about this code is whether my way of coding is efficient.
At this moment, I am mostly concerned about two things in my game.
I am not sure I am doing the collision the correct way (they still work, but it looks bad in the code as it can be seen below).
I created 4 rectangles at left,right,top and bottom of the main character and each object has just one rectangle that is the size of the objects.
After this I tell the program if there is collision on the left and do not walk left and the same for other 3 sides of the main character.
Would it be possible to give me and tips if there is a better way of creating collisions?
So that the 'mainCharacter' cannot walk over an object.

```
if (keys_pressed[pygame.K_LEFT] and not mainCharacterCoordinatesLeft.colliderect(treeCoordinates8) and not mainCharacterCoordinatesLeft.colliderect(treeCoordinates7) and not mainCharacterCoordinatesLeft.colliderect(treeCoordinates6) and not mainCharacterCoordinatesLeft.colliderect(treeCoordinates5) and not mainCharacterCoordinatesLeft.colliderect(treeCoordinates4) and not mainCharacterCoordinatesLeft.colliderect(treeCoordinates3) and not mainCharacterCoordinatesLeft.colliderect(treeCoordinates2) and not mainCharacterCoordinatesLeft.colliderect(treeCoordinates1) and not mainCharacterCoordinatesLeft.colliderect(crossCoo

crossImage1 = pygame.image.load('cross.png')
crossImage2 = pygame.image.load('cross.png')
crossImage3 = pygame.image.load('cross.png')
crossImage4 = pygame.image.load('cross.png')
crossImage5 = pygame.image.load('cross.png')
crossImage6 = pygame.image.load('cross.png')
crossImage7 = pygame.image.load('cross.png')
crossImage8 = pygame.image.load('cross.png')

This is bad practice on so many levels.

• You have a bunch variables named like myNameX; always a bad sign.

• These variables are all equal to the same thing.

Now, I would go through how to fix these individually, but looking at how you are using these variables, there is no need. Again, each of these variables all contains the same exact image. And, the only time you are using these variables is to print the image out. Here is an example of one place:

gameDisplay.blit(treetopImage1,( -1200 - CameraX , 269 - CameraY))
gameDisplay.blit(treetopImage2,( -800 - CameraX , 469 - CameraY))
gameDisplay.blit(treetopImage3,( -400 - CameraX , 369 - CameraY))
gameDisplay.blit(treetopImage4,(  -600 - CameraX , 369 - CameraY))
gameDisplay.blit(treetopImage5,( 100 - CameraX , 269 - CameraY))
gameDisplay.blit(treetopImage6,( 500 - CameraX , 569 - CameraY))
gameDisplay.blit(treetopImage7,( -1000 - CameraX , 469 - CameraY))
gameDisplay.blit(treetopImage8,( -300 - CameraX , 669 - CameraY))

Why do you need 8 different variables holding the same image to do this? Why can you not just use a single variable? You are wasting so much space and time to store these and the way you are using them is super overkill.

All you really need is love one:

crossImage = pygame.image.load('cross.png')

...

gameDisplay.blit(crossImage, ...)
gameDisplay.blit(crossImage, ...)

The same can be applied to those similar series of repetitive image variables, and simplifies the rest of your code too.

treeCoordinates1 = pygame.Rect((-1200,300),(30,31))
treeCoordinates2 = pygame.Rect((-800,500),(30,31))
treeCoordinates3 = pygame.Rect((-400,400),(30,31))
treeCoordinates4 = pygame.Rect((-600,400),(30,31))
treeCoordinates5 = pygame.Rect((100,300),(30,31))
treeCoordinates6 = pygame.Rect((500,600),(30,31))
treeCoordinates7 = pygame.Rect((-1000,500),(30,31))
treeCoordinates8 = pygame.Rect((-300,700),(30,31))

Again. These chains of variables with numbers at the end of them. Typically, when you have variables named like this, ya dun goofed.

Instead of keeping a bunch of variables like this, you should be storing them in an array (or in this case, a tuple).

treeCoordinates = (pygame.Rect(...), pygame.Rect(...))

This will really simplify your code in a lot of places. For example, inthosereallylongifstatementswhereyouarecheckingwhichkeywaspressed, rather than going through each coordinate in the conditionals, you can simplify it with a loop:

for treeCoordinate in treeCoordinates

This will make your code a lot easier on the eye (and could made even better if incorporated into a comprehension).

if (x = 660 and):
    graveyard = pygame.image.load('graveyard right.png')
if(y = 380 and ):
    graveyard = pygame.image.load('graveyard front.png')

What's with these floating ands? If you don't have a second clause, then you aren't to put an and down; it's just plain wrong.

StackExchange Code Review Q#118703, answer score: 12