Skip to content
This repository has been archived by the owner on Aug 9, 2023. It is now read-only.

3d_armor doesn't enchant properly #81

Open
aerozoic opened this issue Aug 14, 2017 · 13 comments
Open

3d_armor doesn't enchant properly #81

aerozoic opened this issue Aug 14, 2017 · 13 comments

Comments

@aerozoic
Copy link

I have a fresh install of 3d_armor and xdecor. I've not made any modifications to either mod. Every single piece of armor of every variety, when enchanted, shows a level of 1.8 on the HUD. I've also done a kill test with diamond armor. Wearing the full set, it took 21 clicks with a steel sword to kill while the enchanted diamond armor only took 10 clicks.
🔶

@kilbith
Copy link
Collaborator

kilbith commented Aug 14, 2017

@stujones11 Something that could have broke my mod in 3d_armor recently?

@ghost
Copy link

ghost commented Aug 15, 2017

cessna151 maybe it makes a difference if you're using clothing, multiskin, and 3d_armor multiskin branch?

@aerozoic
Copy link
Author

Yes i am using 3d_armor multiskin branch. If it helps this is the repo for my server: https://github.com/UGXaero/UGXrealms
🔶

@mgl512
Copy link

mgl512 commented Aug 30, 2017

Something that could have broke my mod in 3d_armor recently?

It's been broken for ages by 3d_armor's new api. We fixed it in our rather heavily updated fork of xdecor but I didn't bother to report. You may want to check this commit: MT-Eurythmia@cff1ba4. I don't know if it was broken even more after that. Looked ok last month.

I remember that the enchanment table requires armor textures names it doesn't provide and that it doesn't even use them if you provide them. It uses the ones from 3d_armor.

@kilbith
Copy link
Collaborator

kilbith commented Aug 30, 2017

So now it's clear we can't rely safely on 3d_armor's API from external mods.
Thanks for such maintenance, @stujones11 👎

Someone needs to import the patches from @mgl512. And please mind to backport fixes in upstream, it's very important.

@kilbith kilbith added the bug label Aug 30, 2017
@kilbith
Copy link
Collaborator

kilbith commented Oct 9, 2017

Problem radically solved: 85b023c

I don't give a crap about unstable APIs causing deal-breaking regressions on other mods.

@kilbith kilbith closed this as completed Oct 9, 2017
@stujones11
Copy link

stujones11 commented Oct 10, 2017

@kilbith Well that should not have been necessary, I am sure there was a much less radical solution had I known about this issue (sorry I missed your 'pings'). I did make substantial re-work to the mod a while back but did my best to maintain backwards compatibility with other mods, obviously I must have missed something though I am sure it would have been a simple enough fix.

Please consider reverting that commit and let me sort this out now that I am aware of the issue.

@aerozoic
Copy link
Author

@sofar Would you be willing to reopen this issue and work with @stujones11 to resolve it properly?

Thanks
🔶

@sfan5 sfan5 reopened this Nov 16, 2017
@stujones11
Copy link

stujones11 commented Nov 17, 2017

Unfortunately, it looks like it was broken by adding support for non-fleshy damage groups so there is no easy fix from my end. To be fair though, this has nothing to do with unstable api's as these groups were not previously documented and the method used for enchantment was far from ideal.

There is, however, an open feature request for supporting ItemStackMetaRef which should make things like this much easier stujones11/minetest-3d_armor#114 I will make a PR to re-add 3d_armor support once this is done.

In the meantime, @mgl512 has already provided a (lgtm) patch for this #81 (comment) This could have simply been cherry-picked by now, had the original support not been so 'radically' removed.

@sofar
Copy link
Member

sofar commented Nov 20, 2017

I'll take a patch that reverts 85b023c and adds the patch from @gml512 on top of it. Please make a PR.

@stujones11
Copy link

stujones11 commented Nov 21, 2017

@sofar thanks but if I am making a PR it will (hopefully) be a cleaner implementation when I add support for ItemStackMeta. However, I have no objection to anyone else doing this. Since this is now really just a request you can, at least, remove the 'bug' label :)

@sofar sofar added enhancement and removed bug labels Nov 25, 2017
@aerozoic
Copy link
Author

aerozoic commented Mar 31, 2018

Edit: Here's the simple fix.
🔶

@sofar
Copy link
Member

sofar commented Apr 12, 2018

Indentation needs to remain using tabs and not spaces. Please change the spaces to tabs again.

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

No branches or pull requests

6 participants