wavesinaroom / my_odin_project_battleship

0 stars 0 forks source link

Gameboard makeover #17

Closed wavesinaroom closed 1 year ago

wavesinaroom commented 1 year ago

Shipslog and tiles should be merged into one variable/property that makes more sense with having ships and missiles objects. That facilitates rendering them and the ships panel and shots panel. It's gonna take time and effort but I want to get this app right

wavesinaroom commented 1 year ago

I learnt the importance of double-checking tests and scripts involved in a refactory process thoroughly

wavesinaroom commented 1 year ago

Really good work today Juan. You must check what's going on with checkExistingCoordinates and 'there's already a ship on that coordinate' exception. The latter is not triggering when trying to put a ship on the game board

wavesinaroom commented 1 year ago

Keeping stick to a standard isn't always the answer to your problems. Instead, it's better to understand how the language work to make the right decision, in my case I tried to follow airbnb standard regarding loop but that backfired since for loop was the only way to go in checkExistingCoordinates

wavesinaroom commented 1 year ago

Adding shots to the missiles array in GameBoard wasn't hard at all. However, testing missiles that hit on its target wasn't possible although I've console logged the boolean and everything runs perfect

wavesinaroom commented 1 year ago

Do I actually need an event manager? The code inside eventmanager.js show me I don't and I never wrote a testing suite for it but covered its functionality inside the gamemanager testing suite

wavesinaroom commented 1 year ago

I actually like having an independent event manager better. Perhaps my mistake was not to write a test suite for it.

wavesinaroom commented 1 year ago

Missiles are badly handled, I messed up

wavesinaroom commented 1 year ago

I'd better pass a missile as handleAttack parameter

wavesinaroom commented 1 year ago

I tried to test shots on target in the eventmanager test suite but it threw failed in spite of double-checking the entire flow. Means, code works well test behaves funny. I'm gonna continue with changing turns with the help of the enventmanager

wavesinaroom commented 1 year ago

Done with EventManager

wavesinaroom commented 1 year ago

I definitely need to use mocks to test the eventmanager test suite. Declaring GameManager.player and GameManager.cpu there interferes with my plans for testing gamemanager suite

wavesinaroom commented 1 year ago

Again, jest does something out of comprehension. Eventmanager.test.js works perfect trying to import GameManager module in gamemanager.test.js simply breaks this test suite, Why??

wavesinaroom commented 1 year ago

Testing with Jest can be tricky cause the console gives misleading info from time to time. I acknowdlege that TDD with jest is extremely powerful although I must practice a lot to avoid wasting time debuggin' tests. That's what has delayed the project mostly

wavesinaroom commented 1 year ago

I'm done with making over gameboard. It affected other scripts and took a long time but I'm much happier with the result. Perhaps, I will need to expand its capabilities but this is definitely a very good foundation for work I'm gonna do later

wavesinaroom commented 1 year ago

The cpu automic ship placement bug reappeared apparently. However, I double-checked that the exception was properly handled. Unfortunately, I'm not gonna spend time perfecting the test suite to get that test passed every single time