Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Player skinning: Enhanced player_api, skins and skin_admin mods #2122

Closed
wants to merge 6 commits into from
Closed

Player skinning: Enhanced player_api, skins and skin_admin mods #2122

wants to merge 6 commits into from

Conversation

bell07
Copy link
Contributor

@bell07 bell07 commented May 14, 2018

As mentioned in #1965 this is my proposal for skins implementation for minetest_game

Forum discussion around this pull: https://forum.minetest.net/viewtopic.php?f=9&t=20682

player_api - enhanced

  • manage the skins list and model assignment
  • "default" and "default_sprite" skins provided by player_api
  • Skin modifier functions cab be regisered
  • On Skin Change functions can be registered
  • Provides read_textures_and_meta() function for other mod to read skin files in standardized way
  • Auto assign player named skin to player by default

How it works

Basics

1.1. Registered Skin is basically a texture, but could be a table with multiple textures
1.2. There is a default model, but for each skin an model override could be assigned (seel sam vs sprite)
1.3. The skin assingment to player can be changed. The model is reassigned automatically based on skin setting (or default).
1.4. A skin could be dynamically trough skin modifiers

Dynamic skin updates

To update the (dynamic) current skin visual update_textures(player) is called
2.1. Get current skin and build/copy the a textures table
2.2. Call all registered skin-modifier functions to change the textures table
2.3. Call the model:skin_modifier() function to change the textures table for model specific adjustments
2.4. Apply modified textures to player and store in lua-table for globalstep animation updates

If the skin is changed

3.1. Update textures
3.2. Store assingment in player meta if it is not initial assignment
3.3. Call all registered on_skin_change functions

Skin defnition:

Basically a registered skin is just a table with some attributes. The required attributes are the registered "name" and the texture (or textures). All other data is optional metadata. The next metadata is "standardized" to allow interact different mods with the same skins.

Parameter Meaning
name Skin technical name as registered (always set by player_api.register_skin())
model_name Player model to be used with the skin. If not set, the default model is used
texture Single file texture, will be used as textures = { texture }
textures Skin textures table
Other mods require or use additional attirubes that can be added in registration
format Skins format. "1.0" (default) or "1.8"
formspecs related
description Descriptive skin name to be shown in formspecs
preview Skin preview image to be shown in formspecs
author Skin author to be shown in formspecs
license Skin texture license to be shown in formspecs
Skins list related
playername Private skin, to be used only by given player
in_inventory_list If set to false the skin is not visible in inventory skins selection but can be still applied to the player
sort_id Sort order in skins lists. If not given, the skin name or key is used

The API support reading the metadata from text files in meta folder. The metadata file syntax is
key = "value",

skin_admin - new mod

Chat-Command "/skinadm" for server admins to reassign any skins to any player

skins - new mod

Based on #1967 proposal a skins manager for players that allow to select skin on sfinv page

@bell07 bell07 mentioned this pull request May 14, 2018
@paramat paramat mentioned this pull request May 14, 2018
@paramat
Copy link
Contributor

paramat commented May 15, 2018

Thanks, i was intending to ask you if you might be interested in writing skins code for MTG.

@bell07
Copy link
Contributor Author

bell07 commented May 16, 2018

My "full-blown" skinning version is skinsdb. Should I apply additional features to spsp from there? Maybe the "private" and "not_in_inventory" flags + the "Simpler skins"-UI + preview using perspective change?

I personally like more the preview images on buttons because this way it is possible to see alls skins appearence side-by-side. But agree additional preview textures arent good.

@paramat
Copy link
Contributor

paramat commented May 16, 2018

I feel it's best to wait for more input first, it will be a while before i look at this carefully, and MTG dev is slow recently.

@bell07
Copy link
Contributor Author

bell07 commented Jun 21, 2018

Added a new hook player_api.register_skin_modifier((layer, modifier_func) to be able to easy suport skin modifiers like wielded-items, armor or clothing.
@stujones11, can you look trough this PR changes

@bell07
Copy link
Contributor Author

bell07 commented Jul 11, 2018

Some additional fixes done. Started work porting other mods to use the API changes: https:/bell07/minetest-player_api_modpack
The compatible skinsdb is "skinsdb5" now.

@rubenwardy
Copy link
Member

rubenwardy commented Jul 11, 2018

Two things:

Firstly, I don't like the name spsp and have no idea what it means

Secondly, in the future I'd like player_api to facilitate multiple skin mods working but not conflicting. In practice this would be done by doing something like this from a skin mod:

player_api.register_skin_provider("mymod", function(player)
    local options = player:get_meta():get("mymod_option")
    -- actually set skin
end)

function set_skin(player, options)
    player:get_meta():set_string("mymod_option", options)
    player_api.set_skin_provider(player, "mymod")
end

Haven't read this mod yet, but I think it's important to design it in such a way that adding the above will be possible in future

@bell07
Copy link
Contributor Author

bell07 commented Jul 11, 2018

Thank you for suggestions
spsp is just "Simple Player Skins Provider", but we can rename it.

The skins provider does not need to be registered. A Skins provider is just a mod that does register skins. The registered skin does have minimal definition: textures + model (optional).

The "Skin modifiers" (clothing + armor) could be registered using player_api.register_skin_modifier()

@bell07
Copy link
Contributor Author

bell07 commented Jul 11, 2018

Added some additional changes:

  • player_api.set_textures(player, textures) => renamed to player_api.update_textures(player) without the textures parameter. Textures should be calculated by framework as described above.
  • Added support for hook functon model.skin_modifier(model, new_textures, player)

I follow the next concep to calculate skinst:

Basics:
1.1. Registered Skin is basically a texture, but could be a table with multiple textures
1.2. There is a default model, but for each skin an model override could be assigned (seel sam vs sprite)
1.3. The skin is assigned to player. The model is assigned automatically based on skin setting (or default).

If a skin visual update requested: function player_api.update_textures(player)
2.1. Get current skin and build/copy the a textures table
2.2. Apply all registered skin-modifier functions in sorting order to the textures table
2.3. Apply the model:skin_modifier() function to the textures table
2.4. Apply modified textures to player and store in lua-table for globalstep updates

@bell07
Copy link
Contributor Author

bell07 commented Jul 15, 2018

Did some cleanup. 1-texture skins can use "texture" attribute now, the "textures" is reserved for multi-textures skins now. Added documentation about possible skin attributes to the first post and to the player_api/skin_api.txt file.
Any additional attributes are missed to be standardized?

@bell07
Copy link
Contributor Author

bell07 commented Jul 15, 2018

Just for information: If spsp is to simple, I stripped down the "Flexible Skins API" from skinsdb and adjusted according the proposed API changes. Maybe the "skinsdb5" is a candidate for minetest_game now?
https:/bell07/minetest-player_api_modpack/tree/master/skinsdb5

@bell07
Copy link
Contributor Author

bell07 commented Jul 16, 2018

Last but not least added the skins mod from #1967 and ported to the player_api enhancements. All mods (player_api, spsp, skins + all of player_api_modpack) does work together now trough enhanced API. All puzzle parts are done, now please decide which of them should be added to the Minetest Game.

@bell07 bell07 changed the title Player api skins spsp Player skinning: Enhanced player_api, skins and spsp mods Jul 16, 2018
@bell07 bell07 changed the title Player skinning: Enhanced player_api, skins and spsp mods Player skinning: Enhanced player_api, skins and skin_admin mods Jul 16, 2018
@bell07
Copy link
Contributor Author

bell07 commented Jul 16, 2018

Did some cleanup. Moved the private skins assingment from spsp to player_api (to be default behaviour) and renamed spsp to skin_admin. The reason for renaming is the mod contains only the admin chat command for skins reassignment.

Updated the initial pull request text. Now I am satisfied the mods could be merged to minetest game. Of course the documentation needs to be done, I cannot do it because of my still insufficient english skills.

@bell07
Copy link
Contributor Author

bell07 commented Aug 21, 2018

Forum discussion around this pull: https://forum.minetest.net/viewtopic.php?f=9&t=20682

@bell07
Copy link
Contributor Author

bell07 commented Aug 21, 2018

@paramat, you mentioned there is a way to swich automatically into third person view. I do not see it in lua_api.txt. Can you please provide the piece of code for "skins" mod?

@paramat
Copy link
Contributor

paramat commented Aug 26, 2018

Sorry for delayed reply. I was wrong, i don't think there is a way.
My skins mod code is here #1967

@bell07
Copy link
Contributor Author

bell07 commented Aug 26, 2018

Ah, ok. The "skins" mod in this pull is based on your code in #1967

@bell07
Copy link
Contributor Author

bell07 commented Jan 10, 2019

Rebased. Set preview image for demo sprite skin. Semi-standard as used in new skinsdb5

@paramat
Copy link
Contributor

paramat commented Feb 8, 2019

I'm still interested in this, thanks for the work. We may be considering skins issues after release, possibly as a way to provide male and female default skins.

@bell07
Copy link
Contributor Author

bell07 commented Mar 14, 2019

rebased against stable-5 and did a bugfix

@binyamin
Copy link

@paramat I see that @bell07 isn't doing modding anymore. I would be willing to pick up his trail. What needs to be done for this PR to be merged?

@paramat
Copy link
Contributor

paramat commented Apr 23, 2019

This isn't being delayed by changes needed, more core devs (me included) need to consider and review it. As usual core dev time is the 'bottleneck', not contributors.
It's uncertain whether bell07 will now neglect this, they may maintain it.

@bell07
Copy link
Contributor Author

bell07 commented Apr 23, 2019

I am still available advisory and can still do small adjustments on my work if needed. But nothing pro-active anymore, just because I do not use/play it. I am glad to see someone wants to take care of my work. I think this PR we can get into Minetest Game together

@binyamin
Copy link

binyamin commented Apr 30, 2019

@paramat

...more core devs (me included) need to consider and review it.

Maybe make a note of that. Use github's review function, or make a label. That way, the current stage of this PR is understood.

@paramat
Copy link
Contributor

paramat commented May 1, 2019

b3u, no need for notes or comments, the PR is like every PR in that it needs core dev attention.
Skins is in the MTG roadmap, is important, but not urgent.

@bell07
Copy link
Contributor Author

bell07 commented Jun 6, 2019

It is possible to have Skin-preview for formspec based on the skin texture. I found it in https:/GreenXenith/skinsdb/blob/master/skin_meta_api.lua (Thanks to @GreenXenith) and copied to the skinsdb5. So may this skin manager can be added to the minetest_game / to this PR?

@bell07
Copy link
Contributor Author

bell07 commented Jun 7, 2019

Rebased and add parameter to force update to update the same skin. Required for character_creator preview update

@bell07
Copy link
Contributor Author

bell07 commented Dec 3, 2019

Rebased against origin/stable-5

@bell07
Copy link
Contributor Author

bell07 commented Dec 3, 2019

rebased to origin/master

@bell07
Copy link
Contributor Author

bell07 commented May 25, 2020

squashed and rebased agains current stable-5. Merged current origin/master. Did some modernizations

@bell07
Copy link
Contributor Author

bell07 commented May 25, 2020

travis error are false-positives, related to the unified_inventory integration in skinsdb5.
False-positive because of

if minetest.get_modpath("unified_inventory") then
	dofile(modpath.."/unified_inventory_page.lua")
end

@paramat
Copy link
Contributor

paramat commented Aug 2, 2020

Keeping this feature PR open due to at least 2 core devs supporting and the amount of work that has gone into this.

@bell07
Copy link
Contributor Author

bell07 commented Sep 6, 2020

Found some time to work again on my player_api modpack. Updateted this PR by the way.
Removed the skinsdb5 mod again because the preview generation is splitted to separate mod. You can still check the mods in my player_api_modpack that contains all mods already ported to my player_api changes.
By the way, the newhand mod is ported in the modpack so each skin shows the 3d arm matching the skin in the modpack.

Changes in the last PR to the player_api:
Added metatable to all skins for dynamic values calculation for missed attributes. The framework part can be used with player_api.register_skin_dyn_values(key, hook) See how it works for skin.texture or skin.model_name or in modpack for skin.preview.

Removed the "hook" from player_api.read_textures_and_meta() Beter way is to replace the player_api.register_skin() function with additional code. For To be able to detect file format, the skin.filename is added in player_api.read_textures_and_meta() by default.

fixed also formspec issue in the skins mod

@sfan5 sfan5 closed this Jun 1, 2021
@bell07
Copy link
Contributor Author

bell07 commented Jun 2, 2021

Why closed without comment?

@sfan5
Copy link
Member

sfan5 commented Jun 2, 2021

Minetest Game has been frozen for new features for a while now (#2710), with that plus very low dev interest this has practically zero chance to be merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants