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

Fix sfinv support to retain sfinv menus. #10

Open
Skaapdev opened this issue Mar 19, 2024 · 6 comments
Open

Fix sfinv support to retain sfinv menus. #10

Skaapdev opened this issue Mar 19, 2024 · 6 comments

Comments

@Skaapdev
Copy link

Skaapdev commented Mar 19, 2024

@MrRar
Hi MrRar,

I have been looking at using edit_skin and my mod soup uses sfinv at this point in time.
It is a small annoyance but when a player clicks on the edit_skin menu it doesn't actually use the sfinv formspec.
So as a consequence there is no way to click back to other menus such as armour/craft etc.
Well I happen to just figure out how to integrate some other mod "properly" with sfinv and while I was doing that I had a go at doing the same for edit_skin.

My changes are here:
Skaapdev@82458b1

That diff is quite hard to read, I am not sure I believe it. Better to just look at the raw file or do your own diff from that.

Unfortunately I am working from a fork of a fork, I suspect a PR would be inappropriate. Alas my understanding of both sfinv and edit_skins is suspect at the very least. However this is what I am going to use for the time being.

Thanks,
Skaap

@MrRar
Copy link
Owner

MrRar commented Mar 22, 2024

I honestly didn't know sfinv had a size parameter. I thought it was just fixed size. I will look into this.

@MrRar
Copy link
Owner

MrRar commented Mar 22, 2024

I tried it out: 0179946

I'm running into two issues: The hack that I used to get the color sliders to work doesn't work very well with sfinv. It's probably possible but not super simple. Other issue is that it's impossible to know when an sfinv formspec is closed so the player skin has to be saved after every little change. I guess that's not so bad.

@Skaapdev
Copy link
Author

@MrRar
Hi MrRar,

Thank you for looking at this.
I tried your branch, and it works well for me.

  1. the color sliders to work doesn't work very well with sfinv.

Could you be more specific about what part doesn't work? I tried both the sfinv and /skin form specs and could not find anything wrong with the color sliders.

Not related but, is it complicated to add the color sliders for the eyes?

  1. Other issue is that it's impossible to know when an sfinv formspec is closed so the player skin has to be saved after every little change. I guess that's not so bad.

What about just saving the skin on player logout?
If that is not possible I would suggest some kind of cool down wrapper queue to saving.

Something like this pseudo code below:

skin_saved_cooldown = {}
skin_last_cache = {} -- Perhaps this isn't even needed?
...
--Before use:
if skin_saved_cooldown[player_name] == true then
    skin_last_cache[player_name] = `the data about the current skin`
    return 
end
skin_saved_cooldown[player_name] = true
minetest.after(5, function()
    edit_skin.save(player,  skin_last_cache[player_name])
    skin_saved_cooldown[player_name] = nil
end)

Kind Regards,
Skaap

@MrRar
Copy link
Owner

MrRar commented Mar 30, 2024

Could you be more specific about what part doesn't work? I tried both the sfinv and /skin form specs and could not find anything wrong with the color sliders.

Maybe the fact that moving them does nothing? Anyway I added a new commit that got them working again. I had to do some hacky stuff to get them working. I also added some code in the latest commit that fixes the bug where if you update the player using /skin the changes don't show on the sfinv page. I feel like these changes are just too hacky to add such a minor feature so I probably wont add it to the master branch right now.

Not related but, is it complicated to add the color sliders for the eyes?

Not really. I made some things not colorable because I thought it wouldn't mater that much but some people care about it. Particularly the shoes and the eyes. I might add that some time.

@adikalon
Copy link

@MrRar what about implementation in unified_inventory?

@MrRar
Copy link
Owner

MrRar commented Jul 28, 2024

@MrRar what about implementation in unified_inventory?

I don't think it would really be possible. The area Unified Inventory provides for custom formspecs is way to small to fit Edit Skin. Plus there's already a button to get to Edit Skin from Unified Inventory.

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

No branches or pull requests

3 participants