Hilfe Bug (Evtl Anfänger Fehler) ;)

[HardwareManager]

Hallo,
ich arbeite gerade an einem Programm, wo man Rechtecke "bauen" kann und wenn es sich mit einem anderen Rechteck überschneidet wird es Rot gezeichnet, wenn es sich nicht mit einem anderen Rechteck überschneidet ist es in einer anderen Farbe.

Hier der Code:
(Übrigens muss man einfach 2x (an unterschiedlichen stellen) ins Schwarze klicken)

Code: Alles auswählen

try:
    from tkinter import *
    print("[ OK ] Loaded tkinter")
except:
    print("[FAIL] Can not load tkinter, maybe you did not installed it or using Python 2?")
import _thread
"""
    Variablen und Infos
"""


Name = "Landnahme"
Welt = []
MainFrame = None # Main tkinter Fenster
Can = None # Main Canvas
Farben = ["gold", "blue", "yellow", "green", "grey30", "grey40", "grey50", "grey60", "grey70", "grey80", "grey90"]
startx = 0 # CanEvent
starty = 0 # CanEvent
Count = 1
"""
    Definitionen
"""

def WeltGen():
    global Welt
    x = 1200
    y = 800
    for xtemp in list(range(0, x - 1)):
        Welt += [[]]
        for ytemp in list(range(0, y - 1)):
            Welt[xtemp] += [0]
    print("[ OK ] World gen ready.")

def CanEvent2(event):
    global Can, Welt, startx, starty, Count
    Can.unbind("<Button-1>")
    endx = event.x
    endy = event.y
    
    print("[ OK ] CanEvent2 Clicked at " + str(endx) + "," + str(endy))
    erlaubniss = True
    for x in list(range(startx, endx)):
        for y in list(range(starty, endy)):
            if Welt[x][y] == 0:
                print(Welt[x][y])
            else:
                print(Welt[x][y])
                erlaubniss = False
            
    if erlaubniss == True:
        for x in list(range(startx, endx)):
            for y in list(range(starty, endy)):
                Welt[x][y] = [Count]
        Can.create_rectangle(startx, starty, endx, endy, fill=Farben[Count-1])
        Count += 1
        
    else:
        Can.create_rectangle(startx, starty, endx, endy, fill="red")
    Can.bind("<Button-1>", CanEvent)
    
    print(endx)
    print(endy)
    print(startx)
    print(starty)
    print("[ OK ] plot calculated")
    startx = 0
    starty = 0
def CanEvent(event):
    global Can, startx, starty
    Can.unbind("<Button-1>")
    startx = event.x
    starty = event.y
    Can.bind("<Button-1>", CanEvent2)
    print("[ OK ] CanEvent Clicked at " + str(startx) + "," + str(starty))


"""
    Main
"""
_thread.start_new(WeltGen, ()) # Im hintergrund Welt generieren
MainFrame = Tk()
MainFrame.resizable(width=FALSE, height=FALSE) # Fenstergroesse ist unveraenderbar
MainFrame.geometry("1200x800") # Fenstergoesse auf 1200x800 setzen
Can = Canvas(MainFrame, bg="black")
Can.place(x=0, y=0, width=1200, height=700)
Can.bind("<Button-1>", CanEvent)

MainFrame.mainloop()
print("[ OK ] Program was closed")

Mein Problem ist das manchmal einfach nicht erkannt wird das ein Rechteck sich mit einem anderen überschneidet.
Danke
Luis

[BlackJack]

@HardwareManager: Schmeiss als erstes mal den Thread da raus. Es kann ja passieren das der Benutzer rumklickt obwohl die Welt noch gar nicht fertig ist. Das `_thread`-Modul wäre hier sowieso falsch weil das nicht für Dich zum benutzen gedacht ist. Das sieht man am führenden Unterstrich. Die offizielle API ist das `threading`-Modul.

Dann schmeiss alle ``global``-Anweisungen raus und auf Modulebene sollte nur Code stehen der Konstanten, Funktionen, und Klassen definiert. Und um Klassen kommt man bei sauberer GUI-Programmierung in Python nicht herum.

Die Ausnahmebehandlung beim ``import`` sollte weg. Zum einen sollte man kein nacktes ``except:`` ohne konkrete Ausnahmen verwenden, denn das behandelt *alles*, zum Beispiel auch wenn man einen Tippfehler bei einem Namen gemacht hat, und es gehen bei der Art wie Du das behandelst selbst in dem Fall den Du eigentlich behandeln willst (`ImportError`) eventuell wichtige Informationen verloren. Die Ausnahme kann nämlich Text enthalten der einen Hinweis darauf gibt *warum* der ``import`` fehlgeschlagen ist.

Die Namensschreibweise hält sich zum grossteil nicht an den Style Guide for Python Code.

Namen oben im Quelltext an `None` zu binden ohne das dieser Wert jemals verwendet wird macht keinen Sinn. Die erste Definition von einem Namen sollte man erst dann machen wenn man etwas *sinnvolles* daran binden kann, und möglichst auch in der Nähe von dem Code der dann auch tatsächlich etwas damit anfängt. Also nicht am Anfang alle Namen binden wenn davon einige erst sehr viel weiter unten tatsächlich benötigt werden.

Man sollte auch keine Namen abkürzen wenn die Abkürzung nicht allgemein bekannt ist. `Can` sollte beispielsweise `canvas` heissen, dann weiss der Leser sofort worum es sich handelt ohne rätseln zu müssen.

Fenstergrössen sollte man nicht hart vorgeben und absolute Positionen sind bäh. Das Fenster (und andere Container-Widgets) werden automatisch so gross wie sie sein müssen wenn man die richtigen Layout-Manager (`pack()` oder `grid()`) verwendet.

``+=`` auf Listen würde ich nicht verwenden. Das macht keiner und es sieht sehr komisch aus. Und es ist auch die falsche Operation um *ein* Element an eine Liste anzuhängen. Die `WeltGen()`-Funktion könnte zudem mit einer einfachen verschachtelten „list comprehension“ ausgedrückt werden. Statt 0 würde ich `None` als ”nichts” verwenden, dann muss man `Count` auch nicht mit 1 anfangen lassen und beim Zugriff auf die Farben dann wieder 1 abziehen damit der Index stimmt.

``list(range(…)`` ist in keinem der Fälle eine gute Idee. Was soll das `list()` dort?

Um Werte in Zeichenketten zu formatieren gibt es die `format()`-Methode auf Zeichenketten. `str()` und ``+`` ist eher BASIC als Python.

Das Maus-Ereignis immer wechselseitig neu zu binden kann man machen, würde ich aber nicht. Man kann sich auch merken ob eine Startposition ausgewählt wurde in dem man diese Werte entweder mit `None` oder der entsprechenden Zahl belegt.

[pillmuncher]

Könnte man objektorientiert zB. so machen:

Code: Alles auswählen

#!/usr/bin/env python3

import tkinter as tk


invalid = ValueError(
    'Argument "box" must be an iterable of zero or four numbers.')


X0, Y0, X1, Y1 = range(4)


class Rect(tuple):

    __slots__ = ()

    def __new__(cls, box):
        try:
            box = tuple(box)
        except TypeError:
            raise invalid
        if not box:
            return tuple.__new__(cls)
        if len(box) != 4:
            raise invalid
        if box[X0] > box[X1] or box[Y0] > box[Y1]:
            return tuple.__new__(cls)
        return tuple.__new__(cls, box)

    def __and__(self, other):
        """Return the overlapping area of self and other. If they don't
        overlap, return an empty Rect."""
        xs1, ys1, xs2, ys2 = zip(self, other)
        return Rect((max(xs1), max(ys1), min(xs2), min(ys2)))

    def __or__(self, other):
        "Return the bounding box of self and other."
        xs1, ys1, xs2, ys2 = zip(self, other)
        return Rect((min(xs1), min(ys1), max(xs2), max(ys2)))


def normalize_rect_points(x1, y1, x2, y2):
    if x1 > x2:
        x1, x2 = x2, x1
    if y1 > y2:
        y1, y2 = y2, y1
    return x1, y1, x2, y2


class Model:

    def __init__(self, overlap_color, colors):
        self._overlap_color = overlap_color
        self._colors = list(reversed(colors))
        self._rects = []

    def add_rect(self, rect):
        self._rects.append(rect)

    def has_colors(self):
        return bool(self._colors)

    def dequeue_color_for_rect(self, rect1):
        if any(rect1 & rect2 for rect2 in self._rects):
            return self._overlap_color
        elif self.has_colors():
            return self._colors.pop()
        else:
            return None


class View(tk.Tk):

    def __init__(self, width, height, on_click):
        super().__init__()
        self.resizable(width=tk.FALSE, height=tk.FALSE)
        self.geometry("{}x{}".format(width, height))
        self._canvas = tk.Canvas(self, bg="black")
        self._canvas.place(x=0, y=0, width=width, height=height)
        self._canvas.bind("<Button-1>", on_click)

    def draw_rect(self, color, x1, y1, x2, y2):
        self._canvas.create_rectangle(x1, y1, x2, y2, fill=color)


class Controller:

    def __init__(self, overlap_color, *colors):
        self._last_point = None
        self._model = Model(overlap_color, colors)
        self._view = View(width=1200, height=800, on_click=self._handle_click)

    def _handle_click(self, event):
        if not self._model.has_colors():
            return
        x, y = event.x, event.y
        if self._last_point:
            rect = Rect(normalize_rect_points(x, y, *self._last_point))
            color = self._model.dequeue_color_for_rect(rect)
            self._model.add_rect(rect)
            self._view.draw_rect(color, *rect)
            self._last_point = None
        else:
            self._last_point = x, y

    def run(self):
        self._view.mainloop()


def main():
    controller = Controller(
        'red',
        'gold',
        'blue',
        'yellow',
        'green',
        'grey30',
        'grey40',
        'grey50',
        'grey60',
        'grey70',
        'grey80',
        'grey90',
    )
    controller.run()


if __name__ == '__main__':
    main()

Wenn der Benutzer konzeptionell mit Rechtecken operieren soll, dann sollten diese auch als Objekte im Programm vorkommen. Zudem kann man die Überschneidungsrelation dort bequem unterbringen.

Auch eine Aufteilung in Model, View und Controller (genannt das MVC-Pattern) ist immer sinnvoll, wenn man grafische Benutzerflächen baut. Dabei packt man die Datenstrukturen, die die Konzepte des Benutzers repräsentieren, in das Model, allen Code, der die Benutzeroberfläche modifiziert, in die View und der Controller steuert das ganze. Die View muss nichts von den zugrundeliegenden Datenstrukturen und Algorithmen (der "Geschäftslogik") wissen und das Model muss sich nicht mit Buttons, Mausklicks, Zeichenoperationen und ähnlichem herumschlagen. Model und View stellen nach außen klar definierte Schnittstellen zur Verfügung, über die sie vom Controller gesteuert werden können. Der Controller reagiert auf Nachrichten aus der View, teilt dem Model mit, wann und wie es seinen internen Zustand ändern muss, und benachrichtigt die View, was sie wann und wie darzustellen hat.

Ich gebe BlackJack recht, dass absolute Größen- und Positionsangaben in einer GUI sehr schlecht sind. Ich war aber zu faul, das auch noch zu richten.