Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix incompatibility with lithium explosion exposure optimization #1937

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

2No2Name
Copy link
Contributor

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
Copy link
Collaborator

altrisi commented Jun 24, 2024

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
Copy link

manuelgrabowski commented Aug 11, 2024

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
Copy link

Can this be merged ASAP?

@tajemniktv
Copy link

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

Copy link

@supersaiyansubtlety supersaiyansubtlety left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ModifyArg would be less brittle than local capture

Apply code review by supersaiyansubtlety

Co-authored-by: Will <[email protected]>
@Ka1nnn
Copy link

Ka1nnn commented Aug 19, 2024

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
Copy link

Thank you @Ka1nnn 😄

@MeeniMc
Copy link

MeeniMc commented Aug 22, 2024

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
Copy link

MeeniMc commented Aug 24, 2024

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..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants