search

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
329 stars 87 forks source link

DeflateStream and LayerTile #26

Closed kreynes closed 8 years ago

kreynes commented 8 years ago

As of .NET 4.5, DeflateStream uses the Zlib, so, is it possible to remove the external dependancy on DotNetZip? Also, wouldn't it be more memory efficient to represent the LayerTile as a struct and not an object? Since big maps can create tons of instances of that object?

marshallward commented 8 years ago

Re: DeflateStream

Yeah, it's a good idea. I considered it at one point, but at the time Unity was still dependent on .NET 3.5, so I ultimately rejected the idea (and even stripped out a few .NET 4 dependencies).

Since then I've heard that Unity has upgraded its .NET runtime (I don't myself use Unity) so maybe there's a case to go back to this.

If you have more info on this, then I'd be interested to hear about it.

Re: structs for LayerTile

I don't know much about structs in C# other than they are a bit more than structs in C. From the little that I know, I don't believe the memory usage is much different, although I think in certain cases they can provide better performance. But if you have profiling numbers then I'm happy to consider it.

As for memory performance in general, I think there are some even bigger problems as it is. Currently the library just reads in all the maps at initialization and holds them in memory, which is already pretty silly. That's probably the most pressing issue with respect to memory.

kreynes commented 8 years ago

Well, unfortunately I have no information on the DeflateStream apart from MSDN pointing out it is now using zlib. I'm currently working on Implementing it with TiledSharp on my end, and I guess I'll see how it works out.

As for the structs vs class: Objects are allocated on the heap. So, Imagine loading 10k instances of the LayerTile class. On the other hand, structs behave like built-in types and allocate on the heap. If we would be talking about just a few instances, I wouldn't even bother, but when it comes to a huge amount like that, It's really worth considering.

As for the issues you pointed out, if anyone's using the source-code I'm sure they can modify it to their own need. Personally, keeping one map loaded is nothing terrible (although the optimal way would be to load just a portion of the map corresponding to the users viewport), unless you have thousands or tens of thousands of instances of the current LayerTile. :)

marshallward commented 8 years ago

From what I can tell, it looks like the latest Unity (5.3) is still not running on the latest .NET, so it probably still does not support zlib. So I probably can't replace the current ZLib implementation without breaking Unity support.

It's probably possible to create multiple versions for different .NET runtimes, although I've never done this before.

But I do want to get rid of the DotNetZip ZLib implementation, because it's been a barrier to converting TiledSharp to a PCL. I've just been hesitating on the best solution (add ZLib.Portable dependency, clean up DotNetZip myself, switch to .NET 4.5 but break Unity support, etc.).

As for the struct thing, you may be right about the Collection<TmxLayerTile> of ~10k objects and its references being an unnecessary burden. I don't know too much about the implementations of the various data structures or their scalability, but I am open to replacing the Collection<TmxLayerTile> with some sort of struct (I think that is your suggestion, right?) if there's a measureable advantage.

marshallward commented 8 years ago

Zlib decompression is now done using DeflateStream so I will close this issue.

The scalability of particular data structures and objects is probably best handled in a separate issue, along with an example and some profiling numbers.