Refactor tile class#53
Open
ksk5280 wants to merge 3 commits into
Open
Conversation
Removed excess args from tile constructor. Edited get position function in tile to round to nearest 10.
| U: { quantity: 8 }, | ||
| V: { quantity: 9 }, | ||
| W: { quantity: 4 }, | ||
| X: { quantity: 1 } |
There was a problem hiding this comment.
This is a lot cleaner. Awesome
|
Good refactor here - would be good to address the concerns pointed out by Adrienne and then 🚢 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What's this PR do?
Removed extra arguments out of the tile constructor now it just takes in a tile type. Moved the tile types into another file. It's not being used now, but will be used when we add validations for placing tiles only next to matching sides. Changed the tests to reflect the changes.
Where should the reviewer start?
Start by looking at the tile class, then the tile inventory class.
How should this be manually tested?
Play the game and make sure the tiles still show up and are playable.
Any background context you want to provide?
Another refactor that had to be added because tiles were not able to be played on occasion: On line 65 and 66 of the tile class, I divided and multiplied by 10 to make the numbers round to the nearest 10. When they were rounding to the nearest 1, sometimes they wouldn't be placed close enough together and this caused an alert saying that the tile placement was invalid.
@rrgayhart
@adriennedomingus
@scottfirestone
closes #51