Enchantment Upgrade Reachitecture#701
Conversation
|
Overall this rework looks much better than the former system. To address your points:
|
|
While Datagen will be a nice thing in the future, I am not sure if holding our breath for it is exactly a great idea currently. The main dev for it (Electro) has gone quiet for a bit; given the way she tends to cycle through things it is unlikely we will see her again for some time. I would rather set them up manually and get this merged instead of waiting overlong. As for the recipe display thing. I will give look at switching recipe views on a timer. It would match the behavior of a preexisting feature that does something similar - the titty barrel. |
|
The future is now, as datagen is about ready for merge. Turns out it was almost finished, so you should save some headaches on not stumbling into comma errors with it 🙂 |
|
Draft time is joever, it is now ready-ish to merge. The internals are all functional, but the rest of the recipes do have to be readded. Evidently this will ultimately be done via datagen and as such is... outside the scope of this PR? |
|
Oh, oh that is a funny merge. Hm. I am not updating the datagen for this :3 |
DaFuqs
left a comment
There was a problem hiding this comment.
- Having one recipe.json per enchant is nice in a way that it's less likely to break
- The XP scaling is quite a bit harder to understand compared to the previous "always indexed" method. It certainly is more dynamic, so its more difficult to write recipes for without throughoutly studying the docs. Not a dealbreaker, but also a so/so change
- After trying it out not a fan of the current EMI/REI displays. They need some improvements, still
- the cycling makes it hard to see what exactly you need (especially since there is no way to pause them atm). Personally I'd favor one entry per level more
- they list the level, but not the enchant (which you can only see by hovering the enchanted book). We could add that info below the recipe to make it quick to see
- pressing "r" when hovering over enchanted books does not show the recipes anymore
- the
spectrum:enchanter_upgradingmodonomicon book pages are broken (see Clover's Favor entry for reference)
|
the spectrum:enchanter_upgrading modonomicon book pages are broken (see Clover's Favor entry for reference) WAIT THOSE EXIST |
|
yes, those exist |
|
Also but after looking at how things used to be a bit more I just have to completely disagree. 100 pages of recipes is entirely unnavigable |
Done in by modonomicon's shit logging 😔 |
|
Depends on how you use the recipe viewer. My usual user story would be: "I want to be able to see how I can get a sharpness 3 book" Pull request version: Current Version: Especially for people that are a bit slower, not having to catch the right timings might be a big deal. |
|
That is a fair view. I did mention I wanted to integrate a way to pause cycling. There is a small issue where uh, I am 99% sure only EMI has the internals to support that. REI has quickly proven to be a lot less flexible |
|
You know, we look at many things daily and wonder why they are written that cursed and it certainly could be improved. Sadly most things have a reason why they were written as cursed as they are. This is the "why" for the enchant upgrade recipes (pain) |
|
Hmph, that is true. At the same time though we are coming up with the question of maintainability vs ideals and... frankly maintainability wins out here imho. I can see the case for how things were, but I believe that with some work most of the issues at hand can be suitably addressed. May require I do something funny with the recipe viewers tho. |
|
Yeah, I am sure we can come up with a nice way to handle the recipe viewers with the new way the recipes are implemented. It certainly is a big improvement once that is done. |
|
Perhaps I should look into the button idea again... |
|
The UX with the button variant is really nice. I like it a lot! A few things are still broken (all tested with EMI):
using this json {
"type": "spectrum:enchantment_upgrade",
"bulk_item": {
"item": "spectrum:light_blue_pigment"
},
"enchantment": "spectrum:big_catch",
"item_scaling": {
"type": "spectrum:doubling",
"scaling_factor": 2.2,
"scaling_value": 4
},
"level_cap": 3,
"required_advancement": "spectrum:unlocks/enchantments/big_catch",
"xp_scaling": {
"type": "spectrum:doubling",
"scaling_value": 200
}
} |
|
Steve Jobs died of Ligma. |
Sex - I mean sync




Rewrites enchantment upgrade recipes to actually function in 1.21.1, and also just generally be less goofy? Some flexibility is sacrificed in favor of a much more robust system that shouldn't implode every major version bump - or you know, any time PissJS sneezes.
Comes with a side of RecipeScaling.java.exe.sex (free, download now no virus!!!)
This is, as of now, a Draft because there are some unanswered questions.