JavaScript: Clear up common confusions in Battleship

[takinabradley]

Checks

• [X] This is not a duplicate of an existing issue (please have a look through our open issues list to make sure)
• [X] I have thoroughly read and understand The Odin Project Contributing Guide
• [X] Would you like to work on this issue?

Describe your suggestion

There's a couple instruction points in the Battleship project that I feel lead learners a bit astray, or feel particularly vague.

The first such point comes up in step 3.1:

Players can take turns playing the game by attacking the enemy Gameboard.

Assuming these instructions are meant to encourage learners to create Player objects that contain their own board, and can attack other player objects with a method such as attack(coords, player), I feel this may be mashing together two separate ideas, and confusing learners:

1. Players should be able to take turns while playing the game
2. Player objects should be able to attack other player objects/the opposing player's board

This creates a situation where learners may feel the need to handle the turn-logic/game-logic somehow within Player objects.

One way I've seen this manifest is by adding something like a currentPlayer property on Player objects, and an attack method like the one mentioned previously setting the attacking object's currentPlayer property to false, and setting the receiving Player object's currentPlayer property to true.

This may not be the worst approach/outcome, but learners are further encouraged to push all game logic into the Player, Gameboard, or a module for DOM interaction due to interpretations of instruction 4.4

The game loop should step through the game turn by turn using only methods from other objects. If at any point you are tempted to write a new function inside the game loop, step back and figure out which class or module that function should belong to.

There is also room here for other interpretations of what a Player object should do. Some people may simply create a player object that designates the type of the player, and perhaps allows them to output some random coordinates, and are unsure what they should do with it exactly.

An issue came up recently about instructions relating to the Player object ( #27155 ), and I feel it's likely valid, especially when considering the instructions that come directly afterwards.

---

This brings us to the second area of common confusion. Learners don't understand (and I'm certainly unsure as well) exactly how this project expects them to implement the "main game loop" in step 4. I feel this project could be a little more prescriptive on this front.

I often see learners attempting, and struggling, to get something working by putting a bunch of code in something like a while loop.... which has lead to me seeing some interestingly grueling code that feels like has only come about due to the wording throughout step 4 . I'm currently raising this issue after helping someone who essentially made the game loop a recursive async function, and was resolving user input into promises that it waited around for.

While I'm unsure exactly what the intention is meant to be, it feels like perhaps this is a good point in the project to tell users to create their objects and start sending them commands via event listeners.

An edit to step 4 might look like this:

4. Drive the game by attaching event listeners to elements that send messages to your objects. Create a module that helps you manage actions that should happen in the DOM
    1. At this point it is appropriate to begin crafting your User Interface.
    2. You should start the game by creating all the objects you feel you need, like the Players and their Gameboards. For now, populate each player's gameboard with ships at predetermined coordinates. You are going to implement a system for allowing players to place their ships later.
    3. We’ll leave the HTML implementation up to you for now, but you should display both the player’s boards and render them using information from the Gameboard class/factory.
        1. You'll need methods to render the gameboards. For attacks, let the user click on a coordinate in the enemy Gameboard. Send the user input to methods on your objects, and re-render the boards to show the user new information.
    4. Your event listeners should step through the game turn by turn using only methods from other objects. If at any point you are tempted to write a new function inside the game loop, step back and figure out which class or module that function should belong to. 
    5. Create conditions so that the game ends once one player’s ships have all been sunk. This function is appropriate for the Game module.

---

I might have just opened a PR with my suggestions, but since I've been unsure myself how learners are meant to interpret parts of this project, I felt it would be nice to invite some discussion.

Path

Node / JS

Lesson Url

https://www.theodinproject.com/lessons/node-path-javascript-battleship

(Optional) Discord Name

takinabradley

(Optional) Additional Comments

Notes:

I asserted an interpretation of step 3 (creating the player Class/factory) where player objects contain their own boards, and players can attack other players. It's possible it isn't the intention of this project to do that, due to the current wording in step 4.2. I feel this is just an additional reason to clarify what TOP expects learners to do with this object:

The game loop should set up a new game by creating Players and Gameboards. For now just populate each Gameboard with predetermined coordinates. You are going to implement a system for allowing players to place their ships later.

---

A link to a recent help session surrounding the game loop in Discord. It highlights some struggles learners have with step four. I made the suggestion to add event listeners at this point, and I feel they found the advice pretty helpful: https://discord.com/channels/505093832157691914/505093832157691916/1225467017763622964

[Slashmyaxl]

As one of the aforementioned recipients of the generous assistance of @takinabradley let me be the first to agree with his latter point about Step 4. Ultimately I felt like it was time well spent to go into my rabbit hole and figure out how to make an event listener resolve a Promise. However, given the complexity of Project: Battleship I'm all for adding a bit more clarity to and removing constraints from these instructions to prevent others from getting discouraged.

I think this also ties in well with the first point about the Player object. The instructions indeed initially led me to create an attack method and a gameboard inside of the Player object. This attack method ultimately got plugged into the opposing player's gameboard's receiveAttack method, anyway. While I did sense the redundancy, I went along because, well... I guess I'm good at following orders... hah! Having done some refactoring by now, it seems much more sensible to guide students away from this idea, particularly after having just learned about "function purity." Players are players, and Gameboards are gameboards, thus distinction can more easily be referenced within the Game loop.

[CouchofTomato]

@TheOdinProject/javascript Can someone review this please.

[MaoShizhong]

@takinabradley @Slashmyaxl Thank you both for sharing your thoughts in such detail!

Just to make sure I know what you're pointing towards/suggesting be amended, let me share what I'm understanding and feel free to correct me if I've misunderstood.

1. Step 3.1 is particularly ambiguous. It might be just a more general "rules of the game" sentence, or it might be a more specific instruction for how Player objects should act and what they therefore should contain. My reading was always the former, but given the things you've shared, it's clear it's ambiguous enough to warrant further clarification.
2. Step 4 as a whole, and especially step 4.2 could do with making it clearer that we want to make things event-driven. But currently, the wording is again ambiguous where enough people have dealt with infinite loops and async behaviour, which is definitely far less friendly to work with than an event-driven approach, as with previous projects like TTT?

[takinabradley]

@MaoShizhong

That seems to sum up my thoughts well.

Regarding 3.1: I think "Players can take turns" is particularly problematic in the context of creating this object type. That's likely something that should happen in whatever the 'game loop' is meant to be, and I think learners might feel discouraged from including anything in the game loop at all. (Edit:) For the main specs of the project, learners may not need turn-based logic at all, since there's only one real player.

Changing it to something like "Player objects can attack enemy gameboards" might be better, as it's more clear about the functionality a player object should have- but I'm unsure if the project means to be so prescriptive about the functionality. If I had to call it though, I might lean towards setting some requirements in stone like what was done for the Gameboard or Ship object types.

Step 4: I think clarifying what the project means by 'the main game loop' may go far to help learners in this project. It's a pretty commonly asked-about portion of this project. I think driving students to be thinking about events rather than loops at that point would be beneficial, for sure.

Alternatively, a new Game object could be introduced meant to encapsulate the functionality of the game, similar to what students might be exposed to from @fortypercenttitanium 's "Building a House From The Inside Out" article. Then we could instruct learners to hook events up to that.

That may raise some testing-related questions, and is definitely not required, though. I think learners could easily get by just by interacting with their Player/Gameboard objects directly with event listeners.

[takinabradley]

I feel there's a couple 'unmentioned' possible responsibilities of the 'game loop' as well:

• It may control who's turn it currently is
• It may hold some kind of game state, such as if players are currently placing ships or hitting ships

I think both of those things are reasonable issues learners might come across, in their various implementations. They are also things that don't really 'fit in' to any other object mentioned, like the project may suggest.

Neither of those are necessary to get a game up-and-running up to spec, but learners are likely to come across issues if they're thinking about them, depending on the approach. The game phase one might crop up more often for those who implement drag-and-drop and handling events for it in the DOM, and the turn handling one might crop up more for those that implement multiplayer.

[Slashmyaxl]

I'm afraid if I agreed with @takinabradley anymore I'd only sound like a poorly burned CD! - so I'll just state my issue with "Player objects can attack enemy gameboards." Similar to the current instructions, this would lead me to believe that Player objects should have an attack method. While this works, it becomes redundant for the reasons I mentioned in my previous post. I do agree that something like "Players can attack enemy gameboards" should, however, be included within the instructions regarding the game flow. In my opinion, this would make said instructions much more clear and likely to lead one to develop an event-driven game flow.

As an aside, considering the confusion that "game loop" has caused with so many students, what do we think of the "game flow" phrasing?

[takinabradley]

@Slashmyaxl

I'm not sure the idea that player objects having an attack method is a terrible idea. I think it actually makes a lot of sense for this project. I'm just not sure it's being clearly communicated whether that's why students should do or not, and the current instructions make it seem like player objects should somehow manage turns.

I don't see a problem with something like this for example:

player1.attack([0, 0], player2) // attacks the other player

I think either both approaches are completely valid:

• Players containing gameboards and having methods for attacking other players,
• Gameboards and players being completely separated, and interactions such as attack logic being completely managed by the game loop

[MaoShizhong]

Thanks both again.

I think the intent of the project instructions regarding step 3.1 is supposed to just be a casual "the two 'people' will take turns", and the verbiage just introduces ambiguity. I think it's not helped that 3.2 says that the AI should and shouldn't know certain things. A fireShot method on Player objects might work given the Gameboard has a receiveAttack method, may involve a little more flexible thinking. But I'm not entirely sure what the clearest approach for this would be - would be interesting to hear a suggestion/proposal, or thoughts from any other people as well, of course.

Regarding step 4, the "game loop" I'm very sure was intended to refer to the game flow, like with what Bender's Connect4 and TTT's GameController would've handled, with "loop" being the general term used for this sort of concept. But I think it's just an unfortunate coincidence that "loop" has a different meaning already which has led people to think a literal JavaScript loop where the game runs.

I think the verbiage for step 4 should be definitely be updated to reflect it referring to the general flow of the game, and to highlight the event-driven nature of things. They can both coincide (while my solution from way back when is by no means the best, I made mine very event driven, but still had a Game class that started the game, instantiating Gameboards, switching players etc.).

I think the exact implementation of step 4 should definitely be left up to the learner, but we should ideally avoid using "loop", and to also try and hint towards an event-driven nature rather than some async approach which doesn't seem intuitive to me at all (but I can see where people might get the idea from given previous lessons and using some kind of infinite loop perhaps).

[takinabradley]

Perhaps we could do something like this for step three, and move the instructions regarding taking turns into step 4:

3. Create `Player`
    1. There will be two types of players in the game, 'real' players and 'computer' players

4. Import your classes/factories into another file, and drive the game using event listeners to interact with your objects. Create a module that helps you manage actions that should happen in the DOM
    1. At this point it is appropriate to begin crafting your User Interface.
    2. Set up a new game by creating Players and Gameboards. For now just populate each Gameboard with predetermined coordinates. You are going to implement a system for allowing players to place their ships later.
    3. We’ll leave the HTML implementation up to you for now, but you should display both the player’s boards and render them using information from the Gameboard class/factory.
        1. You'll need methods to render the gameboards, so put them in an appropriate module.
    4. Your event listeners should step through the game turn by turn using only methods from other objects. If at any point you are tempted to write a new function, step back and figure out which class or module that function should belong to.
    5. For attacks, let the user click on a coordinate in the enemy Gameboard. Send the user input to methods on your objects, and re-render the boards to display the new information.
        1. Players should take turns playing the game by attacking the enemy Gameboard. If you feel the need to keep track of the current player's turn, it's appropriate to manage that in this module, instead of another mentioned object.
        2. The game is played against the computer, so make the ‘computer’ capable of making random plays. The AI does not have to be smart, but it should know whether or not a given move is legal (i.e. it shouldn’t shoot the same coordinate twice).
    6. Create conditions so that the game ends once one player’s ships have all been sunk. This function is also appropriate for this module.

This removes suggestions of possible responsibilities for the Player object to manage anything related to turns, but it also (somewhat) removes the interpretation that player objects may contain their own boards and such, and can attack each other (which I feel is a valid approach). It doesn't eliminate it completely, it's just less likely to be at the forefront of learners minds.

It makes the responsibilities/structure of the previously mentioned 'game loop' less ambiguous, but kind of removes any interpretation of a creating a 'GameController' module like in Bender's article. It kind of just skips to the 'ScreenController'.

I think if we want to lean towards having learners create something like a GameController, it might require more significant changes to the current instructions.

I could see the result of these changes (ideally?) leading learners to write game-driving code that might look something like this (Note that I didn't test this code, I just kind of wrote it up as pseudocode. Tell me if you think I'm missing something important):

Spoiler warning

import Player from './player.js' import Gameboard from './gameboard.js' import display from './display.js' const player1 = new Player('player') const player2 = new Player('computer') const player1Board = new Gameboard() const player2Board = new Gameboard() const player1BoardElem = document.querySelector(/*...*/) const player2BoardElem = document.querySelector(/*...*/) const placeShipForm = document.querySelector(/*...*/) let currentPlayer = player1 // if needed by the learner, not needed in this code // event listener to place ships while it's required placeShipForm.addEventListener('submit', (e) => { e.preventDefault() if (/* player1 still needs to place ships */) { const shipCoords = placeShipForm.elements.coords.value const shipType = placeShipForm.elements.type.value player1Board.placeShip(coords, type) display.renderGameboards(player1Board, player2Board) if (/* all the ships are placed */) { // have the computer player place thier ships display.hideShipForm() placeShipForm.hidden = true /* A loop that places all the computer player's ships randomly */ } } }) // event listener to attack the computer board player2BoardElem.addEventListener('click', (e) => { const coords = e.target.dataset.coords.split("") const hitNewSquare = player2Board.recieveAttack(coords) // returns true/false depending on result? if (hitNewSquare) { // probably a loop here to make sure that the computer hits a target player1Board.recieveAttack( player2.attackRandomly() ) display.renderGameboards(player1Board, player2Board) } // check for win conditions and stuff.... ? })

[MaoShizhong]

I think the idea makes sense, and the work is definitely justified here. I think details can be refined as needed in a PR - I'll assign this to you, @takinabradley.

I also think some of the ideas here might cover what was suggested in #27672. @damon314159 thoughts?

[damon314159]

@MaoShizhong this definitely covers what I was trying to achieve Some sort of a hint to split things

[MaoShizhong]

Awesome. Then @takinabradley when you make your PR, if you could set it to close both issues, that'd be great.

[takinabradley]

@MaoShizhong

On second thought, I'm feeling conflicted about what I've proposed.

The reason being is that, if you look at the code sample I shared, no Player objects in fact end up being used for a 'minimum spec' version of battleship. It's only once a learner wants to implement the 2-player option that they really become relevant. In that sample, I'm basically interacting with only the Gameboard objects.

Perhaps this realization is why you, and others, might have looked at the instructions for Player and felt it's more of a "casual the two 'people' will take turns" kind of thing.

I'm starting to think however, that players having their own boards and attacking other players may be a good idea.

• It gives player objects a reason to exist for a minimum spec version of battleship
• It sets learners up for the two-player option nicely, since it's more likely learners will actually use player objects in their code
• It creates a unique testing situation for learners, where they may have to think about what to test
• It could encourage moving some logic from the game-driving code into player objects. If you look at the example code I shared, there's a couple places where I comment something like /* a loop for the computer player */, and that might actually be functionality that fits into a player object nicely.

We might keep step three looking something like so:

3. Create `Player`
    1. There will be two types of players in the game, 'real' players and 'computer' players
    2. Players should contain Gameboards, and be able to attack other player's Gameboards. Create an `attack` method that accepts another player object or Gameboard, and coordinates to hit the enemy player's board at.
    3. The game is played against a computer, so player objects should also have a way to attack randomly.  The AI does not have to be smart, but it should know whether or not a given move is legal (i.e. it shouldn’t shoot the same coordinate twice).
    4. Similarly, player objects should have an interface for placing ships on their boards, and computer players should have a way to place all their ships at valid coordinates.

What do you think?

[MaoShizhong]

@takinabradley How would you propose to reconcile any duplication between .attack on Player objects, and .receiveAttack on Gameboard objects?

[takinabradley]

@MaoShizhong

I think it'd be reasonable for an .attack() method on a player to call .recieveAttack() on the enemy object's gameboard. With the wording, I'm not sure learners would have too much trouble coming to that conclusion.

If you'd really rather avoid that, we could leave out the attack/placement methods and expect learners to utilize each player's gameboard in a more direct way in the game-driving event code. This has the benefits of giving player objects a use in mimimum-spec versions of the project, while not having both an attack and recieveAttack method.

The benefits of suggesting an attack method duplicated on a Player object, as well as methods for placing ships, is that functionality that might otherwise be in the game-driving code (placing a computer ships randomly and making sure it's in a valid spot/attacking randomly in a valid spot), and may not necessarily need to be on a Gameboard, can be moved to the Player object instead. Perhaps that would be harder for learners to test, however (too hard to expect, with the randomness?).

(Edited, sorry about that. Need to proofread more)

I think at minimum, I'd like to suggest Player objects containing a gameboard. That encourages learners to use it, and they have the option of adding any additional functionality to it while following step 4. I think that resolves the issue well enough.

[MaoShizhong]

@takinabradley Let's go with that - having both methods. I think it might be beneficial to have something in either of the methods that helps make it clear that having both methods is intentional. I can definitely see how some people will question "what's the point in both methods?"/"why call playerOne.attack([3, 0]) instead of just calling gameboardTwo.receiveAttack([3, 0]) directly?" etc. So it's more to clarify intentions than to provide hints, if that makes sense.

But let's roll with that, and I think having a PR will be easier to discuss and refine things as necessary in context of the whole project.