Uberi / Minetest-WorldEdit

The ultimate in-game world editing tool for Minetest! Tons of functionality to help with building, fixing, and more.
https://forum.minetest.net/viewtopic.php?f=11&t=572
GNU Affero General Public License v3.0
162 stars 81 forks source link

reduce the .we files sizes #59

Closed HybridDog closed 9 years ago

HybridDog commented 10 years ago

Lots of information about each node was saved into the .we files and the files were fairly big, so I disabled adding infos about meta, param1, [...] if it's not necessary.

I also think about saving node data into pictures (and adding meta to a separate file) and converting these images for a smaller size.

sfan5 commented 10 years ago

and converting these images for a smaller size.

How do you want to do that in Lua?

HybridDog commented 10 years ago

How do you want to do that in Lua?

I would do it with os.execute (saving data into pictures would be an optional, additional option).

sfan5 commented 10 years ago

Sorry, @ShadowNinja doesn't want you to do that, see minetest/minetest#1606 (This is to be taken with a grain of salt.)

HybridDog commented 10 years ago

Sorry, @ShadowNinja doesn't want you to do that, see minetest/minetest#1606

I think ShadowNinja currently can't watch my not uploaded mods.

ShadowNinja commented 10 years ago

The idea behind this is good, but the small option is unnecessary, there's no reason that I can think of that you'd need it to include all fields. Also, rather than removing/adding empty meta entries after the fact they shouldn't be added in the first place.

Storing nodes in images wouldn't be very good, that's not what images are for. An array of MapNodes would be good, but that's basically what Minetest schematics are.

os.execute() would be a terrible way to work on images, not only because it's insecure, but also because it's non-cross-platform and slow.

Also, a way to compress this further would be a minetest.{de,}compress() API with ZLib, which I may add soon. The downside would be readability.

Uberi commented 10 years ago

Does the small version break forward/backward compatibility? Can we load the new format in old versions of WorldEdit? Can we load old formats in the new WorldEdit?

ShadowNinja commented 10 years ago

@Uberi: Old shematics in new WorldEdit works, new schematics in old WorldEdit doesn't; which doesn't seem like a big issue to me.

@HybridDog: I also noticed that you clear the x, y, and z values if they're 0. For those values 0 doesn't mean "unused" or "empty", and they will rarely be 0, so they probably shouldn't be cleared. param1 and param2 are fine though.

HybridDog commented 10 years ago

I also noticed that you clear the x, y, and z values if they're 0. For those values 0 doesn't mean "unused" or "empty", and they will rarely be 0, so they probably shouldn't be cleared. param1 and param2 are fine though.

Yes, I made it smaller that I can read .we files of big areas even with gedit. And if the area is big, it doesn't help enough if the 0 coords are nil. And I think about adding small as a param.

Uberi commented 10 years ago

This seems fine to me, but because this is not forward compatible we have to bump the serialization format version to 5, and update version detection in worldedit.valueversion.

I also think this would be a good opportunity to change the "param1", "param2" fields to "p1" and "p2", respectively, since they are shorter and still maintain readability. This would also have the benefit of making the changes to worldedit.valueversion much simpler.

ShadowNinja commented 10 years ago

I don't think this is ready because:

  1. It uses a small parameter, which has no reason to be false.
  2. It removes fields after adding the data to a table instead of not adding the fields in the first place.
  3. Same as 2, but vice-versa with reading.
  4. It uses two tables and two loops to remove/add fields where only one is needed (although that entire part should be removed as in 2 and 3.

I checked worldedit.valueversion and it's terribly hacky. You should add a version field to the file and have that function read it, like <Version number in ASCII decimal> ':' <Data>, for example 12:{{x=0, ...}, ...}. Also, just changing the version is pointless: old versions of WorldEdit will still try to load it and fail, and new versions will load old files properly without complaint.

HybridDog commented 10 years ago

I don't think this is ready because:

  1. It uses a small parameter, which has no reason to be false.
  2. It removes fields after adding the data to a table instead of not adding the fields in the first place.
  3. Same as 2, but vice-versa with reading.
  4. It uses two tables and two loops to remove/add fields where only one is needed (although that entire part should be removed as in 2 and 3.

I added the small param to be able to save it without shrinking too because I thought removing empty stuff would slow down the save function and the file can be changed with a text editor (not gedit) too and maybe faster because lua doesn't have to be used. I haven't added the small param to the commands yet.

ShadowNinja commented 10 years ago

Currently it does take longer because it's done seperately, but if it were inlined it wouldn't be as long. And in any case the speed difference will be negligible, especially compared to the time spent reading the nodes and their meta. Also, the files are still human-editable with or without the reduction, you can just add fields if you need them. Finally, Lua is obviously used for the entire process, you're not preventing Lua usage.

HybridDog commented 10 years ago

Currently it does take longer because it's done seperately, but if it were inlined it wouldn't be as long. And in any case the speed difference will be negligible, especially compared to the time spent reading the nodes and their meta. Also, the files are still human-editable with or without the reduction, you can just add fields if you need them. Finally, Lua is obviously used for the entire process, you're not preventing Lua usage.

ok, I'll change it

ShadowNinja commented 9 years ago

This can be closed now since #61 replaces it.

HybridDog commented 9 years ago

ok, is minetest.compress() used, too?

ShadowNinja commented 9 years ago

@HybridDog: No, I've delayed that until after the next release so that users of stable MT versions can read schematics created by dev MT versions.