-
Notifications
You must be signed in to change notification settings - Fork 58
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Simple way to remove duplicated tiles to improve performance #24
Comments
Oh, this is fantastic!!! I usually like the keep the code identical to the video (but maybe I can add something about this to the video!). Regardless, I think this is worth putting in there with comments explaining it's an optimization added after the fact. Would you like to pull request it or shall I add it myself? |
I made a pull request, you can modify the code I added as you wish :) Choo choo 🚂 |
Wouldn't this fail when 2 tiles have the exact same edges by overwriting the already existing key? For example, tiles 6 and 12 share the same edges. |
@MrAmericanMike Oh you're absolutely right! For this deduplication technique to be correct, we would have to store the image number (or filename) in each But then it's not a trivial change anymore to make to the existing code ^^ |
Couldn't we just check for duplicates amongst the 4 rotations of each tile? So make a little array of 4, remove duplicates, and then add what is remaining to the final tiles array? |
@shiffman Great idea! I implemented this solution in PR #25 (but you can implement it yourself if you want). I noticed that this also fixes an error in the original code : tiles[11] = new Tile(tileImages[11], ["BCB", "BCB", "BBB", "BBB"]);
tiles[12] = new Tile(tileImages[12], ["BBB", "BCB", "BBB", "BCB"]);
for (let i = 2; i < 14; i++) {
for (let j = 1; j < 4; j++) {
tiles.push(tiles[i].rotate(j));
}
} The initial tiles are indexed from 0 to 12, but the loop was going up to index 13. |
Hi 😃 We can write a function to get only the unique tiles in an array :
So we can apply rotations to all the tiles and then keep only the unique ones :
In our case, we have 13 images. By rotating all of them, we get 52. And by removing the duplicates, we get down to 33.
On my PC, the generation time goes from 23s to 14s 🚀
The text was updated successfully, but these errors were encountered: