World File Parsing

[JtotheThree]

Adds feature to parse Tiled World format.

Includes the ability to parse patterns as well (I think this will cause oddities with the ResourceReader though if the other files aren't preloaded into the reader).

serde and serde_json have been added as dependencies.

I tried adding an option to pre-parse all of the TMX files referenced in the world file, but I ran into the same issue which I think the patterns might have as well, the files won't exist in the "folder" if they weren't already loaded into the ResourceReader.

I think loading all the TMX files might make sense to expect it to happen downstream.

The ResourceReader concept is a bit new to me, so I'm going to open this PR and hopefully someone will have advice on which direction to take it. I'm also not using the cache in the World parse_xml, I'm not sure if it's needed?

[aleokdev]

Thank you! I've got a few comments. Also please don't forget to update the CHANGELOG.md.

[aleokdev]

Seems like some of the README examples broke, maybe because of some compiler update that changes how paths are handled with include_str!. Could you please fix them as well? Should be as easy as replacing Ok(Cursor::new(include_bytes!("../assets/tiled_xml.tmx"))) by Ok(Cursor::new(include_bytes!("assets/tiled_xml.tmx"))) and Ok(std::io::Cursor::new(include_bytes!("../assets/tiled_csv.tmx"))) by Ok(std::io::Cursor::new(include_bytes!("assets/tiled_csv.tmx")))

[bjorn]

Some further feedback from my side. Also I think this PR should target next instead of current (but that can be changed without opening a new PR).

[JtotheThree]

Thanks for taking the time to provide thoughtful review. I haven't contributed much to open source yet but I would like to change that so I really appreciate it.

I think the last couple of things I'm not certain on.

match_filename vs match_path. I've provided reasoning in the comment above, but my reasoning is that Tiled doesn't support actual paths in the world files, the tmx files are expected to be relative to the world file.

I went with Result instead of Option since regular expressions can be really frustrating to work with. That way the user doesn't provide a list of files and just get None back. This has fallen a bit short with testing a filename against every pattern in the world file since it's to be expected that some of the patterns won't fit so it doesn't make sense to return why. I still think it's OK as we can return an error if the arithmetic overflows instead of panicking. The world file patterns are hand edited by the user so they are more prone to errors.

[JtotheThree]

I'll look into rebasing to next.

[bjorn]

I'll look into rebasing to next.

I've changed the target branch to next. It looks like there are no further changes necessary. We were lucky this time because no new commits had been made to current yet that hadn't been already merged to next. :-)

About the latest version:

• I kind of liked that it was possible to match with a single WorldPattern against a path before, and had expected the World::match_path to just loop over the patterns and use the existing function. Even if you think such API isn't useful, I would still suggest doing this with a private function for cleaner code.

• Since Path::to_str might involve a check on valid unicode, it would be good to do it only once per path when iterating the patterns.

I hope you're not growing tired of the back and forth with the reviews! I have a feeling these will be my last concerns, but let's see what @aleokdev will say. :-)

[JtotheThree]

* I kind of liked that it was possible to match with a single `WorldPattern` against a path before, and had expected the `World::match_path` to just loop over the patterns and use the existing function. Even if you think such API isn't useful, I would still suggest doing this with a private function for cleaner code.

I think that's a good suggestion. I've added it back to the WorldPattern impl as public and World now utilizes it.

* Since [`Path::to_str`](https://doc.rust-lang.org/std/path/struct.Path.html#method.to_str) might involve a check on valid unicode, it would be good to do it only once per path when iterating the patterns.

I added a single conversion before the iterative call.

` pub fn match_path(&self, path: impl AsRef) -> Result<WorldMap, Error> {
let path_str = path.as_ref().to_str().expect("obtaining valid UTF-8 path");

    if let Some(patterns) = &self.patterns {
        for pattern in patterns {
            match pattern.match_path(path_str) {`

WorldPattern::match_path makes the same conversion though. My understanding is that since a &str is passed in the World::match_path call that it'll fall through without the valid UTF-8 check so it's still an improvement.

I hope you're not growing tired of the back and forth with the reviews! I have a feeling these will be my last concerns, but let's see what @aleokdev will say. :-)

I don't mind at all. I don't have a lot of experience writing generalized public libraries so this is very educational for me and I appreciate you making the effort for review.

[bjorn]

Looking great now! Just left a few small comments.

[aleokdev]

We should probably also have a build check without all features. However this could be improved later on using cargo hack to check with the powerset of features instead. So all good here for now.

[JtotheThree]

Added the match_path_impl pub(crate) function and updated the public interfaces to use that internally.
Update the readme to reflect a version change.

[JtotheThree]

Ok, I think that's everything.

There's some oddities with the readme include_bytes paths. On Mac and on the automated build the paths work as "assets/" but on my Linux machine it wants them back as "../assets". I run the tests from CLI so nothing like vscode would interfere with that.

[bjorn]

No further comments from my side. Thanks for this nice addition!

[aleokdev]

Thank you!