Add utility for generic sprite sheet format. #350
Conversation
| width: Int, | ||
| height: Int, | ||
| tileWidth: Int, | ||
| tileHeight: Int, |
There was a problem hiding this comment.
The convention is that the main constructor will use Size instead of separate width and height values. What we then do is has a companion with an apply method that gives additional (more loosely typed) construction options. So this would be (perhaps):
assetName: AssetName, imageSize: Size, tileSize: Size, margin: IntThere was a problem hiding this comment.
So, move the current argument list to an apply method? OK.
indigo/indigo/src/main/scala/indigo/shared/formats/GenericTileset.scala
Outdated
Show resolved
Hide resolved
indigo/indigo/src/main/scala/indigo/shared/formats/GenericTileset.scala
Outdated
Show resolved
Hide resolved
indigo/indigo/src/main/scala/indigo/shared/formats/GenericTileset.scala
Outdated
Show resolved
Hide resolved
indigo/indigo/src/main/scala/indigo/shared/formats/GenericTileset.scala
Outdated
Show resolved
Hide resolved
| y <- 0 until height by tileHeight + margin | ||
| x <- 0 until width by tileWidth + margin | ||
| rectangle = Rectangle(x, y, tileWidth, tileHeight) | ||
| } yield rectangle |
There was a problem hiding this comment.
So this works, but you can just do this with math. You don't even need the image height:
https://github.com/PurpleKingdomGames/roguelike-starterkit/blob/701c3e2f7ffb027fae694ea900a29708ab5ffd85/roguelike-starterkit/src/main/scala/io/indigoengine/roguelike/starterkit/utils/PathFinder.scala#L160
There was a problem hiding this comment.
Of course, you may want the image height for validation, and the link above would then need to be multiplied out by the size.
The advantage of your approach is that you've pre-calculated the rectangles just one, at an instantiation cost. If someone made one of these on every frame to look up a single index, they're paying a lot for little benefit. If they make one once, build a bunch of graphics ahead of time and store them and throw this instance away, then the cost is totally worth it. However, the maths calculation is cheap, so unless I've missed something (I've yet to have my first coffee of the day so... likely!) then you might as well do the math?
There was a problem hiding this comment.
I guess this was a case of premature optimization. In my own code, I instantiate the class once, but call its apply method ~240,000 times per second, so it seemed like a no-brainer.
| * and increases as it goes right and down | ||
| * @return an optional Graphic, None if the given index was out of bounds. | ||
| */ | ||
| def apply(idx: Int): Option[Graphic[Material.Bitmap]] = |
There was a problem hiding this comment.
I think this apply method is a good idea - I wouldn't have thought of doing it this way - but it's good. Might be nice to offer another version more explicitly named like fromIndex (helps people find what can be done via their IDE), and have this apply method delegate to it?
|
Follow up thought: The point of this is convenient use of tilesets so that people can focus on their game building. What other things would be helpful here that we can do for them? val maxRows: Int = ???
val maxColumns: Int = ???
val maxIndex: Int = ???
val length: Int = maxIndex
def fromCoords/Point(pt: Point): Option[Graphic[Material.Bitmap]] = ???Anything else you can think of? On that last one, lots of excellent reasons to need to look up by index, but people are bad at indexes, and they might also like to just be able to look up by coordinates now and again. |
NB: Changing the class name from GenericTileset to Tileset breaks the build on my Mac laptop. It seems to be caused by a naming conflict between Tileset and TiledMap's TileSet (note the capital S). Perhaps there is something peculiar to my environment that makes the build case-insensitive?
No description provided.