Skip to content
This repository was archived by the owner on Apr 13, 2021. It is now read-only.

Remove TileInstance and just pass around TileId instead #11

Open
randomPoison opened this issue Jun 2, 2020 · 0 comments
Open

Remove TileInstance and just pass around TileId instead #11

randomPoison opened this issue Jun 2, 2020 · 0 comments
Labels
shared Changes that affect the shared codebase tech debt Internal technical improvements or code cleanup

Comments

@randomPoison
Copy link
Owner

TileInstance was originally setup so that we could try to take advantage of Rust's ownership system when moving tiles around with the hope of avoiding bugs that could result from accidentally duplicating a tile. However, in practice a lot of communication around tiles is based around TileId. This is especially common around client/server communication, where we always send a TileId rather than a full TileInstance in order to minimize bandwidth usage. With the addition of tile::by_id() it's now possible to look up the value of a tile with just its ID, making TileInstance effectively pointless.

At this point we should remove TileInstance and just use TileId everywhere, using tile::by_id() to lookup the value of the tile in the cases where it is needed. In practice it may still be convenient to keep TileInstance so that we can have a concept of a combine (ID + value) pair when referring to a tile, but we should largely remove from the public APIs of the different components of the game.

@randomPoison randomPoison added shared Changes that affect the shared codebase tech debt Internal technical improvements or code cleanup labels Jun 2, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
shared Changes that affect the shared codebase tech debt Internal technical improvements or code cleanup
Projects
None yet
Development

No branches or pull requests

1 participant