search

minetest-mods / skinsdb

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

Add updater script using Minetest API #27

Closed SmallJoker closed 5 years ago

SmallJoker commented 5 years ago

Requires https://github.com/minetest/minetest/pull/8573 to work

How to run it: 1) Add the mod to secure.trusted_mods 2) Uncomment the stuff in init.lua

SmallJoker commented 5 years ago

Fixes #21. The script is CC0/WTFPL (cc @tenplus1, if you want to adapt it for simple_skins).

bell07 commented 5 years ago

Tested shortly, Nice!

But..

  1. "character_1.png" is provided already in mod, but named as "character.png" to override the minetest_game default character. So you need to checkout starting by 2.

  2. The old bash script was able to get > 1000 Skins. Using this updater I got only ~100 skins, but not all. character_12 is missed for example, 13.-18 are missed too, and a lot more gaps in numeric order.

Of course the change needs to be more user friendly / useable without code change. Maybe an admin-chat-command to start the updater is more accustomed

SmallJoker commented 5 years ago
  1. Will skip that one, fine.
  2. By accident I committed a page limit of 3. Set _SKIN_PAGE_END_ to nil if you want to test it before my next commit. Also I cannot download skins which don't exist. 13-18 were removed from the repository.

My target was to provide a rudimentary but still usable skins downloader. A fancy GUI or chatcommand belongs into another PR (see file header comments).

SmallJoker commented 5 years ago

@bell07 I updated the PR accordingly. You should now skins up to ID 1400.

SmallJoker commented 5 years ago

@bell07 The PR is now merged into the engine. Do you insist on a GUI or chat command? Because I would prefer to implement that in a follow-up PR.

bell07 commented 5 years ago

I prefer to have a version compatible to minetest stable in master. Therefore unsure if it should be merged to master or to new branch "dev" till the PR above is in a released version. Or should I create an "stable" branch instead with current state?

It is ok for me to not have an gui or chat command. The updater burds the skins server, therefore should be used rare with manual effort.

Previously I had an own branch with all ~1200 skins in github to releave the fensta server. Maybe the "with_skins" branch can be revived now again with this updater?

SmallJoker commented 5 years ago

I prefer to have a version compatible to minetest stable in master.

This is the default case. It's the reason why the updating script is commented out in init.lua. However, I don't agree that there should be skin branch since git is not made for binary data, and downloading the skins is very easy.

bell07 commented 5 years ago

You are right. Maybe you add some words to the readme about the updater? Then it can be merged to master.

bell07 commented 5 years ago

@SmallJoker, I added 2x reviews. The manual addition documentation is incomplete. May the reference to textures/readme.txt is enough. And the step "comment the updater lines again out" is missed

bell07 commented 5 years ago

Thank you! If you interested on Skins related development, you can look to my "next generation" fork skinsdb5 in player_api_modpack