issue with filling the correct grid cell in tkinter

Question

im trying to make a battleship game in python using tkinter, the minimum reproducible example is:

from tkinter import *
from random import randint
import time

tk = Tk()


class player:
    def __init__(self, master):
        self.master = master
        self.placedships = []
        self.bombed = []
        self.sunk = []
        self.ship_sizes = [5, 4, 3]
        self.player_canvas = Canvas(master, height=300, width=300, highlightbackground='black', highlightthickness=0.5)
        self.ai_canvas = Canvas(master, height=300, width=300, highlightbackground='black', highlightthickness=0.5)
        self.player_canvas.grid(row=1, column=0, padx=50)
        self.ai_canvas.grid(row=1, column=1, padx=50)
        self.direction = 'v'
        self.shipChosen = 0

        gridLabel_player = Label(master,text="Your grid \nA       B       C       D       E       F       G       H       I       J ")
        gridLabel_player.grid(row=0,column=0)
        gridLabel_ai = Label(master,text="AI's grid \nA       B       C       D       E       F       G       H       I       J ")
        gridLabel_ai.grid(row=0,column=1)

        for x in range(10):
            for y in range(10):
                self.player_canvas.create_rectangle(x * 30, y * 30, 300, 300, fill = 'white')
                self.ai_canvas.create_rectangle(x * 30, y * 30, 300, 300, fill = 'white')

        #variables to store data for cells on game grid
        #         # self.player_ocean = 10 * [10 * [0]]
        #         # self.ai_ocean = 10 * [10 * [0]]
        self.player_ocean = []
        self.ai_ocean = []
        temp = []
        for i in range(10):
            for y in range(10):
                temp += [0]
            self.player_ocean += [temp]
            self.ai_ocean += [temp]
            temp = []
        self.selectedCoord = [0,0] # [0] = x coord, [1] = y coord




    def placeShip(self):
        def moveShip(event):
            if event.keysym == 'Down' and self.selectedCoord[1] != 9:
                self.selectedCoord[1] += 1
            elif event.keysym == 'Up' and self.selectedCoord[1] != 0:
                self.selectedCoord[1] -= 1
            elif event.keysym == 'Left' and self.selectedCoord[0] != 0:
                self.selectedCoord[0] -= 1
            elif event.keysym == 'Right' and self.selectedCoord[0] != 9:
                self.selectedCoord[0] += 1
            print('selected coord:',self.selectedCoord)

        def selectPlacement(event):
            col = self.selectedCoord[0]
            row = self.selectedCoord[1]
            if self.direction == 'v':
                v_range_start = row - self.ship_sizes[self.shipChosen] + 1
                for y in range(v_range_start, row+1):
                    '''insert validation to reject ship clashing'''
                    self.player_ocean[y][col] = 1
                    self.placedships += [y,col]
                self.refresh_ocean()




        self.master.bind("<Up>", moveShip)
        self.master.bind("<Down>", moveShip)
        self.master.bind("<Left>", moveShip)
        self.master.bind("<Right>", moveShip)
        self.master.bind("<Return>", selectPlacement)

    def refresh_ocean(self): # 1s turns to green
        for y in range(10):
            for x in range(10):
                if self.player_ocean[y][x] == 1:
                    self.player_canvas.itemconfig(self.player_canvas.create_rectangle( (x+1) * 30, (y-1) * 30,x * 30, y * 30, fill='green'))


player1 = player(tk)
player1.placeShip()
tk.mainloop()

the problem i have is that lets say if i press the down arrow until my selected coordinate is [0,9], the code should colour the 6th to 10th box from top down, in the first column, but it colours the 5th to 9th box.

i have tried debugging it by checking if the coordinates x and y used in the last function refresh_ocean were wrong, but they were as expected

Sounds like an index vs row issue you must be doing you counts off of an index but need at + 1 it somewhere. — Mike - SMT, Oct 24 '19 at 13:54
The quick fix is to change `(y-1)` to `(y+1)`. There are some other changes I would make but this will correct your placement. — Mike - SMT, Oct 24 '19 at 14:32
Just for clarity your post is not a ***minimum reproducible example***. There is no need to have the `ai` portion as you do nothing with it in this example. That would cut out a good chunk of code. — Mike - SMT, Oct 24 '19 at 15:57

Mike - SMT · Accepted Answer · 2019-10-24T15:55:03.260

So the short and simple fix is to change (y-1) to (y+1).

That said I made a few changes:

Rewrote some of your names to follow PEP8 style guide.
Converted your refresh_ocean method to handle the initial draw and all draws afterwords. As well as added a delete('all') to clear the board so we are not draw on top of the previous objects. This will prevent using up more memory as the game progresses.
Cleaned up imports and changed from tkinter import * to import tkinter as tk. This will help prevent overwriting other things in the namespace.
By writing your refresh_ocean to handle all the drawing we can specify what needs to be drawn with tags like 'player1' and 'ai'.
Changed += to .append() for your list generation as append() is twice as fast as '+=. See this post: Difference between “.append()” and “+= []”?
Changed you single label of headers into a loop to create each label evenly. This will allow for a more accurate header then trying to manually set spaces in a single string.
Lastly because you are already checking of the key is one of the 4 arrow keys we can use a single bind to '<Key>' and get the same results as all 4 binds.

Updated code:

import tkinter as tk


class Player(tk.Tk):
    def __init__(self):
        super().__init__()
        self.placed_ships = []
        self.player_ocean = []
        self.ai_ocean = []
        self.bombed = []
        self.sunk = []
        self.bord_size = list('ABCDEFGHIJ')
        self.selected_coord = [0, 0]
        self.ship_sizes = [5, 4, 3]
        self.direction = 'v'
        self.shipChosen = 0
        wh = 300

        p_label_frame = tk.Frame(self, width=wh, height=30)
        a_label_frame = tk.Frame(self, width=wh, height=30)
        p_label_frame.grid(row=1, column=0, padx=50)
        a_label_frame.grid(row=1, column=1, padx=50)
        p_label_frame.grid_propagate(False)
        a_label_frame.grid_propagate(False)

        for ndex, letter in enumerate(self.bord_size):
            p_label_frame.columnconfigure(ndex, weight=1)
            a_label_frame.columnconfigure(ndex, weight=1)
            tk.Label(p_label_frame, text=letter).grid(row=0, column=ndex)
            tk.Label(a_label_frame, text=letter).grid(row=0, column=ndex)

        self.player_canvas = tk.Canvas(self, height=wh, width=wh, highlightbackground='black', highlightthickness=0.5)
        self.ai_canvas = tk.Canvas(self, height=wh, width=wh, highlightbackground='black', highlightthickness=0.5)
        self.player_canvas.grid(row=2, column=0, padx=50)
        self.ai_canvas.grid(row=2, column=1, padx=50)

        temp = []
        for i in range(10):
            for y in range(10):
                temp.append(0)
            self.player_ocean.append(temp)
            self.ai_ocean.append(temp)
            temp = []

        self.refresh_ocean()
        self.bind("<Key>", self.move_ship)
        self.bind("<Return>", self.select_placement)

    def move_ship(self, event):
        if event.keysym == 'Down' and self.selected_coord[1] < 9:
            self.selected_coord[1] += 1
        elif event.keysym == 'Up' and self.selected_coord[1] > 0:
            self.selected_coord[1] -= 1
        elif event.keysym == 'Left' and self.selected_coord[0] > 0:
            self.selected_coord[0] -= 1
        elif event.keysym == 'Right' and self.selected_coord[0] < 9:
            self.selected_coord[0] += 1
        print('selected coord:', self.selected_coord)

    def select_placement(self, _=None):
        col = self.selected_coord[0]
        row = self.selected_coord[1]
        if self.direction == 'v':
            v_range_start = row - self.ship_sizes[self.shipChosen] + 1
            for y in range(v_range_start, row+1):
                '''insert validation to reject ship clashing'''
                self.player_ocean[y][col] = 1
                self.placed_ships += [y, col]
            self.refresh_ocean(side='player1')

    def refresh_ocean(self, side=None):
        self.player_canvas.delete('all')
        self.ai_canvas.delete('all')

        for y in range(len(self.bord_size)):
            for x in range(len(self.bord_size)):
                if self.player_ocean[y][x] == 1 and side == 'player1':
                    self.player_canvas.itemconfig(self.player_canvas.create_rectangle(
                        (x+1) * 30, (y+1) * 30, x * 30, y * 30, fill='green'))
                else:
                    self.player_canvas.create_rectangle(x * 30, y * 30, 300, 300, fill='white')

                if self.ai_ocean[y][x] == 1 and side == 'ai':
                    self.ai_canvas.itemconfig(self.ai_canvas.create_rectangle(
                        (x + 1) * 30, (y + 1) * 30, x * 30, y * 30, fill='green'))
                else:
                    self.ai_canvas.create_rectangle(x * 30, y * 30, 300, 300, fill='white')


if __name__ == '__main__':
    Player().mainloop()

Results:

score -1 · Answer 2 · answered Oct 24 '19 at 14:24

your refresh_ocean had an off by 1 error

def refresh_ocean(self): # 1s turns to green
    for y in range(10):
        for x in range(10):
            if self.player_ocean[y][x] == 1:
                self.player_canvas.itemconfig(self.player_canvas.create_rectangle( x*30, y * 30, x*30+30, y*30+30, fill='green'))

and select placement was placing ships in with a negative index. i changed your iteration from range(row-size, row) to range(row, row+size) and added bounds checking, but that should probably be moved to the movement handling.

def selectPlacement(event):
    col = self.selectedCoord[0]
    row = self.selectedCoord[1]
    if self.direction == 'v':
        if row + self.ship_sizes[self.shipChosen] > 9:
            row = 10 - self.ship_sizes[self.shipChosen]
        for y in range(row, row + self.ship_sizes[self.shipChosen]):
            '''insert validation to reject ship clashing'''
            self.player_ocean[y][col] = 1
            self.placedships += [y,col]
        self.refresh_ocean()

ps: your drawing new squares on top of the old squares. you might want to clear the ocean before drawing.

issue with filling the correct grid cell in tkinter

2 Answers2