benno1237 / MinePI

Minecraft utility library
MIT License
20 stars 8 forks source link

Can't render "Classic" Skins #2

Closed Metriximor closed 3 years ago

Metriximor commented 3 years ago

Hi, I was trying to render a classical skin, however I got this error:

Traceback (most recent call last): File "main.py", line 9, in <module> asyncio.run(main()) File "Python\Python39\lib\asyncio\runners.py", line 44, in run return loop.run_until_complete(main) File "Python\Python39\lib\asyncio\base_events.py", line 642, in run_until_complete return future.result() File "main.py", line 6, in main im = await MinePI.render_3d_skin("50e1ba76234c4496ad0f2b72a8701d6c") File "Python\Python39\lib\site-packages\MinePI\functions.py", line 23, in render_3d_skin im = await Render(user, vr, hr, hrh, vrll, vrrl, vrla, vrra, ratio, False, display_hair, display_second_layer, aa).get_render(skin_image) File "Python\Python39\lib\site-packages\MinePI\skin_render.py", line 110, in get_render self.generate_polygons(hd_ratio, skin) File "Python\Python39\lib\site-packages\MinePI\skin_render.py", line 552, in generate_polygons color = skin.getpixel(((40 * hd_ratio - 1) - i, 20 * hd_ratio + j + 16)) File "Python\Python39\lib\site-packages\PIL\Image.py", line 1369, in getpixel return self.im.getpixel(xy) IndexError: image index out of range

This error only happens with skins that use the "classic" model, "slim" skins work just fine. Example classic skin that crashes the library: Metriximor Example slim skin that works as expected: Vend3tta

I wonder if this is a limitation of the library? And if there's any way to make it also able to render Classic skins? Thank you

image

benno1237 commented 3 years ago

Your skin has an old style which i thought doesnt exist anymore/gets automatically converted by the MojangAPI. At least i never saw one like that. The error is quite obvious because the body parts in your raw image are not where they are supposed to be. Here a comparison:

test test2

Fixing that shouldnt be too hard tho. i only have to copy the parts over. Thanks for letting me know!

benno1237 commented 3 years ago

Not as quick as i thought. will let you know once it is fixed

Metriximor commented 3 years ago

Awesome, and yeah, Mojang still supports them afaik. Let me know if you need help with anything :D

Metriximor commented 3 years ago

If you refer to the mojang api you can actually see they have the "variant" field in the response, slim ones are the currently supported ones and "classic" ones are the ones like mine

benno1237 commented 3 years ago

Thats the point. There are two different kinds of classic skins. Both of the skins i linked above are actually classics. After 1.7 I think Mojang switched from the 32x64px style raw skin image to a 64x64px one because they needed the extra space for the overlays. Slim skins didnt exist back then so they are not affected. But apparently some classic skins still use the old format. I tested like 50 accounts and none of them had it. I am very sorry for that

Metriximor commented 3 years ago

Oh I see, damn that's a hard bug to track down. Gg my dude

benno1237 commented 3 years ago

Sorry for the slight delay. I just pushed an experimental fix to the (newly created) develop branch. If you want to test it, you can install it using pip install git+https://github.com/benno1237/MinePI.git@develop#egg=MinePI --force-reinstall

Short clarification in case you are interested. Till 1.7, Minecraft skins always had the same left and right leg/arm. Afterwards, they allowed the users to customize both individually tho. Thus they needed more room in the image and made it bigger. I resized the image of those old skins now, copied all parts over to their second location and changed the order to have them display correctly.

I will do some testing and PR it to the main branch. If you encounter any issues, let me now pls!

Metriximor commented 3 years ago

Thank you, I have a wide variety of minecraft accounts that I will need to generate renders for, I'll let you know if I find any issue. Nice work and thanks for the clarification :D

benno1237 commented 3 years ago

In case you didnt reinstall yet: using --upgrade instead of --force-reinstall would probably be better. Thanks again for having a look and testing the library

Metriximor commented 3 years ago

Ok so today I was trying further and checking out the overall model and I think I found some issues: As you can see the hands are on the shoulders, and the bottom of the feet doesn't seem to be correct either.

image

For comparison, this is what it should look like: image

image

benno1237 commented 3 years ago

Thanks for letting me know. Just pushed another version to the develop branch. Should be fixed. If you encounter any other issues, let me know please.

Metriximor commented 3 years ago

By the way just wanna let you know it worked perfectly, kinda busy right now with irl stuff but in a few days/weeks I'll post an update on how this library is being used on our server wiki!

Thank you so much for this library it's a life saver.

benno1237 commented 3 years ago

Looking forward to it! I will close this issue now. If you encounter any other bugs or want to suggest something, feel free to open a new issue or contact me on discord (Benno#8969)