Forgery

Forgery

823k Downloads

Furnace Cart resupplying works incompatible with lithium

skymen opened this issue ยท 14 comments

commented

Hi, thanks for fixing my issue yesterday!

I'm trying out the new release, but I can't figure out how furnace cart resupplying is supposed to work. I have a hopper pointed at a furnace cart with fuel inside of it, and the cart doesn't get filled up.

Reading the code, I don't understand what the trigger for refueling is.

I am on latest released version, and I have the new feature enabled in the config.

commented

I suggest to change your implementation to actually make your custom furnace minecart a vehicle inventory entity (similar to chest boat) that immediately acts when it receives an item, similarly to a composter that is getting filled up. Vanilla furnaces and similar blocks also already implement the possibility of disallowing item transfers under certain conditions.
Alternatively, fabric api also allows creating inventories at certain locations, but I am not sure if this is suitable for entities

commented

That would be the best solution.
it just requires fabrication to implement a vanilla interface on a vanilla class.
which gets really messy as soon as another mod has that idea, and since fabrication has houndrets of features i don't think it can affort the risk of using those kinds of injections.

"immediately acts when it receives an item", makes some mods have dupe bugs, sadly an inventory has to be reversable at least in the same tick. (but that's not really relavent)

Thanks for the advice, i immagine you deal with a fair bit of this.

commented

mixin.world.block_entity_ticking.sleeping.hopper=false Isn't the only conflict

commented
out.mp4

i have no idea maybe a mod conflict? (also this made me notice the feature crashes on forge, which i may have forgotten to test)

commented

also ye sorry the code is jank.
i thought of two ways to implement this.

a.) make the furnace minecart an actuall inventory
this would be the best since it would ensure copatability for basicly any mod which does inventory transfer as well.
however it means i gotta add new methods to the FurnaceMinecart class which means implementing a vanilla interface.

Fabrication currently has no way to do this is a safe manner and tbh probably? wont get one.

b) make the hopper check for furnace carts and give it a fake entity with an inventory to act as an intermediary.

i pulled a bit of a hack to make it not have to check for entities twice, which is better for performance.
i think i have an idea to make this less hacky.

commented

try 3.4.17

commented

Ok, I found the culprit. The mod isn't compatible with lithium's default config. Specifically the rule mixin.block.hopper that seems to be doing a bunch of underlying changes to the hopper mechanic
https://github.com/CaffeineMC/lithium-fabric/blob/44f4330b9ab1cef568e80336d949c61386f747b9/src/main/resources/lithium.mixins.json#L51

I'm not too well versed with how Minecraft works internally, but reading over the general purpose of each rule, this mixin might be the culprit?
https://github.com/CaffeineMC/lithium-fabric/blob/44f4330b9ab1cef568e80336d949c61386f747b9/src/main/java/me/jellysquid/mods/lithium/mixin/block/hopper/AbstractMinecartEntityMixin.java#L20

I'm guessing it's trying to optimise the minecart-hopper interaction by preventing minecarts with no inventory attached from triggering events that it deems unnecessary.

In case you can't have it fixed in fabrication, I can try reporting it as a bug to lithium so they can take it into account, or at least provide a config rule that disables that specific mixin instead of every hopper optimisations at once. Worst case scenario, I just add one line of config to disable the whole thing and I should be good to go ๐Ÿ‘

Still, since this rule is enabled by default with lithium, and it's installed in many modpacks it might cause confusion in the future.

commented

actually no wait i think 376 was fixed, so this should be do-able

commented

I'll add it to the feature description. there's not much else that can be resonably done about this.
(reminds me of 376, which should also have it added to desc)

commented

Thanks for debuging it.
I got half way through making a compatability fix but it's waay too hacky and requires messing with lithium in ways which break from one version to the next.
Really wish i could just implement inventory...

Try asking the lithium devs, they seem to have added handling for the wierd wool hopper mechanic so maybe thei've got an approach for this. (if they think compat code is required on both sides message me)

Here's the relavent code:
https://github.com/FalsehoodMC/Fabrication/blob/3.0/1.18/src/main/java/com/unascribed/fabrication/mixin/d_minor_mechanics/furnace_minecart_resupplying/MixinHopperBlockEntity.java#L31

(FabInject is just mixins Inject but with a few extra checks)

commented

Ok, I found the culprit. The mod isn't compatible with lithium's default config. Specifically the rule mixin.block.hopper ...

... this mixin might be the culprit? https://github.com/CaffeineMC/lithium-fabric/blob/44f4330b9ab1cef568e80336d949c61386f747b9/src/main/java/me/jellysquid/mods/lithium/mixin/block/hopper/AbstractMinecartEntityMixin.java#L20

Lithium developer here. The mixin only influences lithium's internal movement listening system, which is supposed to tell hoppers to check for inventory minecarts after a nearby inventory minecart moved.

Lithium has another feature that makes block entities (including hoppers) sleep under certain conditions, with certain events waking them up again (e.g. an inventory entity moving nearby). This might be incompatible with your additional hopper feature, because the sleeping and waking conditions are not implemented. I suggest testing with the line mixin.world.block_entity_ticking.sleeping.hopper=false added to the lithium.properties file in the config folder in the .minecraft folder. If that fixes the issue:

Here is an instruction how you can disable that lithium setting automatically when your mod is installed:
https://github.com/CaffeineMC/lithium-fabric/wiki/Disabling-Lithium's-Mixins-using-your-mod's-fabric-mod.json

For example: fabric-mod.json :

"custom": {
  "lithium:options": {
    "mixin.world.block_entity_ticking.sleeping.hopper": false
  }
}```
commented

@2No2Name (mentioning in case gh didn't follow the issue)

i probably should have repeated what was mentioned in 376

Fabrications features can generally be toggled while in game. so anything todo with fabric-mod.json is not an option.

the way 376 ended up being resolved is this
https://github.com/FalsehoodMC/Fabrication/blob/3.0/1.18/src/main/java/com/unascribed/fabrication/support/MixinConfigPlugin.java#L517
(inserting a FabConf.isEnabled check into lithiums mixin)
this has worked suprisingly well, considering that was is 2021 and i haven't heard a peep about it since.

i tried doing something simmilar in this case, that being mostly just going arount and changing "instanceof Inventory" to "instance of Inventory | instanceof FabricationFurnaceMinecart" inside lithium.
this mostly works(with additional tweaks), unlike with 376 it also maintains both mods changes, however it is far more unsafe and lithium has refactored this code across versions meaning it'd be an absolute pain to maintain.

ideally we could get something that works without loosing the optimization and i think it's possible, although it probably requres changes in both mods?

otherwise i'll just have to add a discraimer to the feature description to go manually disable the lithium optimization.

commented

"immediately acts when it receives an item", makes some mods have dupe bugs, sadly an inventory has to be reversable at least in the same tick. (but that's not really relavent)

Of course you could also implement the updating during the furnace minecart tick, checking whether any items were inserted until then

commented

yup, just pointing it out in case you have any code which needs to be reconsidered. (it's not the important part, just a side note)