Add layer offsets, dynamic object/tile creation, isometric support, and TexturePacker support #1
Conversation
The previous condition for the if loop ONLY worked if both the objType and name were BOTH present. If a user wanted to filter by just the objType or just the name, it was not possible. I've rewritten the code to allow this feature.
This method is nearly identical to map:getObjects and should be removed.
So by removing getObject and unpacking getObjects you can obtain the
same functionality very easily.
If a user just wants a single object all they need to do is:
single_object = map:getObjects()
Regardless if other objects match, they will be discarded as the
variable assignement will only happen to the first object in the table.
The same is true for the second, third, etc. objects.
Ie. object_a, object_b, object_c = map:getObjects()
If a user desires multiple objects then they can pack them into a table
easily:
object_list = { map:getObjects() }
Refactored the code so that the object creation was put into a local function. This function is then copied to the map method.
This method assumes the player is only going to be creating an object layer using a name as an argument.
This makes method declaration easier. Additionally this decouples the initialization process from everything else. This was becoming a problem when I was trying to reuse code in the initialization for methods, so this way will allow me to get around that without changing too much. (other than making a neater code structure)
Also changed map.group to be the display.newGroup(). This may change later if I can find a solution so that the display.newGroup() metatable doesn't conflict with the Map metamethods and metatable.
Already been declared via ipairs loop below
Trying to cleanup the Map:new function
Saves us some code since both layer.types get put into map group regardless.
Also applied saved json data to map table.
This is a WIP.
TO DO LIST
|
Also shortended the variable name staggered to stagger
This got added in on accident when I was testing things. My bad!
|
Alright so here is a rough summary of the changes made. Animation StuffChanged TexturePacker SupportTexturepacker support is now successfully enabled. A user has several ways of loading the sprites into Berry.
For 1 & 2 both the image and lua file names must be the same. (items.png/items.lua, floor.png/floor.lua) If the image name is different from the lua file name for whatever reason, the 3rd option is how you load the files instead. All that is needed is a path for the image file and a path for the lua file. (so you could have items.png for the image sheet and then objects.lua for the code) Once the sprites are loaded they can be added to the map very easily. Just use One thing I may look into later is applying Map CachingI also gave a map it's own cache variable. Unsolved Math ProblemsAs I said in the discord, I am having issues translating the x/y position of the texture packer sprite for isometric maps. You may want to take a look at that and see if you can solve it. Also I did notice that when Minor Code ImprovementsLast thing for me to nitpick is that I see you have made some really long ternary operations in the code that I can't make sense of. Generally it's a good idea to encapsulate that into a descriptive local variables that will help both other coders and yourself later on if you come across the same code and forget. So instead of: if objectgroup and objectgroup.objects and #objectgroup.objects > 0 then
...can be rewritten as: local has_objects_present = objectgroup and objectgroup.objects and #objectgroup.objects > 0
if has_objects_present then
...Other than that, I'm glad this is finally being closed to finished. Took a lot damn longer than I expected. |
|
These are the files and images I used to profile the code. Unfortunately, there wasn't much of a performance boost. I finally realized why. I had initially assumed that the old version of berry was sorting through every tile for all tilesets in a map. Turns out this was not the case. Instead it filters by using |
|
This is the texture pack I was testing. Try placing it somewhere in an isometric map via The code is elseif object.sprite then
local image_sheet, frame = getImageSheet( map.image_cache,
object.sprite )
local width, height = getImageSize( map.image_cache, object.sprite )
image = display.newImageRect( layer, image_sheet, frame, width, height )
if map.orientation == 'isometric' then
image.x, image.y = isoToScreen(
object.y / map.tile_height,
object.x / map.tile_height,
map.tile_width,
map.tile_height,
map.dim.height * map.tile_width * 0.5
)
image.anchorX, image.anchorY = 0.5, 1
elseif map.orientation == 'orthogonal' then
image.anchorX, image.anchorY = 0, 1
image.x, image.y = object.x, object.y
end |
When a sprite is inserted into a layer, it apparently inherits the position of both the map and layer already. No need to add the map/layer x/y positions directly to the sprite x/y.
So this code was used from Simple Tiled Implmentation for Love2Ds version. In their code they start counting rows and columns at 1, instead of Berry which is 0. Due to this, the layout for all staggered maps was off slightly. By checking for the remainder of 1 instead of 0, this fixes this issue.
|
Alright it is 100% finished and ready for testing. |
Due to the way `getObjects` works now it no longer returns a table automatically. So if you want to sort through the returned objects like a table, you need to put them in curly brackets.
Consider this PR an unfinished WIP, since I"m trying to get feedback for the final changes that need to be made. With that said, I'm going to run through a list of the commits made and explain my reasoning.
Layer Offsets
Starting with the first few commits, I added layer offsets to all objects. It was trivial to add and implement this feature.
In Tiled a user can manipulate a layer by horizontal or vertical offsets. Neither Berry or Pontiled surprisingly supported this feature. While I doubt many users will absolutely need this feature, it is definitely nice to have. (especially useful for isometric layers)
The only things I have tested layer offsets with are isometric and orthogonal tiles. At some point polygons and other objects (text/rectangles) will need to be tested as well to confirm it's working for all objects.
Isometric Support
Commit b17c034 adds support for both regular isometric and isometric staggered map types. This took a while to get working. Isometric staggered was the most challenging because I was not aware there were extra variables involved. Notable Tiled has a
staggeraxisandstaggerindexvariable that appears in the JSON file for only staggered isometric maps. I used Simple Tiled Implementation for Love2D as a reference when making the staggered map type. The relevant code implementation can be found at this location.I have tested both map types and they appear on screen but the map position is not centered. Definitely going to need to troubleshoot and fix this at some point.
Method
map:getObjectsImprovementsCommit 169d9e4 lets a user filter an object by either objType or name. Previously you could only filter by both.
Commits 678bf93 and 1877e0e combine
getObjectfunctionality into thegetObjectsmethod. Both methods were nearly identical and IMO there was no need to distinguish between plurality of the object(s). I wrote a commit message in 1877e0e explaining the finer details how this works and why.Major Revision Warning
So all my commits after 1877e0e required me to make a major refactor to Berry. I don't expect you to accept all the features past this point. You need to weigh the benefits and whether or not that is the direction you want to take your library. The above commits I'd argue are a standard necessity that add basic features. Everything in my next commits add some features users may want, but stuff they definitely might not need. You may also not like the way I have refactored things, so feel free to say no or request a revision.
With that said, I do like that you have refactored Berry to be a more elegant and smaller version of ponytiled. Ponytiled had a lot of public methods that an end user was not realistically going to be using.
Since my game needs to have dynamic objects generated after a map loads I had to rip out the tile and object creation code and put them into their own functions. This was a complex endeavor because a lot of that code was coupled together.
Another problem I came across was that it was hard to decipher the proper scope for variables being used since they all were declared on the same lines.
When in reality the scopes were something like this:
Onto the next changes!
Refactor createTile and createObject into their own functions
99b4974 rips out the code in M.new() and puts them into their own functions. The reason we are doing this is so we can reuse the code when we want to dynamically generate objects later after the map has been loaded.
Add
map:addLayermethod to BerryCommit b21679d adds a method for a user to add a new object layer after the map has been loaded. Note - the added layer is only an object layer. The only argument is a name, which names the object layer. My reasoning behind this is the only dynamical content a user might want to add is going to be objects. Adding a tile to a tile layer or adding a new tile layer after the map has been loaded seem like it would be pointless not to mention painfully difficult.
Initially I had designed the method to handle both tile and object layers and even a option table for an argument to fill the values, but realistically it would be overkill. Most users are only going to want to add an object at x/y coordinates.
a57aa34 makes the method also returns the layer after it's added. This saves a user from having to write another line of code to get the layer or they can just ignore the returned value. Lua default library does this with some of the functions and I think it's smart.
Change module and map names
e5b85a8 changes the module's name to
Mapinstead ofM. I also was in the process of trying to setup the metatable for the map instance, but found out later that a display group already has a metatable! I got around this by adding the Map methods directly to the display group via the functionsetupDisplayGroupin 14c4b8aChange variable names to be assigned by pairs and ipairs
I noticed you were ignoring the variable assignment of the pairs and ipairs operators which could have saved a few lines of code. This is one example but there were a few others I encountered.
Can be rewritten as:
I fixed this with 10638f1 and a few other misc commits.
Attach data variables to map and layer
Since I had split the createTile and createObject into their own functions the data variables were not in scope. To remedy this I attached the variables to where they were needed.
Change
tileLayer,objectLayer, andlayerThere was no need to have a different display group for
tileLayerandobjectLayer. Additionally thelayervariable name was taken by the json.layer.data, so I just renamed this toinfo. Nowlayeris the sole display group for bothtileLayerandobjectLayer. This made the code shorter and easier to understand IMO. I kept getting confused which layer was doing what. These changes happened in 99164c0 but I forgot to mention it.