marshallward / TiledSharp

C# library for parsing and importing TMX and TSX files generated by Tiled, a tile map generation tool
http://marshallward.github.io/TiledSharp
Apache License 2.0
328 stars 87 forks source link

Replace DotNetZip with Zlib.Portable #16

Closed KevinBalz closed 7 years ago

KevinBalz commented 9 years ago

Removed zlib source files and added Zlib.Portable Nuget.

marshallward commented 9 years ago

Thanks for this. I will need to give it a try before I can move ahead, so it might be a bit slow.

I must admit that I am a bit worried about adding a new dependency, which might just add to the difficulty of using it. But maybe I am worrying too much; getting something like MonoGame built is far more difficult!

As for using NuGet - I have added support but don't actually use it much myself. Does this need to be included in the TiledSharp.nuspec file?

On Sun, Apr 12, 2015 at 9:06 PM, Kevin Balz notifications@github.com wrote:

Removed zlib source files and added Zlib.Portable Nuget.

You can view, comment on, or merge this pull request online at:

https://github.com/marshallward/TiledSharp/pull/16 Commit Summary

  • Add Nuget packages to gitignore
  • Replace DotNetZip with Zlib.Portable

File Changes

Patch Links:

— Reply to this email directly or view it on GitHub https://github.com/marshallward/TiledSharp/pull/16.

marshallward commented 9 years ago

Also, I haven't looked into the state of gzip/zlib compatibility with the standard .NET libraries. Maybe this is an option, which eliminates the entire dependency issue?

KevinBalz commented 9 years ago

You're right, you must add dependencies to the nuspec, haven't thought about that. src: https://docs.nuget.org/create/creating-and-publishing-a-package (example .nuspec) .
I'm no nuget expert either, so i will look for a way if it works.
And System.IO.Compression would be sufficient if Tiled would only support gzip compression, zlib compression seems to be different, but i can't tell how and why as im no compression expert also.

marshallward commented 9 years ago

I have read that zlib and gzip libraries can often be used together, since zlib is capable of writing gzip headers, and gzip is capable of understanding the DEFLATE algorithm. But there have also been reports of bloated zlib files under .NET, and unfortunately I never really learned much about the issue.

Anyway, it is probably best to keep using a zlib-dedicated library.

BTW the Zlib.Portable README says that its work is being sent to the official DotNetZip repository. Do you know if there's any advantage to using one vs the other?

(Also, I am glad to see that DotNetZip is still being maintained!)

PS: I've emailed AdvancedREI to see which one they would recommend.

marshallward commented 9 years ago

Just heard back from Robert McLaws, Zlib.Portable maintainer (that was fast!), he suggested going with the DotNetZip version. Can you give that one a try and see if it still works with PCLs?

KevinBalz commented 9 years ago

I have problems findng the DotNetZip version ( there are a lot of packages named Zlib.Portable). To which one are you referreing to?

marshallward commented 9 years ago

Yeah... I see what you mean now.

Zlib.Portable's README points to the 2011 release, which is I was already using and therefore does not support PCLs.

The DotNetZip version of NuGet does not have any record of patches or contributions from AdvancedREI, which means this is not where they are sending their patches (unless they are just sending raw patches via email).

And the NuGet version of DotNetZip does not appear to be any more "official" anyway, but rather an effort to keep the project alive by new people.

I will contact Robert McLaws again, and find out to which repo they have been sending their patches.


Very sorry for making this more complicated than it probably needs to be. It's only that if I do add an external dependency, then I want to make sure it's the right one to add!

On Tue, Apr 14, 2015 at 5:58 PM, Kevin Balz notifications@github.com wrote:

I have problems findng the DotNetZip version ( there are a lot of packages named Zlib.Portable). To which one are you referreing to?

— Reply to this email directly or view it on GitHub https://github.com/marshallward/TiledSharp/pull/16#issuecomment-92682045 .

marshallward commented 7 years ago

The new version does not require an external zlib library, so I'll probably close this issue if there are no problems.

marshallward commented 7 years ago

Closing this, since an external zlib library is no longer necessary. Issues with the zlib changes can be handled in separate issues.