TiledSharp icon indicating copy to clipboard operation
TiledSharp copied to clipboard

PCL Support

Open KevinBalz opened this issue 10 years ago • 12 comments

Would it be possible to convert TiledSharp to a Portable Class Libray ?

It would be really useful for my Monogame project, because i can't put all map related code into my Game PCL.

I've started porting, so far i've replaced the current zlib with Zlib.Portable. The main issues i encountered is the current Constructor, loading files with paths is not possible in PCL's, so you have to pass a Stream or pass a class which implements some kind of 'IMapLoader' interface, see http://timheuer.com/blog/archive/2014/05/23/porting-taglib-sharp-to-portable-class-library.aspx , which are obviously breaking changes to the current API.

So i wanted to aks if it would be ok to do this, or if there any other plans or suggestions before i continue.

KevinBalz avatar Apr 11 '15 11:04 KevinBalz

I must admit that I had not heard of PCLs until you mentioned them, so I'm happy to defer to you on the advantages of supporting them,

For the specific issues:

  • Replacing Zlib is not a problem. As far as I know, the DotNetZip version bundled into the source is not even supported anymore. Moving to Zlib.Portable sounds like a great idea.
  • Replacing the constructor could be an issue for some people, but I am not against the idea if it improves PCL support.

Personally I try to use embedded resources, rather than explicit paths. The pathname resolution is supposed to be the fallback option. But I do know of people who still rely on pathnames.

Do you think there is some way to support multiple constructors? Or would that break the PCL support?

As I said, I do not know much about this stuff, so maybe the best thing to do is start with a pull request, if you have it working. Then we can iterate from there.

marshallward avatar Apr 11 '15 13:04 marshallward

Multiple constructors could be possible, but pathname resolution doesn't work, but resource loading could work, i would have to look into that.
Maybe it's possible to make use of conditional compiling, so if IO is usable on the current plattform it will use pathname resolution, i will look into it, as im fairly new to it.
I will start by replacling Zlib, the rest could take some time.

KevinBalz avatar Apr 12 '15 10:04 KevinBalz

Hi Kevin, I just wanted to say that I haven't forgotten about this. I never heard back from Robert, and since then I've been overseas visiting my family. But I'd like to get this working when I return in a couple weeks.

I was looking into using ZipArchive to do the .zip unpacking (which is what zlib is presumably using). It is now bundled with .NET 4.5 (so I'm told) and may resolve the PCL issue that you are seeing. It would also remove an annoying dependency. (Apparently it is also slower than DotNetZip but that doesn't worry me too much.)

But I'll be able to look into all this when I return.

marshallward avatar Apr 28 '15 08:04 marshallward

I'm finally putting some time into this problem of PCL migration.

I'm relying on the Xamarin scanner to check for portability. This is the report:

http://scan.xamarin.com/Home/Report?scanId=4475954e-d645-41da-9788-516500cba1a2

Am I right that the following issues would need to be resolved before it could be compiled as a PCL? (I can see how most, but not all, of them are due to DotNetZip)

marshallward avatar Jun 19 '15 11:06 marshallward

Most are caused by DotNetZip ( because some functions of System.IO and System.Console), some are caused in the library, because System.Path strangely isn't supported that well(Combine ...) and the use of Assembly.GetEntryAssembly.

And yes you're right, the calls of unsupported libraries have to be replaced with supported libraries, depending on the configuration of the PCL ( configuration = plattforms the PCL will support ).

KevinBalz avatar Jun 19 '15 11:06 KevinBalz

OK, I think I understand this problem much better now.

Dumping the Zlib.Portable source on top of the old DotNetZip code, and switching from GZipStream to Ionic.Zlib.GZipStream seems to have sorted out just about all of the problems.

The only other major issue is the Assembly and path resolution stuff, but I reckon that there are other ways around those problems. We can talk about that afterwards.

The only other problem I see at the moment is that Zlib.Portable has some code that won't compile under .NET 3.5, which is only an issue for Unity users (which I've only just learned about this week).

I'll try to put some time into this during the weekend, though it will probably just be a zlib.portable dump into the source (assuming it's ok with the AdvancedREI authors).

I'm sorry that it took me so long to understand this issue!

marshallward avatar Jun 19 '15 13:06 marshallward

I've improved some (though not all) of the issues related to Zlib, but still have not resolved the dependencies on Assembly.

I think you are right, and that the current interface needs to be changed. I think the correct solution is to only allow an input stream containing the XML file, and leave the particulars of path or resource management to the executable.

I hate changing the current API, but the current one just has too many quirks and incompatibilities.

(And I see that was your original suggestion... oops)

marshallward avatar Jun 25 '15 04:06 marshallward

The first problem I can see is in TmxTileset, when the source attribute is set. This is basically a filename (or path) which would need to be opened. So the code still needs to have some capacity to read and open files based on filename, rather than solely working with input streams.

Now that I think about it, image resources are also referenced by their filenames in the TMX file.

But, none of this is necessarily dependent on the GetEntryAssembly call.

marshallward avatar Jun 25 '15 04:06 marshallward

The image resource path in the TMX file should be no problem, because the user would anyway load the image by themselfes.

Regarding TmxTileset: It's possible to solve this using a interface which is capable of loading files, for example:

public interface IFileLoader {
    Stream load(string path); 
}

which will then be used to get a stream of the tsx file This obviously requires the user to implement said interface and giving it as a argument to the constructor. But maybe we could make this interface optional, so if the user uses no TSX files he can use the api as before. The drawback would be the need to throw an error if the uses omits this interface but has TMX referencing TSX files.

This use of this method was inspired from here: http://timheuer.com/blog/archive/2014/05/23/porting-taglib-sharp-to-portable-class-library.aspx

KevinBalz avatar Jun 25 '15 12:06 KevinBalz

The way PCL's work will change it seems: http://blogs.msdn.com/b/mvpawardprogram/archive/2015/06/18/demystifying-pcls-net-core-dnx-and-uwp-redux.aspx

As far as i understood the article there will be no profiles you have to set yourself, instead the library will work on every plattform , as long as the library does not use a library/api call which is not supported on the given plattform.

KevinBalz avatar Jun 25 '15 12:06 KevinBalz

OK, I think I understand your idea now... so the user would create their own FileLoader class (or whatever) using the IFileLoader interface and provide it to the Load method. (Very sorry that you've had to repeat this so many times.)

I agree that would be very different, and it would shift the burden of path manipulation onto the user, in addition to encapsulating it into a proper class. But maybe that's a good thing.

I think that I am OK with removing the whole Embedded Resource scanner, which was always a bit wonky. It probably makes more logical sense to move that to my own personal FileLoader class. Same could be said for the whole parent TmxDirectory tracking.

So yeah, I think I am convinced, assuming that the transition is not too painful.

It would be nice to keep the old path management code somehow available, perhaps as example FileLoader classes or as additional projects in the solution. Defaulting to filepaths would be wonderful, although I'm guessing that would break the ability to compile as a PCL.

marshallward avatar Jun 26 '15 03:06 marshallward

I guess i'm not that good in explaining^^

Regarding the File managment, the link i've provided (the taglib example) explained a way to give the user a default implementation of this file loader, although i not fully understanded how. I would wait how .net Core will probably change the way PCL's work, maybe drastically easing the process ( having more api calls availabe etc.. )

KevinBalz avatar Jun 26 '15 14:06 KevinBalz