gnembon / fabric-carpet

Fabric Carpet
MIT License
1.73k stars 275 forks source link

fix incompatibility with lithium explosion exposure optimization #1937

Open 2No2Name opened 5 months ago

2No2Name commented 5 months ago

Lithium redirects the same call to replace the computation with a cached value lookup.

I don't see a way to avoid lithium's redirect, without getting rid of the optimization, which is why I am suggesting to change it in carpet.

This PR uses the @Local instead of CAPTURE_FAILHARD to get the local variable, because it is easier.

I don't know how to use scarpet, so I couldn't test it, but it at least didn't crash the game when I exploded a tnt in a flat world

altrisi commented 5 months ago

You can test events by loading the built-in event_test (/script load event_test), which should print the info received by them.

I'll try to take a look and merge later.

manuelgrabowski commented 3 months ago

Any chance this could get merged? The current latest versions of Lithium and Carpet are now crashing when used together.

(mixin.world.explosions.cache_exposure = false in lithium.properties is a workaround, in case anyone else ends up here.)

djmrFunnyMan commented 3 months ago

Can this be merged ASAP?

tajemniktv commented 3 months ago

This really needs to be merged on Carpet side. Both mods are essential and crucial for many players, and further optimization of explosions is always a good thing to see. If anything breaks with that, we will report immediately, don't worry lmao

Ka1nnn commented 3 months ago

Right so due to the fact that this will prob never get merged as gnembon just doesnt seem to have the time ig to fully maintain carpet mod [kinda sad but i get it ig...] here is a complied jar for anyone who needs it, for 1.21, 1.21.1

fabric-carpet-1.21-1.4.147+v240819.zip

sternschnaube commented 3 months ago

Thank you @Ka1nnn 😄

MeeniMc commented 3 months ago

You can use the compiled artifact for the PR instead of relying on builds from unknown sources https://github.com/gnembon/fabric-carpet/actions/runs/10369618488

MeeniMc commented 3 months ago

Event script reports events normally, (with and without the optimizeTNT carpet rule active). I ran a set of farms from scicraft Blitz world and they all worked as intended (with and without optimizeTNT active), blast chambers, world eater etc..

gnembon commented 2 weeks ago

Sorry, was busy.. explosion code has changed drastically. Is that still an issue?

2No2Name commented 2 weeks ago

I worked around this on lithium's side

djmrFunnyMan commented 2 weeks ago

But wouldn't it be better if carpet implements the fix so you don't need to do a workaround?