-
Notifications
You must be signed in to change notification settings - Fork 87
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Can't be used with Web or Android target in Unity #32
Comments
Yes, thanks for raising this issue. This particular issue can be rolled into the more general PCL support problem (#15), but I'll leave this issue open to highlight the problem. Unfortunately I haven't put much effort into this for a long time, but the most likely solution is to replace filepaths with some sort of File Object (also alluded to in issue #29). But I haven't done much with file objects in C#, so am not sure how to do this in a safe way. I can try to take another crack at this over the next few days, but feel free to offer your own suggestions. As mentioned in #29, I have no problems with changing the constructor or other parts of the API. |
The simplest fix would be to just add another constructor that takes an XDocument directly, and have the file path constructor call into that. Anything that needs to pass it an XDocument created using something else could use that new constructor and it wouldn't break anything else. Edit: |
I have some local changes that support opening maps from streams, that might work. I'll try to remember to clean that up and submit a pull request. |
Workaround code snippet for people encountering this issue as well: private const string TempFilePath = "./streamingasset.tmp";
public static IEnumerator LoadTmx(string assetPath)
{
var fullPath = Application.streamingAssetsPath + assetPath;
string finalPath;
// Check if we need to use WWW or load directly
if (fullPath.Contains("://"))
{
Debug.Log("Loading in \"" + assetPath + "\" using temporary file");
// Load the asset in from the remote path
var www = new WWW(fullPath);
yield return www;
// Unfortunately, we have to copy to a temp file in this situation
// Android and Web targets can't load directly as a file and Tmx can only accept file paths
File.WriteAllText(TempFilePath, www.text);
finalPath = TempFilePath;
}
else
{
yield return null;
finalPath = fullPath;
}
yield return new TmxMap(finalPath);
} |
Thanks @ShawnCowles, that would be very much appreciated. @LaylConway 's suggestion is also fine, although I'd like to completely remove any reference to filepaths eventually. |
As a side-note, this problem is unrelated from the need for PCL support, as Unity (and other Mono targets) aren't limited to PCL on mobile. |
@LaylConway I can't seem to be able to implement the workaround snippet you posted, could you explain it? |
@Cryru it's not needed anymore as #33 allows you to open a stream or xdocument directly from a string rather than using a file in the middle. If you really want to use the workaround, you need to call it like this:
This is because it needs to wait on WWW to finish loading. Quite a shame Unity doesn't have support for the "async" keyword because it would simplify the APIs a lot. |
Oh apparently that merge was rolled back |
Oh, why was it rolled back? Also if I'm using MonoGame instead of Unity it's the same deal right? |
I assume MonoGame will have the same issue with Web and Android targets, but the workaround is for Unity only. It makes use of Unity's way of loading external assets from Android and Web sources, which relies on the |
Oh right that makes sense. Hopefully they will return the string loading function. Thanks for clearing things up. |
Sorry for the rollback, some of my tests weren't working, but I don't think it's related to the patch. Hopefully I'll restore the changes very soon. |
Thanks! Would be nice to have a guide or something on the wiki on how to load from a string as well. |
Ok so I did manage to get this working. The difference is that I needed an explicit reference in my test project to I don't think this is a problem, and I'm happy to re-instate the patch, but I wanted to make sure it wasn't something more serious. |
I've restored the changes added by @ShawnCowles so should hopefully be working now. But before I close it, I have to say I'm surprised to see people using this and engaged enough to contribute here. Would it be worth setting up a mailing list or something around this project? Also, how would people feel if all the filepath parsing code was removed and everything was handled with |
@marshallward All of my projects use Tiled in one way or another and TiledSharp is really useful for me. Removing file path stuff and handling everything with Stream or a giant string representing the file would solve a lot of multi-platform issues for sure. I personally do not think beginners will have that much of a hard time dealing with it as reading files is one of the first things one learns, and .tmx files are as easy to read as any .txt. As long as everything is documented and explained well it will go okay. Thanks for the great work. |
@marshallward It should probably be fine for most use cases as long as you document it, it will only be one or two lines extra for trivial use. |
Because TmxMap takes a file path, it can't be used with a web or android target. Instead, there should be an option to provide a string of data.
In Unity, there's a variety of ways to load in streamed/external asset data. On Web and Android targets however, there's no way to directly open streamed data as a file. Here you would use in unity a WWW object, which on Web downloads the file and on Android loads it in from the zipped package.
https://docs.unity3d.com/ScriptReference/Application-streamingAssetsPath.html
However, WWW will only give you a text string, which can't be fed into TiledSharp.
Currently the only workaround I can figure out is to use temporary files (which on Web will make use of local storage). This is however a needlessly error-prone and badly performing solution.
The text was updated successfully, but these errors were encountered: