7
\$\begingroup\$

This question is a follow-up question of this one.

I've learnt a bit more Python since then. I recently came back to making battleships again, and I think I did better.

These are the changes I've made:

  1. Completely rewritten.
  2. Multiple battleships.
  3. Removed difficulty settings (as the way I was doing it before wouldn't apply to multiple battleships)
  4. Separated UI and game logic

This is the code:

"""A simple in-interpreter game of battleships."""
__version__ = "2.0.5"

from random import randint
from math import sqrt


def getBoolFromInput(prompt, defaultReturn=None, yes=(), no=()):
    """Returns True / False / None from user input"""
    inp = "".join((char for char in input(prompt).lower() if char.isalpha()))
#   inp is now all the alphabetic characters in the input
    if inp in ("yes", "y", "yh", "definitely") or inp in yes:
        return True
    elif inp in ("no", "n", "nope") or inp in no:
        return False
    return defaultReturn


def getOrdinal(num):
    """Returns an ordinal string of the number that is passed.
 (e.g. 1 --> "1st", -32 --> "-32nd", etc."""
    negative = False
    if num < 0:
        num *= -1
        negative = True
    unit = num % 10
    if 4 <= num <= 20 or 4 <= unit <= 9 or unit == 0 or num == 0:
        suffix = "th"
    else:
        suffix = ["st", "nd", "rd"][num % 10 - 1]
    if negative:
        num *= -1
    return str(num) + suffix


def win(gameState):
    print("Well done! You won battleships! That was your",
          "{} time today.".format(getOrdinal(gameState.gamesWon)))
    print("You have played {} games so far, losing {}.".format(
        gameState.gamesPlayed, gameState.gamesLost * -1))
    return endText(gameState)


def lose(gameState):
    print("Oh no! You lost! That was your {} time today. Better luck".format(
        getOrdinal(gameState.gamesLost * -1)), "next time!")
    print("You have played {} games so far, winning {}.".format(
        gameState.gamesPlayed, gameState.gamesWon))
    return endText(gameState)


def endText(gameState):
    print("This give you {}% win and you have a {} game win streak.".format(
        gameState.gamesWon // gameState.gamesPlayed * 100,
        gameState.winStreak))
    while True:
        inp = getBoolFromInput("Would you like to play again? ",
                               None, ("again"), ("notagain"))
        if inp is None:
            print("Please type yes or no.")
            continue
        return inp


class Board:
    def __init__(self, height, length):
        self.height = height
        self.length = length
        self.items = []
        for i in range(height):
            self.items.append([])
            for i2 in range(length):
                self.items[i].append({"Current mark": "-",
                                      "Has battleship": False})

    def __repr__(self):
        """Returns the board with " " between every comlumn and "\\n"
 between every row."""
        return "\n".join([" ".join([self.items[h][l]["Current mark"] for h in
                                    range(self.height)]) for l in range(
                                        self.length)])

    def updateMark(self, x, y):
        if x > self.length or y > self.height or x < 1 or y < 1:
            return "OUT OF OCEAN"
        x -= 1
        y -= 1
#       -1 so a user's (1, 1) turns into an array index [0][0]
        if self.items[y][x]["Current mark"] != "-":
            return "ALREADY HIT"
        elif self.items[y][x]["Has battleship"]:
            self.items[y][x]["Current mark"] = "B"
            return "HIT"
        self.items[y][x]["Current mark"] = "X"
        return "NO HIT"

    def revealBoard(self):
        for y in range(self.height):
            for x in range(self.length):
                if self.items[y][x]["Has battleship"]:
                    self.items[y][x]["Current mark"] = "B"
                elif self.items[y][x]["Current mark"] == "-":
                    self.items[y][x]["Current mark"] = "O"


class GameState:
    def reset(self, turns, battleships):
        self.turnsTotal = turns
        self.turn = 0
        self.turnsRemaining = turns
        self.battleships = battleships
        self.battleshipsHit = 0
        self.battleshipsRemaining = battleships

    def __init__(self, disableConfirmations):
        self.disableConfirmations = disableConfirmations
        self.gamesPlayed = 0
        self.gamesWon = 0
        self.gamesLost = 0
        self.winStreak = 0

    def nextTurn(self):
        """Increments turn and checks if the game has ended"""
        self.turn += 1
        self.turnsRemaining -= 1
        if self.turnsRemaining == 0:
            return self.gameFinished(False)
        return True

    def hitBattleship(self):
        """Increments amount of battleships hit and checks if all have
 been hit"""
        self.battleshipsHit += 1
        self.battleshipsRemaining -= 1
        if self.battleshipsRemaining == 0:
            return self.gameFinished(True)
        return True

    def gameFinished(self, result):
        self.gamesPlayed += 1
        if result:
            if self.winStreak < 1:
                self.winStreak = 0
            self.winStreak += 1
            self.gamesWon += 1
            return win(self)
        else:
            if self.winStreak > -1:
                self.winStreak = 0
            self.winStreak -= 1
            self.gamesLost -= 1
            return lose(self)


class Battleship:
    def __init__(self, board, size):
        while True:
            east = True
            self.x = randint(0, board.length - size)
            self.y = randint(0, board.height - size)
            self.direction = randint(0, 1)
            valid = True
            if self.direction == randint(0, 1):
                self.direction = "EAST"
                x = self.x
                for i in range(size):
                    if board.items[self.y][x]["Has battleship"]:
                        valid = False
                    x += 1
            else:
                self.direction = "SOUTH"
                east = False
                y = self.y
                for i in range(size):
                    if board.items[y][self.x]["Has battleship"]:
                        valid = False
                    y += 1
            if valid:
                if east:
                    x = self.x
                    for i in range(size):
                        board.items[self.y][x]["Has battleship"] = True
                        x += 1
                else:
                    y = self.y
                    for i in range(size):
                        board.items[y][self.x]["Has battleship"] = True
                        y += 1
                break


def main():
    try:
        print(u"""Hello! Welcome to battleships, version 2.0.
Changelog:
- Complete rewrite
  - The game has multiple battleships now!
  - No longer any difficulty setting (because of said multiple battleships)
  - Now with \u221E% more classes!

Now, let's get started!
""")
    except UnicodeEncodeError:
        print("""Hello! Welcome to battleships, version 2.0.
Changelog:
- Complete rewrite
  - The game has multiple battleships now!
  - No longer any difficulty setting (because of said multiple battleships)
  - Now with infinity% more classes!

Now, let's get started!
""")
    gameState = GameState(getBoolFromInput("Would you like to disable " +
                                           "confirmation messages? ", False))
#   Main game loop
    while True:
        s = "tall"
        while True:
            inp = input("How {} would you like the board? ".format(s))
            try:
                inp = int(inp)
            except ValueError:
                print("Please type in a number!")
                continue
            if inp < 4:
                print("Please type a value greater than 3!")
            elif inp > 25:
                if not getBoolFromInput("Do you really want a board dimensio" +
                                        "n this big?", False):
                    continue
            if s == "tall":
                s = "long"
                height = inp
            else:
                length = inp
                if not gameState.disableConfirmations:
                    a = getBoolFromInput("Are you alright with a " +
                                         "{} by {} board? ".format(height,
                                                                   length))
                    if a is None:
                        print("I'll take that as a \"Yes\"")
                    elif a:
                        print("OK! Let's continue!")
                    else:
                        s = "tall"
                        print("Try again then.")
                        continue
                break
        board = Board(height, length)
        maxBattleships = int(0.5*sqrt(8*(height*length) + 1) - 0.5) - 2
#       Finds which triangle number the area of the board is inbetween
#       so you can't put more battleships than the board can take.
        while True:
            print("You can have a maximum of", maxBattleships, "battleships",
                  "with your board size.")
            inp = input("How many battleships would you like? ")
            try:
                inp = int(inp)
            except ValueError:
                print("Please type a number!")
                continue
            if inp < 1:
                print("Please enter a value greater than 0!")
                continue
            elif inp > maxBattleships:
                print("Please type a value less than", maxBattleships)
                continue
            if not gameState.disableConfirmations:
                a = getBoolFromInput("Are you alright with " + str(inp) +
                                     " battleships? ")
                if a is None:
                    print('I\'ll take that as a "Yes"')
                elif a:
                    print("OK! Let's continue!")
                else:
                    print("Try again then.")
                    continue
            break
        battleships = inp
        while True:
            inp = input("How many turns would you like? ")
            try:
                inp = int(inp)
            except ValueError:
                print("Please type a number!")
                continue
            if inp < 1:
                print("Please enter a value greater than 0!")
                continue
            if not gameState.disableConfirmations:
                a = getBoolFromInput("Are you alright with " + str(inp) +
                                     " turns? ")
                if a is None:
                    print("I'll take that as a \"Yes\"")
                elif a:
                    print("OK! Let's continue!")
                else:
                    print("Try again then.")
                    continue
            break
        gameState.reset(inp, (battleships * (battleships + 1)) // 2)
#                             This equation shows which triangle number
#                             the battleship represents
        allBattleships = {}
        for i in range(battleships, 1, -1):
            allBattleships[i] = Battleship(board, i)
#           This makes the battleships increase in size, with their
#           lengths like (1, 2, 3) or (1, 2, 3, 4) etc.
        playing = True
        while playing:
            if gameState.turn == 0:
                print("Let's play Battleships!\nTurn 1, {} turn{}".format(
                    gameState.turnsRemaining, "".join(
                        ["" if gameState.turnsRemaining == 1 else "s"
                         for i in (1,)])), "remaining.")
            elif gameState.turnsRemaining == 1:
                print("Last turn!")
            else:
                print("Turn {}, {} turns remaining.".format(
                    gameState.turn, gameState.turnsRemaining))
            print(board)
            s = "Y"
            while True:
                inp = input(s + " coordinate: ")
                try:
                    inp = int(inp)
                except ValueError:
                    print("Please type a valid number!")
                    continue
                if s == "Y":
                    s = "X"
                    x = inp
                    continue
                y = inp
                if not gameState.disableConfirmations:
                    inp = not getBoolFromInput("Are you sure that you want" +
                                               "to aim for {}, {}? ".format(
                                                   x, y), True)
                    if inp:
                        print("Try again then.")
                        s = "Y"
                        continue
                break
            aim = board.updateMark(x, y)
            if aim == "OUT OF OCEAN":
                print("That's not even in the ocean...")
            elif aim == "ALREADY HIT":
                print("You already hit that one.")
            elif aim == "NO HIT":
                print("You didn't hit anything...")
            else:
                print("Congratualtions, you hit part of a battleship!")
                cont = gameState.hitBattleship()
            if gameState.battleshipsRemaining != 0:
                cont = gameState.nextTurn()
            if gameState.turnsRemaining != 0 and \
               gameState.battleshipsRemaining != 0:
                continue
            else:
                board.revealBoard()
                print("\n" + str(board) + "\nThese were the battleships!")
            playing = False
            if cont:
                print("\n\nRestarting...\n\n")
                continuing = True
            else:
                continuing = False
        if continuing:
            continue
        print("Thanks for playing!")
if __name__ == "__main__":
    main()

I made it in a few hours (~3 to 4), so there are probably are some bugs; however, the game plays fine, and I can't find any bugs. I would like some feedback on 2 things:

  1. if the classes are necessary for readability or just confusing
  2. if my documentation using comments and docstrings is good enough.

Notes:

  1. I am using camelCase over underscore_case as I prefer it, and find it more readable.
  2. I am repeating many similar while True: loops as they are so very slightly different, that I couldn't get around it.
\$\endgroup\$
1
\$\begingroup\$

Your imports should be ordered.

You mentioned

I am using camelCase over underscore_case as I prefer it, and find it more readable.

but I'm still going to pick you up on it ;).

get_bool_from_input should really have a separation of concerns; the logic that needs abstraction is going from the input to the boolean. As such, you should probably have bool_from_string and let the caller call input.

In changing this, I noticed that you have

inp = not getBoolFromInput("Are you sure that you want" +
                           "to aim for {}, {}? ".format(
                               x, y), True)

Which joins to

"Are you sure that you wantto aim for {}, {}? "

This kind of error is hidden by the somewhat haphazard formatting. Going with the change to bool_from_string, I would use something like:

inp = not bool_from_string(
    input("Are you sure that you want to aim for {}, {}? ".format(x, y)),
    True
)

Even better, split the logic into several source lines:

question = "Are you sure that you want to aim for {}, {}? ".format(x, y)
answer = not bool_from_string(input(question), True)

...although immediately noting the result and not giving it a meaningful name is a surefire way to confuse readers on what the code is actually trying to do.

Back to bool_from_string.

  • I find the formatting a bit strange.

    • The comment is formatted oddly as it should be flush with the text.

    • There should be spacing after the doc comment.

    • Comprehensions inside function calls don't need brackets.

  • The doc comment is wrong and largely unhelpful.

  • default_return should just be default.

  • yes and no should probably be extra_yes and extra_no. These should probably have precedence over the internal options.

  • The internal options should be sets.

  • The comment could be removed if you just named inp better.

  • The sets should probably be globals.

YES_STRINGS = frozenset({"yes", "y", "yh", "definitely"})
NO_STRINGS = frozenset({"no", "n", "nope"})

def bool_from_string(string, default=None, extra_yes=frozenset(), extra_no=frozenset()):
    """
    Tries to decide whether the input means "yes" or "no".

    Returns True if the input means "yes", False if it means "no"
    and default if it's unknown. If extra_yes or extra_no are passed in,
    they must contain only lowercase alpha strings and will take
    precidence over internal checks.
    """

    alpha_lower = "".join(char for char in string if char.isalpha())

    if alpha_lower in extra_yes:
        return True

    if alpha_lower in extra_no:
        return False

    if alpha_lower in YES_STRINGS:
        return True

    elif alpha_lower in NO_STRINGS:
        return False

    return default

Personally, though, it's better just to do:

def bool_from_string(string, yes_strings=YES_STRINGS, no_string=NO_STRINGS, default=None):
    """
    Tries to decide whether the input means "yes" or "no".

    Returns True if the input means "yes", False if it means "no"
    and default if it's unknown. yes_strings and no_strings must
    must contain only lowercase alpha strings.
    """

    alpha_lower = "".join(char for char in string if char.isalpha())

    if alpha_lower in YES_STRINGS:
        return True

    elif alpha_lower in NO_STRINGS:
        return False

    return default

and callers can choose to call as bool_from_string("foo", YES_STRINGS | {"foo"}, NO_STRINGS | {"bar"}).

I would also make default a keyword-only argument:

def bool_from_string(string, yes_strings=YES_STRINGS, no_string=NO_STRINGS, *, default=None):

These changes also highlight this problem:

    inp = getBoolFromInput("Would you like to play again? ",
                           None, ("again"), ("notagain"))

Where ("again") just means "again" and so the test inp in yes just checks that inp is a substring of "again".

getOrdinal again has some strange formatting. Just fixing that and cleaning up the docstring gives:

def get_ordinal(num):
    """
    Returns an ordinal string of the number that is passed.

    get_ordinal(0)  = "0th"
    get_ordinal(1)  = "1st"
    get_ordinal(2)  = "2nd"
    get_ordinal(-32) = "-32nd"
    """

    negative = False
    if num < 0:
        num *= -1
        negative = True

    unit = num % 10
    if 4 <= num <= 20 or 4 <= unit <= 9 or unit == 0 or num == 0:
        suffix = "th"
    else:
        suffix = ["st", "nd", "rd"][num % 10 - 1]

    if negative:
        num *= -1

    return str(num) + suffix

The first block can be changed for:

negative = num < 0
num = abs(num)

Your check

unit = num % 10
if 4 <= num <= 20 or 4 <= unit <= 9 or unit == 0:

doesn't need the final num == 0 as it implies unit == 0. However, it's easier just to invert the check:

if unit in {1, 2, 3} and num not in {11, 12, 13}:
    suffix = ["st", "nd", "rd"][num % 10 - 1]
else:
    suffix = "th"

This isn't enough, though, as get_ordinal(111) should return 111th (it currently gives 111st). You should instead do

if unit in {1, 2, 3} and num % 100 not in {11, 12, 13}:
    suffix = ["st", "nd", "rd"][num % 10 - 1]
else:
    suffix = "th"

It's strange you modify num and then reverse the change; just use a different variable instead. Since the change is so simple, you can just stick with:

def get_ordinal(num):
    """
    Returns an ordinal string of the number that is passed.

    get_ordinal(0)  = "0th"
    get_ordinal(1)  = "1st"
    get_ordinal(2)  = "2nd"
    get_ordinal(-32) = "-32nd"
    """

    mag = abs(num)

    if mag % 10 in {1, 2, 3} and mag % 100 not in {11, 12, 13}:
        suffix = ["st", "nd", "rd"][mag % 10 - 1]
    else:
        suffix = "th"

    return str(num) + suffix

I would also suggest replacing

suffix = ["st", "nd", "rd"][mag % 10 - 1]

with

suffix = {1: "st", 2: "nd", 3: "rd"}[mag % 10]

In fact, you could even write:

# 11th, 12th, 13th
if mag % 100 in {11, 12, 13}:
    suffix = "th"
else:
    suffix = {1: "st", 2: "nd", 3: "rd"}.get(mag % 10, "th")

win is slightly misnamed; it only tells people that they have won and asks if they want to play again. This would be better named alert_won_and_retry. However, I would probably not conflate the two options and just have alert_won, leaving the caller to run end_text. It so happens that this would simplify code, too.

The formatting here is also haphazard. Line wrapping should be logical, not just whereever it fits.

print("Well done! You won battleships! That was your",
      "{} time today.".format(getOrdinal(gameState.gamesWon)))
print("You have played {} games so far, losing {}.".format(
    gameState.gamesPlayed, gameState.gamesLost * -1))

This also hides the same line-concatenation bug as before.

My personal style would use

print("Well done! You won battleships! That was your {} time today".format(
    get_ordinal(game_state.games_won)
))
print("You have played {} games so far, losing {}.".format(
    game_state.games_played,
    game_state.games_lost * -1
))

What matters isn't that your style is the same as mine or that it sticks strictly to line length guidelines but that it makes bugs look wrong. Whether this means you write things on loads of different lines:

number_wins_alert = "Well done! You won battleships! That was your {} time today"
number_wins_alert = number_wins_alert.format(get_ordinal(game_state.games_won))

print(number_wins_alert)

number_games_alert = "You have played {} games so far, losing {}."
number_games_alert = number_games_alert.format(game_state.games_played, game_state.games_lost * -1)

print(number_games_alert)

or do Lisp-style expression embebedding, the aim is to make the structure and logic coincide.

Exactly the same arguments apply to lost.

end_text is really misnamed; it should be something like alert_finish_and_ask_replay. This might suggest again that you split it up into alert_finish and ask_replay.

I have alert_finish doing:

def alert_finish(game_state):
    print("This give you {}% win and you have a {} game win streak.".format(
        game_state.games_won // game_state.games_played * 100,
        game_state.win_streak
    ))

This is buggy; game_state.games_won // game_state.games_played does integer division and then * 100 will always give you 100 or 0. You should have done

int(game_state.games_won / game_state.games_played * 100)

or, better, just used the {:.0%} format specifier. The .0 says there should be 0 fractional digits and the % displays the number as a percentage.

print("This give you {}% win and you have a {:.0%} game win streak.".format(
    game_state.games_won / game_state.games_played,
    game_state.win_streak
))

All of these GameStates are getting to me: it's a class so of course it's state. You should just call it game.

Looking at Board.__init__, we have the generation of a grid. This is simpler written as a comprehension:

cell = {"Current mark": "-", "Has battleship": False}
self.items = [[cell.copy() for _ in range(length)] for _ in range(height)]

I would be really tempted to change the grid cells to objects:

class Cell:
    def __init__(self, current_mark="-", has_battleship=False):
        self.current_mark = current_mark
        self.has_battleship = has_battleship

class Board:
    def __init__(self, height, length):
        self.height = height
        self.length = length
        self.items = [[Cell() for _ in range(length)] for _ in range(height)]

Board.__repr__'s docstring should be reformatted again. Either like

"""Returns the board with " " between every column and "\\n" between every row."""

or like

"""
Returns the board with " " between every column and
"\\n" between every row.
"""

Further, the lines should be split up from their current squished self:

I would also use words to describe newlines.

Transposing can be done with zip(*self.items), giving

def __repr__(self):
    """
    Returns the board with spaces between comlumns and rows on separate lines.
    """

    transposed = zip(*self.items)
    return "\n".join(" ".join(cell.current_mark for cell row) for row in transposed)

updateMark should really not be returning strings, but with this organization I don't see anything much easier than an enum.

class AttackSuccess(Enum):
    hit = 1
    miss = 2
    already_hit = 3
    out_of_bounds = 4

At point of use you should do

if aim = AttackSuccess.hit:
    print("Congratualtions, you hit part of a battleship!")
    cont = game.hit_battleship()

elif aim == AttackSuccess.miss:
    print("You didn't hit anything...")

elif aim == AttackSuccess.out_of_bounds:
    print("That's not even in the ocean...")

elif aim == AttackSuccess.already_hit:
    print("You already hit that one.")

else:
    raise NotImplementedError

which will trigger an error if the enum gets extended and the point of use isn't, not just break silently.

Again, the comment should be flush with the text.

if x > self.length or y > self.height or x < 1 or y < 1:
        return AttackSuccess.out_of_bounds

would be better as

if not (1 <= x <= self.length and 1 <= y <= self.height):
        return AttackSuccess.out_of_bounds

Personally, though, I would do x -= 1; y -= 1 before calling the function: internal functions shouldn't be exposed to user-facing details like 1-based indexing.

No need for elif if you've already returned.

The name could be more descriptive; perhaps attack_cell

This gives:

def attack_cell(self, x, y):
    if not (0 <= x < self.length and 0 <= y < self.height):
        return AttackSuccess.out_of_bounds

    if self.items[y][x].current_mark != "-":
        return AttackSuccess.already_hit

    if self.items[y][x].has_battleship:
        self.items[y][x].current_mark = "B"
        return AttackSuccess.hit

    self.items[y][x].current_mark = "X"
    return AttackSuccess.miss

revealBoard is largely OK.

GameState should have its __init__ function first. Its init function doesn't initialize all of its values; you have to then call reset to get that running. This suggests you have a bad separation of concerns; Game should be split into two classes where one handles the overall statistics and another handles the game itself. I would go with GameStatistics and BattleshipsGame.

With this, gameFinished should be split into three parts; alert_finish and and finish_game which are part of GameStatistics. The global alert_finish function can be merged into the local alert_finish method and the alert_won and alert_lost functions should become methods.

ask_replay would be called from the code that handles controlling the game (currently main) as neither class has any business dealing with it. next_turn and hit_battleship should return whether the game has ended ignoring whether the user wants to replay.

turnsTotal and batteships and aren't used, so remove them.

GameStatistics.finish_game would be

def finish_game(self, result):
    self.games_played += 1

    if result:
        if self.win_streak < 1:
            self.win_streak = 0

        self.win_streak += 1
        self.games_won += 1

    else:
        if self.win_streak > -1:
            self.win_streak = 0

        self.win_streak -= 1
        self.games_lost -= 1

You write games_lost -= 1 but should really write games_lost += 1; you end up multiplying the result by -1 whenever you use it anyway! You can also extract the zeroing logic:

def finish_game(self, result):
    self.games_played += 1

    if (self.win_streak > 0) != result:
        self.win_streak = 0

    if result:
        self.win_streak += 1
        self.games_won += 1
    else:
        self.win_streak -= 1
        self.games_lost += 1

It might be better to stick with the origingal logic for zeroing if this doesn't doesn't seem self explanatory enough, but it's the same deal as the old cmp function or Java's compareTo.

disableConfirmations would be better named as a positive; confirmations_enabled. It should also be removed from the GameStatistics class because it's neither used internally nor really benefits from encapsulation.

The Battleship class is a behemoth with only one method; __init__. This seems completely the wrong way to do it, especially because __init__ shouldn't really be mutating things. If you want battleships to be made inside boards, add a method to the board class. In fact, you write:

allBattleships = {}
for i in range(battleships, 1, -1):
    allBattleships[i] = Battleship(board, i)

but never touch allBattleships again! This implied that a class is really the wrong abstraction in some sense; you're not using Battleship to store state but to stick itself onto the board somehow. Get rid of the class entirely.

In the constructor you use self.direction for two purposes. One is to hold a random boolean you use once and another is to hold an unused string. The first can be replaced with a random.choice([True, False]) at point of use and the second can be removed entirely.

This can be further simplified by just writing east = random.choice([True, False]) and not mutating it further.

Your loops can be replaced with ranges using the start and stop parameters.

This all gives:

def add_battleship(self, size):
    while True:
        east = random.choice([True, False])
        start_x = random.randint(0, self.length - size)
        start_y = random.randint(0, self.height - size)
        valid = True

        if east:
            for x in range(start_x, start_x+size):
                if self.items[start_y][x].has_battleship:
                    valid = False
        else:
            for y in range(start_y, start_y+size):
                if self.items[y][start_x].has_battleship:
                    valid = False

        if valid:
            break

    if east:
        for x in range(start_x, start_x+size):
            self.items[start_y][x].has_battleship = True
    else:
        for y in range(start_y, start_y+size):
            self.items[y][start_x].has_battleship = True

It makes sense to view these as rectangles to remove the two cases:

def add_battleship(self, size):
    while True:
        start_x = random.randint(0, self.length - size)
        start_y = random.randint(0, self.height - size)
        valid = True

        if random.choice([True, False]):
            width, height = size, 1
        else:
            width, height = 1, size

        for x in range(start_x, start_x+width):
            for y in range(start_y, start_y+height):
                if self.items[y][x].has_battleship:
                    valid = False

        if valid:
            break

    for x in range(start_x, start_x+width):
        for y in range(start_y, start_y+height):
            self.items[y][x].has_battleship = True

You could also use all here:

def add_battleship(self, size):
    while True:
        start_x = random.randint(0, self.length - size)
        start_y = random.randint(0, self.height - size)

        if random.choice([True, False]):
            width, height = size, 1
        else:
            width, height = 1, size

        xs = range(start_x, start_x+width)
        ys = range(start_y, start_y+height)

        if not any(self.items[y][x].has_battleship for x in xs for y in ys):
            break

    for x in xs:
        for y in ys:
            self.items[y][x].has_battleship = True

main is the real beast here, but I'm going to wait before tackling that.

I will post the updated code in another answer because I have run out of characters to add it to this one.

\$\endgroup\$
0
\$\begingroup\$

Here is the updated code from the other answer.

"""A simple in-interpreter game of battleships."""
__version__ = "2.0.5"

import random

from enum import Enum
from math import sqrt

YES_STRINGS = frozenset({'definitely', 'y', 'yes', 'yh'})
NO_STRINGS = frozenset({'n', 'no', 'nope'})

def bool_from_string(string, yes_strings=YES_STRINGS, no_string=NO_STRINGS, *, default=None):
    """
    Tries to decide whether the input means "yes" or "no".

    Returns True if the input means "yes", False if it means "no"
    and default if it's unknown. yes_strings and no_strings must
    must contain only lowercase alpha strings.
    """

    alpha_lower = "".join(char for char in string if char.isalpha())

    if alpha_lower in YES_STRINGS:
        return True

    elif alpha_lower in NO_STRINGS:
        return False

    return default

def get_ordinal(num):
    """
    Returns an ordinal string of the number that is passed.

    get_ordinal(0)  = "0th"
    get_ordinal(1)  = "1st"
    get_ordinal(2)  = "2nd"
    get_ordinal(-32) = "-32nd"
    """

    mag = abs(num)

    # 11th, 12th, 13th
    if mag % 100 in {11, 12, 13}:
        suffix = "th"
    else:
        suffix = {1: "st", 2: "nd", 3: "rd"}.get(mag % 10, "th")

    return str(num) + suffix


def ask_replay(game):
    while True:
        inp = bool_from_string(
            input("Would you like to play again? "),
            YES_STRINGS | {"again"},
            NO_STRINGS | {"notagain"},
            default=None
        )

        if inp is None:
            print("Please type yes or no.")
            continue

        return inp

class Cell:
    def __init__(self, current_mark="-", has_battleship=False):
        self.current_mark = current_mark
        self.has_battleship = has_battleship

class AttackSuccess(Enum):
    hit = 1
    miss = 2
    already_hit = 3
    out_of_bounds = 4

class Board:
    def __init__(self, height, length):
        self.height = height
        self.length = length
        self.items = [[Cell() for _ in range(length)] for _ in range(height)]

    def __repr__(self):
        """
        Returns the board with spaces between comlumns and rows on separate lines.
        """

        transposed = zip(*self.items)
        return "\n".join(" ".join(cell.current_mark for cell in row) for row in transposed)

    def attack_cell(self, x, y):
        if not (0 <= x < self.length and 0 <= y < self.height):
            return AttackSuccess.out_of_bounds

        if self.items[y][x].current_mark != "-":
            return AttackSuccess.already_hit

        if self.items[y][x].has_battleship:
            self.items[y][x].current_mark = "B"
            return AttackSuccess.hit

        self.items[y][x].current_mark = "X"
        return AttackSuccess.miss

    def reveal_board(self):
        for y in range(self.height):
            for x in range(self.length):
                if self.items[y][x].has_battleship:
                    self.items[y][x].current_mark = "B"

                elif self.items[y][x].current_mark == "-":
                    self.items[y][x].current_mark = "O"

    def add_battleship(self, size):
        while True:
            start_x = random.randint(0, self.length - size)
            start_y = random.randint(0, self.height - size)

            if random.choice([True, False]):
                width, height = size, 1
            else:
                width, height = 1, size

            xs = range(start_x, start_x+width)
            ys = range(start_y, start_y+height)

            if not any(self.items[y][x].has_battleship for x in xs for y in ys):
                break

        for x in xs:
            for y in ys:
                self.items[y][x].has_battleship = True

class GameStatistics:
    def __init__(self):
        self.games_played = 0
        self.games_won = 0
        self.games_lost = 0
        self.win_streak = 0

    def alert_won(self):
        print("Well done! You won battleships! That was your {} time today".format(
            get_ordinal(self.games_won)
        ))
        print("You have played {} games so far, losing {}.".format(
            self.games_played,
            self.games_lost
        ))

        self.alert_finish()

    def alert_lost(self):
        print("Oh no! You lost! That was your {} time today. Better luck next time!".format(
            get_ordinal(self.games_lost)
        ))
        print("You have played {} games so far, winning {}.".format(
            self.games_played, self.games_won
        ))

        self.alert_finish()

    def alert_finish(self):
        print("This give you {}% win and you have a {:.0%} game win streak.".format(
            self.games_won / self.games_played,
            self.win_streak
        ))

    def finish_game(self, result):
        self.games_played += 1

        if (self.win_streak > 0) != result:
            self.win_streak = 0

        if result:
            self.win_streak += 1
            self.games_won += 1
        else:
            self.win_streak -= 1
            self.games_lost += 1

class BattleshipsGame:
    def __init__(self, turns, battleships):
        self.turn = 0
        self.turns_remaining = turns
        self.battleships_hit = 0
        self.battleships_remaining = battleships

    def next_turn(self):
        """Increments turn and checks if the game has ended"""

        self.turn += 1
        self.turns_remaining -= 1
        return self.turns_remaining == 0

    def hit_battleship(self):
        """Increments amount of battleships hit and checks if all have been hit"""

        self.battleships_hit += 1
        self.battleships_remaining -= 1
        return self.battleships_remaining == 0

def main():
    try:
        print(u"""Hello! Welcome to battleships, version 2.0.
Changelog:
- Complete rewrite
  - The game has multiple battleships now!
  - No longer any difficulty setting (because of said multiple battleships)
  - Now with \u221E% more classes!

Now, let's get started!
""")
    except UnicodeEncodeError:
        print("""Hello! Welcome to battleships, version 2.0.
Changelog:
- Complete rewrite
  - The game has multiple battleships now!
  - No longer any difficulty setting (because of said multiple battleships)
  - Now with infinity% more classes!

Now, let's get started!
""")
    stats = GameStatistics()

    confirmations_enabled = not bool_from_string(
        input("Would you like to disable confirmation messages? "),
        default=False
    )

    # Main game loop
    while True:
        s = "tall"
        while True:
            inp = input("How {} would you like the board? ".format(s))
            try:
                inp = int(inp)
            except ValueError:
                print("Please type in a number!")
                continue
            if inp < 4:
                print("Please type a value greater than 3!")
            elif inp > 25:
                confirmed = bool_from_string(
                    input("Do you really want a board dimension this big? "),
                    default=False
                )
                if not confirmed:
                    continue
            if s == "tall":
                s = "long"
                height = inp
            else:
                length = inp
                if confirmations_enabled:
                    a = bool_from_string(
                        input("Are you alright with a {} by {} board? ".format(height, length))
                    )
                    if a is None:
                        print("I'll take that as a \"Yes\"")
                    elif a:
                        print("OK! Let's continue!")
                    else:
                        s = "tall"
                        print("Try again then.")
                        continue
                break

        board = Board(height, length)
        max_battleships = int(0.5*sqrt(8*(height*length) + 1) - 0.5) - 2
#       Finds which triangle number the area of the board is inbetween
#       so you can't put more battleships than the board can take.
        while True:
            print("You can have a maximum of", max_battleships, "battleships",
                  "with your board size.")
            inp = input("How many battleships would you like? ")
            try:
                inp = int(inp)
            except ValueError:
                print("Please type a number!")
                continue
            if inp < 1:
                print("Please enter a value greater than 0!")
                continue
            elif inp > max_battleships:
                print("Please type a value less than", max_battleships)
                continue
            if confirmations_enabled:
                a = bool_from_string(input("Are you alright with " + str(inp) + " battleships? "))
                if a is None:
                    print('I\'ll take that as a "Yes"')
                elif a:
                    print("OK! Let's continue!")
                else:
                    print("Try again then.")
                    continue
            break
        battleships = inp
        while True:
            inp = input("How many turns would you like? ")
            try:
                inp = int(inp)
            except ValueError:
                print("Please type a number!")
                continue
            if inp < 1:
                print("Please enter a value greater than 0!")
                continue
            if confirmations_enabled:
                a = bool_from_string(input("Are you alright with " + str(inp) + " turns? "))
                if a is None:
                    print("I'll take that as a \"Yes\"")
                elif a:
                    print("OK! Let's continue!")
                else:
                    print("Try again then.")
                    continue
            break

        game = BattleshipsGame(inp, (battleships * (battleships + 1)) // 2)
#                             This equation shows which triangle number
#                             the battleship represents
        for i in range(battleships, 1, -1):
            board.add_battleship(size=i)
#           This makes the battleships increase in size, with their
#           lengths like (1, 2, 3) or (1, 2, 3, 4) etc.
        playing = True
        while playing:
            if game.turn == 0:
                print("Let's play Battleships!\n_turn 1, {} turn{}".format(
                    game.turns_remaining, "".join(
                        ["" if game.turns_remaining == 1 else "s"
                         for i in (1,)])), "remaining.")
            elif game.turns_remaining == 1:
                print("Last turn!")
            else:
                print("Turn {}, {} turns remaining.".format(
                    game.turn, game.turns_remaining))
            print(board)
            s = "Y"
            while True:
                inp = input(s + " coordinate: ")
                try:
                    inp = int(inp)
                except ValueError:
                    print("Please type a valid number!")
                    continue
                if s == "Y":
                    s = "X"
                    x = inp
                    continue
                y = inp
                if confirmations_enabled:
                    question = "Are you sure that you want to aim for {}, {}? ".format(x, y)
                    confirmed = bool_from_string(input(question), default=True)

                    if not confirmed:
                        print("Try again then.")
                        s = "Y"
                        continue
                break

            aim = board.attack_cell(x - 1, y - 1)

            if aim == AttackSuccess.hit:
                print("Congratualtions, you hit part of a battleship!")
                cont = game.hit_battleship()

            elif aim == AttackSuccess.miss:
                print("You didn't hit anything...")

            elif aim == AttackSuccess.out_of_bounds:
                print("That's not even in the ocean...")

            elif aim == AttackSuccess.already_hit:
                print("You already hit that one.")

            else:
                raise NotImplementedError

            if game.battleships_remaining != 0:
                cont = game.next_turn()
            if game.turns_remaining != 0 and \
               game.battleships_remaining != 0:
                continue
            else:
                board.reveal_board()
                print("\n" + str(board) + "\n_these were the battleships!")
            playing = False
            if cont:
                print("\n\n_restarting...\n\n")
                continuing = True
            else:
                continuing = False
        if continuing:
            continue
        print("Thanks for playing!")

if __name__ == "__main__":
    main()

Note that main does not currently handle the changes to game flow made with regards to finishing the game.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy