Skip to content

Catalysis updates the real brewing stand instead of a snapshot - #5335

Open
RasmusKD wants to merge 2 commits into
mcMMO-Dev:masterfrom
RasmusKD:fix/catalysis-brew-display
Open

Catalysis updates the real brewing stand instead of a snapshot#5335
RasmusKD wants to merge 2 commits into
mcMMO-Dev:masterfrom
RasmusKD:fix/catalysis-brew-display

Conversation

@RasmusKD

Copy link
Copy Markdown

AlchemyBrewTask keeps the BlockState it was created with and calls setters on it every tick, but snapshot setters never reach the world. The visible effects:

  • with Catalysis on vanilla ingredients, the client's brew arrow follows vanilla's own timer, so the potion finishes while the arrow is still mid-bar
  • custom-ingredient brews show no progress at all, the potion just pops after x seconds with no feedback
  • the initial fuel decrement for custom ingredients is lost
  • the brewing-time reset in cancelBrew() does nothing

Writes now go through a fresh state fetched and applied within the same tick, so the arrow tracks the accelerated brew and fuel/cancel actually happen. One method added, three call sites changed.

AlchemyBrewTask holds the BlockState snapshot it was created with and
calls setters on it every tick, but snapshot setters never reach the
world. The client's brew arrow follows vanilla's own timer, so with
Catalysis the potion finishes while the arrow is still mid-bar, custom
brews show no progress at all, the initial fuel decrement for custom
ingredients is lost, and the reset on cancel never happens.

Writes now go through a fresh state fetched and applied within the
same tick, so the arrow tracks the accelerated brew.

@nossr50 nossr50 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The fuel write in AlchemyBrewTask.initializeBrewing() causes a bug now that it actually works. It puts back fuel that vanilla already spent:

  1. When the ingredient goes in, the AlchemyBrewTask constructor reads the fuel level from its snapshot. At that moment the brew hasn't started, so it reads the full amount.
  2. A tick later, vanilla starts the brew in BrewingStandBlockEntity.serverTick and spends one fuel.
  3. On the task's first run, initializeBrewing() writes the old number back. The spent fuel comes back.

I tested this on a Paper 26.2 server with some debug logging around the write. The stand had 20 fuel, I brewed nether wart, and the log showed the stand at 19 right before the write and 20 right after. Same thing on every nether wart brew I did, and after several full brews the stand still had all 20 fuel. So with this PR, brewing vanilla ingredients on an owned stand costs no fuel.

This might fix it: delete the fuel write, plus the fuel capture and the == -1 check in the constructor that go with it. Vanilla already handles fuel for its own ingredients. The == -1 check doesn't do anything anymore anyway, an idle stand reports 0 these days, not -1. Custom ingredients not using fuel is a different, older problem. Open to other ideas too.

The constructor reads the fuel level before vanilla starts the brew
and spends one, so writing that number back on the first run refunds
the spent fuel. The write was dead while it targeted the snapshot, so
the stale read never mattered until the previous commit made it live.
Vanilla handles fuel for its own ingredients; the -1 check never fires
on current servers (an idle stand reports 0), so the capture and
decrement go too.
@RasmusKD

Copy link
Copy Markdown
Author

Good catch. The write was dead while it targeted the snapshot, so the stale read never mattered until this PR made it live. Removed the fuel write, the capture and the -1 check as suggested.

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.

2 participants