Fix multiple external tilesets

[lfraker]

Fix for the issue raised in #35. This is a breaking change as some public-facing contracts are being changed.

Updated tile_map_parser to take a list of external tileset TsxProviders. The parser now expects a list of loaded external tileset data for each external tileset. Each TsxProvider must also provide a method of getting the filename (the value stored in the 'source' property of a .tmx that references an external tileset. When parsing the tileset, the parser compares the tilesets 'source' property to the TsxProviders filename to find the correct TsxProvider. This assumes that the 'source' property is unique across tilesets, which seems reasonable. This allows for supporting multiple external tilesets in a map.

This is different from the solution discussed in #35. Since the Flame engine's method of loading tilesets required an async call, adopting the discussed approach (one TsxProvider that loads external tileset data on demand), would have required async code to be propagated throughout the parse tree. It seemed cleaner to generate the list of TsxProviders outside, then select the appropriate TsxProvider at parse time. I am open to making the update if the async propagation route is preferred though.

[spydon]

Lgtm! Some dartdocs of the changed methods would be good.
I know there weren't any previously, but always good to make it better. :)

[lfraker]

Sure thing, will push an update with some comments later today.

[benni-tec]

@lfraker I just encountered #35 and this PR while trying to load tsx files relative to the tmx file.
What was your reasoning for passing the filename to TsxProvider.getSource if the Provider already needs to know the filename of the tsx it can provide?

I would appreciate a clarification 😄

[lfraker]

@benni-tec unfortunately it's been a pretty long time since I looked at this code (I'm no longer actively working on the project that leveraged Flame Engine), but I'll try to answer as best I can!

In short, I didn't implement or change this method. I just added a comment in the commit to the Tiled repo (since there were no comments), and left it untouched in my commit to the corresponding Flame repo flame-engine/flame@80a483f. The filename parameter wasn't used in the FlameTsxProvider implementation, so I'm not sure why it was passed in initially. However, given that existing projects may have had custom implementations of TsxProvider, I didn't want to break a public contract with my changes and left the method as is. I introduced an additional getCachedSource method as a wrapper around getSource that takes no parameters in the FlameTsxProvider class.

If you're implementing a custom TsxProvider with similar caching behavior to the FlameTsxProvider, you could just implement getCachedSource (as long as it doesn't return null, the parsing code won't fall back to calling getSource). You could also implement getSource in a similar manner that just ignores the input parameter. If you're using the FlameTsxProvider and just parsing your Tiled Map with external Tilesets, this should hopefully all be abstracted from you.

Are you running into issues when loading a Tiled map with external Tilesets? Happy to help if I can!

[benni-tec]

Ah yes legacy contracts that makes sense!
I did not run into issues using this class/function, however I was curious what the purpose of that paramter was! Thanks for the explanation.

As mentioned in #70 I'm resolving tsx files relative to their tmx, therfore I'm currently generating TsxProvider's for each file before parsing with this package by parsing the tmx myself.

Just got curious as to the reason for the parameter while looking into this!
Thank you so much for the quick explenation 😄