Das deutsche Python-Forum

Hallo Forum!
Ich habe ein kleines Project gestartet und habe angefangen ein eigenes roguelike zu schreiben.
Leider habe ich Probleme mit dem Algorithmus zum generieren der map.
Hier der Code:

Code: Alles auswählen

import random
import curses


class Tile:  # a `class` with tiles of the map
    def __init__(self, char, blocked):
        self.char = char
        self.blocked = blocked
        self.color = None  #  would be defined in a `function` which will render the map

class Room:  # a `class` for the rooms
    def __init__(self, x, y, w, h):
        self.x_1 = x
        self.y_1 = y
        self.x_2 = x + w
        self.y_2 = y + h
        
        self.mid()

    def mid(self):
        self.mid_x = int((self.x_1 + self.x_2) / 2) if (self.x_1 + self.x_2) % 2 == 0 else int((self.x_1 + self.x_2 + 1) / 2)
        self.mid_y = int((self.y_1 + self.x_2) / 2) if (self.y_1 + self.y_2) % 2 == 0 else int((self.y_1 + self.y_2 + 1) / 2)

    def valid(self, room):  # looks if the place where the room comes is valid
        if self.x_1 <= room.x_2 and self.x_2 >= room.x_1:
            if self.y_1 <= room.y_2 and self.y_2 >= room.y_1:
                return False

            else:
                return True
        
        else:
            return True

class Dungeon:  # a `class` for generating the dungeon
    def __init__(self, size_x, size_y, max_rooms, min_size, max_size):
        '''
        This `function` will be executed when an object will be made later.
        This will generate the dungeon
        '''
        # set the size
        self._map_y = size_y
        self._map_x = size_x

        # sets the option for the rooms
        self._max_rooms = max_rooms
        self._room_size_min = min_size
        self._room_size_max = max_size

        # sets the map
        self._map = []
        
        # fills the dungeon with walls
        for i in range(0, self._map_x):
            self._map.append([i])
            for j in range(0, self._map_y):
                self._map[j].append(Tile('#', True))

#       self._map = [[Tile('#', True)
#               for j in range(0, self._map_y)]
#           for i in range(0, self._map_x)]

        room_num = 0
        self.rooms = []

        for i in range(self._max_rooms):
            # make random width, height, ...
            w = random.randint(self._room_size_min, self._room_size_max)
            h = random.randint(self._room_size_min, self._room_size_max)
            x = random.randint(0, self._map_x)
            y = random.randint(0, self._map_y)

            # make room as an `object` of Room
            new_room = Room(x, y, w, h)
            
            failed = False
            # look if it is valid
            for other in self.rooms:
                if new_room.valid(other) == False:
                    failed = True
                    break
                
                else:
                    failed = False

            # if it is valid go on 
            if failed == False:
                # take it into the map
                self._mk_room(new_room)

                if room_num == 0:  # it is the first room
                    # take the mid coordinates as the place for the player to begin
                    self.begin = (new_room.mid_x, new_room.mid_y)

                else:  # isn't the first room ^^
                    # connect this room with the previous
                    old_x = self.rooms[room_num - 1].mid_x
                    old_y = self.rooms[room_num - 1].mid_y
                    
                    # choose randomly if horizontal or vertical should be made at first
                    if random.choice('01') == '1':  # make horizontal at first
                        self._mk_h_cooridor(old_x, new_room.mid_x, old_y)
                        self._mk_v_cooridor(old_y, new_room.mid_y, new_room.mid_x)

                    else:  # make vertical at first
                        self._mk_v_cooridor(old_y, new_room.mid_y, old_x)
                        self._mk_h_cooridor(old_x, new_room.mid_x, new_room.mid_y)      

            # append it to the list
            self.rooms.append(new_room)
            room_num += 1

    
    def draw(self, stdscr):  # `function` to draw the dungeon
        for x in range(0, self._map_x):
            for y in range(0, self._map_y):
                stdscr.addstr(y, x, self._map[x][y].char, self._map[y][x].color)

    def _mk_h_cooridor(self, x_1, x_2, y):  # `function` for making horizontal cooridors
        for i in range(min(x_1, x_2), max(x_1, x_2) + 1):
            self._map[i][y].char = random.choice('.,')
            self._map[i][y].blocked = False

    def _mk_v_cooridor(self, y_1, y_2, x):  # `function` for making vertical cooridors
        for j in range(min(y_1, y_2), max(y_1, y_2) + 1):
            self._map[x][j].char = random.choice('.,')
            self._map[x][j].blocked = False

    def _mk_room(self, room):  # `function` for making a room
        for i in range(room.x_1 + 1, room.x_2):
            for j in range(room.y_1 + 1, room.y_2):
                self._map[i][j].char = random.choice('.,')
                self._map[i][j].blocked = False

Immer wenn ich ihn ausführe bekomme ich einen ``IndexError``
Hier nochmal die Fehlermeldung:

Code: Alles auswählen

