Problems with dynamic value calculation
Dynious opened this issue ยท 4 comments
I'm updating an addon mod for EE3 that adds support for modded items & (machine)recipes (https://github.com/Dynious/EE3-Addons). I've encountered some issues when updating:
- The current OreStack system is broken. Currently only the first value in the OreDictionary id list is used when an ItemStack is used for OreStack creation. The thing that might seem like a good fix is rewriting the whole system to be able to use multiple ids, but this will break even more. Many mods now add non-equivalent items to an OreDictionary list.
OreStack value assignment should only work from id to itemstack and not the other way around. This will make sure only equivalent OreDict entries are assigned, so we know for sure that they are right. The OreStack constructor using an ItemStack is not used often and most uses can be easily rewritten, the only real problem are the Forge OreRecipes which creates a list of ItemStacks with the OreDict id in the recipe.
The best way to fix this would be to check all OreDict ids and see what id is present on all stacks. There is a (small) chance all items have multiple matching ids. This would be the only point where the system could fail. This can, of course, be resolved with some ASM injection on OreRecipes (but I'm not sure you want ASM in EE3).
I made a PR for the changes: #784 - Add a way to alter or change the way damage values influence the EnergyValue of an item (e.g. a dischanged item (e.g. IC2 battery) should not have an EnergyValue of 0, but should be calculated by the system (so IEnergyValueProvider isn't really what I want))
- Some mods add recipes in postInit. These will not all be picked up by dynEV as it registers recipes in postInit. Possible solution: postpone this to EnergyValueRegistry#runDynamicEnergyValueResolution. (Is also more logical, we don't need to get the recipes when we're not recalculating but loading from file.)
- OreStack is not in the api package (seems like a logical thing to have as a addon maker)
- An 'Always' EV recalculation setting would make things more easy to bugtest
EDIT: More stuff & made things more clear
EDIT2: Added PR.
I think most of this has been addressed now @Dynious, can you confirm?
Closing in order to track issues on the unified ticket #990