search

minetest-mods / skinsdb

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

Previously selected player skins are now reset to default #101

Closed Bastrabun closed 4 weeks ago

Bastrabun commented 1 month ago
  1. After update everyone will have default skin unless we fix that on the database. Could you please add a migration?

2. Could you make process_skin_texture public so that other mods can add skins as well? #102

3. The _hand property removes all separators and simply concatenates the text, that again creates ambiguity. Example: character.my_name.my_skin.png VS character.my.name_my_skin.png, both lead to skinsdb:charactermynamemyskinpng #103

For my reference 6918

SmallJoker commented 1 month ago
  1. After update everyone will have default skin unless we fix that on the database. Could you please add a migration?

https://github.com/minetest-mods/skinsdb/commit/312780c82e36178f63b092f4e4d772b69a377240#diff-b90f0e17d2c8386631457acd5f53ecf60401ceb1ef54b04a1dd29a5be826b123L20-L21

The problem here is that the internal name was also ambiguous. To fix this particular issue, we'd have to resort to guesswork, which might result in false-positive matches. If that's an acceptable trade-off, I can provide a patch for that.

  1. Could you make process_skin_texture public so that other mods can add skins as well?

This should belong into a separate issue. Trivial to add, though.

  1. (hand property)

This also belongs into a separate issue. Relevant code:

https://github.com/minetest-mods/skinsdb/blob/312780c82e36178f63b092f4e4d772b69a377240/skin_meta_api.lua#L61-L62

Unfortunately this can be tricky to fix because . is not an accepted node name character: https://github.com/minetest/minetest/blob/master/doc/lua_api.md?plain=1#L302

Bastrabun commented 1 month ago

I created new issues for 2 and 3, let's continue in this issue exclusively on 1. After update everyone will have default skin unless we fix that on the database. Could you please add a migration?

Since skinsdb writes into modstorage, I could fix on the database, but - just guessing - not many will want to do that. of course, if you detect ambiguity where the guesswork doesn't suffice, a log message during migration will have to do.