Async loading

[Debaug]

Working with async I/O is currently tricky as ResourceReader is not capable of providing a reader asynchronously. I've needed async loading in a project before, so I think adding it as a functionality would be a worthwhile change.

I've implemented a solution here. Broadly speaking, I made the following changes:

• Switched from xml-rs to quick-xml, which provides async reading. This was also proposed in Consider switching from xml-rs to quick-xml #137. A noticeable difference between these crates is that xml-rs expects a Read while quick-xml expects a BufRead. Accordingly, I've changed the Read bound on ResourceReader::Resource to BufRead. This is technically a breaking change, but is worth it in my opinion: most people probably only use FilesystemResourceReader or std::io::Cursor, which would both still work. Otherwise, wrapping a reader in std::io::BufReader should not be a difficult change (and of course, we're before 1.0).

• Added an AsyncResourceReader trait, mirroring ResourceReader, that asynchronously provides a tokio::io::AsyncBufRead.

• Removed the ResourceReader bound on Loader and moved it to the loading functions to which I added async counterparts. The alternative would be to create a separate AsyncLoader struct: this may have the advantage of better communicating intent by placing bounds on the loader types themselves.

• As for the implementation, I've gone down the somewhat 'hacky' road of taking the lowest common denominator and making the whole parsing process async. To make it compatible with both sync and async readers, I've added two new internal traits: Reader, which abstracts quick_xml::Reader and delegates to either quick_xml::Reader::read_event_into or quick_xml::Reader::read_event_into_async depending on the implementor, and ReadFrom, which abstracts ResourceReader and AsyncResourceReader by providing a Reader. On the other end, to get back to the sync world from the async parsing, Loader::load_tmx_map and Loader::load_tsx_tileset 'execute' the futures with FutureExt::now_or_never from the futures crate.

Otherwise, further work on this solution includes:

• Properly documenting everything
• Testing with an actual async reader (though there's almost no room for new errors as sync loading also uses the async parser) and maybe examples
• Feature-gating async functionality

So it's not quite ready yet.

[aleokdev]

Whoa, thanks for this, this is a lot of work! Although I haven't quite had a detailed look into your code yet, I like the approach of making internal constructors async while leaving the 'external' interface of Loader both sync and async. The switch to quick-xml is also a great decision. Just one thing to note, a rebase onto next would be needed since these changes are breaking.

[Debaug]

Thanks for the nice comments :) The readme mentions some gotchas with WASM because the crate currently doesn't support async loading. I don't know much about WASM, but I suppose it would work on WASM targets now? Do we need additional work/documentation for these targets?

[aleokdev]

I'm not completely sure, we could try compiling a simple example to wasm and see if that works. If any issues arise then we can fix those or write about how to fix them if it's some user-side thing

[bjorn]

That's some great work! I'm wondering whether you'd be willing to do a separate pull request for just the switch to quick-xml? It should be easier to review and give a chance to compare performance independently from the async change.

[Debaug]

Sorry for the late response ! I don't have much time to work on this at the moment, but I'll try this when I do.

[bjorn]

@Debaug That's alright, finding time is always the issue but at least we're not generally in a rush! You can also consider just opening a draft PR with your changes for now. Maybe somebody else can help either with finishing it or separating it into smaller changes.