Traceback (most recent call last):
  File "main.py", line 214, in <module>
    render()
  File "main.py", line 116, in render
    dungeon = Dungeon(MAP_X, MAP_Y, NUM_ROOMS, ROOM_MIN, ROOM_MAX)
  File "/home/lambda/git/dwarfs-roguelike/game/data/dungeon.py", line 62, in __init__
    self._map[j].append(Tile('#', True))
IndexError: list index out of range

Ich habe bereits versucht statt einer ``list`` eine ``dict`` zu nehmen aber es kam wie erwartet ein ``AttributError``.

PS: Ich habe erst in diesem Jahr mit programieren angefangen also würde ich mich auch um ein bischen Hilfe/Verbesserungsvorschläge freuen.

MfG

@hexflame: i und j sind auch sehr ähnlich, da kann es schonmal zu Verwechslungen kommen:

Code: Alles auswählen

        self._map = []
        
        # fills the dungeon with walls
        for i in range(self._map_x):
            self._map.append([])
            for j in range(self._map_y):
                self._map[i].append(Tile('#', True))

@hexflame: Wobei man sich dieses Problem auch sparen kann wenn man keine leere Liste an `_map` anhängt, sondern erst die Liste füllt und sie dann erst anhängt. Dann braucht man keinen Indexzugriff und weder `i` noch `j` als Namen weil man dann einfach `_` für beide Namen verwenden kann.

Die Kommentare bei den Klassendefinitionen sind total überflüssig. Die liefern keinen wirklichen Mehrwert über die ``class``-Anweisung.

`Room.mid()` definiert Attribute ausserhalb von `__init__()`, die zudem noch redundant sind. Das könnte man als `property()` ausdrücken. Die Formel sollte man zudem ohne das ``if``/``else`` hinbekommen können, zum Beispiel mit `math.ceil()` oder durch addieren von 0.5.

Hallo Forum
Danke für die schnelle Hilfe.
@BlackJack kannst du mir das mit dem ``property()``
erklären weil ich keine Ahnung habe was das ist

.

Wie gesagt, ich bin sehr neu in dem Gebiet ^^.

Letzendlich habe ich herausgefunden das es an den Variablen liegt wenn da ein ``IndexError`` rausgegeben wird.
Ich habe deshalb in den Funktionen ``_mk_room()``
``_mk_h_cooridor()`` und ``_mk_v_cooridor`` ``try:`` und ``except IndexError:`` eingefügt.
Ich wette es gibt eine elegantere Lösung

.
Ich werde auch versuchen die Idee mit dem "erst füllen, dann einhängen" einzusetzen.

MfG

@hexflame: Ich denke das ist keine gute Idee einfach `IndexError` zu ”behandeln” von denen man nicht weiss warum sie auftreten. Denn Sirius3 hat ja den einen erklärt und wenn Du den jetzt einfach mit einem `IndexError` ignorierst, statt den Fehler zu beheben, wird es schwierig Programmierfehler zu finden. Versuche lieber Indexvariablen zu vermeiden. Das geht in vielen Fällen in Python recht einfach. Und Du machst da eindeutig zu viel mit irgendwelchen Indexvariablen.

Beim Berechnen von `mid_y` hast Du einen Fehler gemacht.

`valid()` könnte einen besseren Namen haben. Einen der beschreibt was da getestet wird. Funktionen mit ``if``/``else``-Ausdrücken die je nach Bedingung literal `True` oder `False` zurückgeben, sind unnötig umständlich. Die Bedingung ergibt ja schon einen Wahrheitswert. Den kann man auch direkt (oder negiert) als Rückgabwert verwenden.

`size_x`, `size_y`, `_map_x`, und `_map_y` sollten besser `width` und `height` heissen. Denn dafür stehen die Werte. Beides braucht nicht an das Objekt gebunden werden. Sollte es wahrscheinlich noch nicht einmal wenn man es brauchen sollte, denn die Werte sind redundant — man kann sie aus `_map` ermitteln.

Auch bei den Raumparametern zum erstellen sehe ich nicht warum die an das Objekt gebunden werden müssten.

Wie schon in einem Kommentar von Dir angedeutet kann man `_map` mit einer „list comprehension“ erstellen.

`failed` ist umständlich ermittelt. Der `else`-Zweig ist überflüssig. Und mit `any()` oder `all()` und einem Generatorausdruck lässt sich das *viel* kompakter und verständlicher ausdrücken.

Wahrheitswertswerte sollte man in Bedingungen auch nicht mit literalen Wahrheitswerten vergleichen. Das ergibt ja nur wieder einen Wahrheitswert. Zudem ist ``if failed == False:`` für mich auch ein wenig verwirrend. Ich muss da erst drüber nachdenken ob es wirklich das bedeutet was ich als erstes vermutet habe. ``if not failed:`` wäre deutlich einfacher zu verstehen.

Auch wenn der Test für einen Raum `failed`, wird bei Dir der Raum an `rooms` angehängt. Und `room_num` wird erhöht. Da lauert auch schon der nächste `IndexError`.

`room_num` ist überflüssig. Zum einen die Sonderbehandlung wenn `room_num` 0 ist, denn das könnte man auch einfach nach die Schleife verlegen und sich den Mittelpunkt des ersten Raumes in `self.rooms` geben lassen. Zum anderen zum ermitteln des vorherigen Raumes, denn den bekommt man statt mit ``room_num - 1`` auch ganz einfach nur mit -1 als Index.

Das `make` in den Methodennamen solltest Du nicht abkürzen.

Warum ``random.choice('01')``? Da würde sich eine Wahl zwischen `True` und `False` viel eher anbieten.

Bei `draw()` hast Du's dann wieder mit den Indexwerten verbockt. Das Zeichen holst Du über [x][y] und die Farbe über [y][x]. Das passt nicht zusammen. Hier sollte man mit `enumerate()` arbeiten um die Koordinaten für `curses` zu bekommen und ansonsten nicht mit Indexwerten in `_map` operieren.

`stdscr` ist ein kryptischer und nicht zwingend passender Name. `screen` wäre viel lesbarer und trifft auch noch zu falls man nicht den Standardscreen verwendet sondern ein Fenster oder ein ”off screen”-Ziel.

Ich lande dann als (untetestes) Zwischenergebnis hierbei:

Code: Alles auswählen

import math
import random
from random import randint


class Tile:
    def __init__(self, char, blocked):
        self.char = char
        self.blocked = blocked
        self.color = None


class Room:

    def __init__(self, x, y, width, height):
        self.x_1 = x
        self.y_1 = y
        self.x_2 = x + width
        self.y_2 = y + height

    @property
    def center_x(self):
        return int(math.ceil((self.x_1 + self.x_2) / 2))

    @property
    def center_y(self):
        return int(math.ceil((self.y_1 + self.y_2) / 2))
 
    @property
    def center(self):
        return (self.center_x, self.center_y)

    def overlaps(self, other):
        return (
            self.x_1 <= other.x_2 and self.x_2 >= other.x_1
            and self.y_1 <= other.y_2 and self.y_2 >= other.y_1
        )


class Dungeon:

    def __init__(self, width, height, max_rooms, min_room_size, max_room_size):
 
        self._map = [
            [Tile('#', True) for _ in range(width)] for _ in range(height)
        ]
        self.rooms = []
        for _ in range(max_rooms):
            new_room = Room(
                randint(0, width),
                randint(0, height),
                randint(min_room_size, max_room_size),
                randint(min_room_size, max_room_size),
            )
            if not any(room.overlaps(new_room) for room in self.rooms):
                self._make_room(new_room)

                if self.rooms:
                    # connect this room with the previous
                    old_x, old_y = self.rooms[-1].center
                    new_x, new_y = new_room.center
                   
                    # choose randomly if horizontal or vertical
                    # should be made at first
                    if random.choice([True, False]):  # make horizontal at first
                        self._make_horizontal_corridor(old_x, new_x, old_y)
                        self._make_vertical_corridor(old_y, new_y, new_x)
                    else:  # make vertical at first
                        self._make_vertical_corridor(old_y, new_y, old_x)
                        self._make_horizontal_corridor(old_x, new_x, new_y)      

                self.rooms.append(new_room)
 
        self.start_coordinate = self.rooms[0].center
 
    def draw(self, screen):
        for y, row in enumerate(self._map):
            for x, tile in enumerate(row):
                screen.addstr(y, x, tile.char, tile.color)
 
    def _make_horizontal_corridor(self, x_1, x_2, y):
        for i in range(min(x_1, x_2), max(x_1, x_2) + 1):
            self._map[i][y].char = random.choice('.,')
            self._map[i][y].blocked = False
 
    def _make_vertical_corridor(self, y_1, y_2, x):
        for j in range(min(y_1, y_2), max(y_1, y_2) + 1):
            self._map[x][j].char = random.choice('.,')
            self._map[x][j].blocked = False
 
    def _make_room(self, room):
        for i in range(room.x_1 + 1, room.x_2):
            for j in range(room.y_1 + 1, room.y_2):
                self._map[i][j].char = random.choice('.,')
                self._map[i][j].blocked = False

Die `_make_*()`-Methoden stimmen nicht weil dort noch X und Y vertauscht sind. Wenn man eine 2D-Struktur mit Listen aufbaut ist es wesentlich sinnvoller als ersten Index Y zu verwenden, denn dann ist die Struktur — eine Liste mit Zeilenlisten wesentlich eingängiger und `draw()` ist so ohne Indexzugriffe in die 2D-Struktur lösbar, wie man oben sehen kann.

Das deutsche Python-Forum

Hilfe bei einem ``IndexError``

Hilfe bei einem ``IndexError``

Re: Hilfe bei einem ``IndexError``

Re: Hilfe bei einem ``IndexError``

Re: Hilfe bei einem ``IndexError``

Re: Hilfe bei einem ``IndexError``