search

minetest-mods / skinsdb

Player skin mod, supporting unified_inventory, sfinv and smart_inventory
27 stars 41 forks source link

updater/update_skins.py: Do not download preview files by default #51

Closed bell07 closed 3 years ago

bell07 commented 3 years ago

Two changes for the python updater script:

  1. Add leading zeros to the file names. Fill up to 4 digit characters. Previously the skins are wrong sorted in some cases (1, 11, 111, 1111, 1112 ...) (rejected / removed from PR)
  2. The python script does not download the preview files by default anymore. The skinsdb is able to compose the preview using some formspec magic. But python update_skins.py with_preview still download the preview files as before
SmallJoker commented 3 years ago

Neither the old license-incompatible nor the new Lua-based skin downloader use the zero-padding approach. It is nice to track in a file manager, but this barely happens on servers where most processes are automated - like the skin download.

How backwards-compatible would it be if a server uses this new change on a filled skin directory? I believe they would end up with duplicates and lost player skin data if the old (character_2.png) files are removed.

Other than the file naming I think this is a good addition, although server traffic will rise if bigger formspec strings have to be sent (for image composing).

bell07 commented 3 years ago

thank you for your feedback @SmallJoker .

I got the issue with the enhanced player_api fork fensta_skins because I changed the prefix from character_[number] to character_fensta_[number] to avoid overlaps to other similar mods. The sort key is "fensta_[number] now that leads to the strange sort by sting in the different skins lists (skinsdb5 or skins). But I agree the skinsdb should remain for the old format without the leading zeros for now. The fensta_skins uses the leading zeros. Other incompatibility is the meta file format that is out of topic at this place..

I remove the leading zeros from this PR. What is about the preview files change?

SmallJoker commented 3 years ago

What is about the preview files change?

Nothing. Looks good as-is.

btw: The "skin preview generator" formspec element might not work properly when high-resolution textures are used (limitation of ^[combine), hence I wonder whether there could be any side-effects if such are uploaded to the database.

bell07 commented 3 years ago

Does fensta provides high-resoluttion textures? If not, the issue is not related to the updater script. For other mods and manually added skins: The preview textures are still supported and are preferred if given. So any issue with generated preview can be solved by providing preview texture file.

Then I merge this PR if all is fine....