Small design question
Closed this issue · 4 comments
@H7KZ I do not know where to contact you for design decision, so I'm asking here.
What should happen if someone try to disenchant an item with an enchantment marked in "disabled-enchantments" (same with enchantment splitting)
I'm asking that because currently it seems like there is code to not continue the process if it detects one disabled enchantment.
Then code to remove disabled enchantment from the result enchantment list (can't happen because previous code do not allow disabled code)
Edit: also, it looks like item disenchantment would remove disabled enchantment on the left item (as it remove every enchantments) (if the design decision is to just not allow when item contain a disabled enchantment then it means nothing)
I will make changes according to how it should behave.
Well I dont really know how it should work :DDD I implemented it with the idea of having 3 states:
allow = disenchantment works normally
keep = this keeps the specific enchantment on the item, meaning you cannot put this on a book
cancel = as name suggests, it cancels the entire disenchatment event, for example for enchantments as curse of binding (you cannot disechant any enchantment at all)
If you have any idea to make it more user friendly or something, sure bring it up! I will try to review and look at all the code and changes before the 24th cause then I fly to greece for a week :DD I will be online but probably wont code as much :DDD
Currently, it looks like it only store in an enchantment|bool map (disabled-enchantments) with true being disabled and false/unset enabled.
Unlike the GUI that (try to) understand false as keep.
And disabled enchantment for book splitting look like has no settings in the GUI. Will also do it.
I try to do it within 24h, but may not be able to.
If I create a PR, do not review it while on your trip. It can wait a week and if someone request it I can merge myself and release it on GitHub.
false == keep
true == cancel
not in the array == enabled