(1.20.1) Adds Capability for Mods to Register Custom Brewing Fuels #10123

Thomas1034 · 2024-10-10T02:17:22Z

It is currently impossible to make brewing stands accept alternate fuels in 1.20.1. This pull request fixes that problem. It adds a new class, BrewingFuelRegistry, which allows mods to associate an item with a brewing stand fuel value. Brewing fuels - like brewing recipes - can be added to the registry during the FMLCommonSetupEvent event.

I have also confirmed that this works with hoppers; they can correctly insert new fuels.

A test mod is included, verifying that both custom items and vanilla items can be added as fuels.

Please let me know if there are any problems I should address with this pull request.

Added a constructor that takes in a block entity type. This allows mods to create their own hanging sign block entities without needing to implement their own rendering methods for the GUI, use mixins, or attempt to hardcode the getter methods. See net.minecraft.client.player$openTextEdit(net.minecraft.world.level.block.entity.SignBlockEntity, boolean) for the reason this would be useful - to render the GUI for a hanging sign, the block entity needs to extend HangingSignBlockEntity.

Partially done.

…m/Thomas1034/MinecraftForge into 1.20.1-extensible-hanging-sign-be

1.20.1 - Import Changes

Also removed extra line in the event class.

Fixed accidental comment out

Why won't GitHub work right?

(1.20.1) Adds Capability for Mods to Register Custom Brewing

LexManos · 2024-10-10T23:10:25Z

src/main/java/net/minecraftforge/event/brewing/RegisterBrewingFuelEvent.java

+
+public class RegisterBrewingFuelEvent extends net.minecraftforge.eventbus.api.Event 
+{
+


Our new code style is to have braces on the same line, so please do that.

Understood. I was wondering if the "how to contribute to forge" document I found might be a little out of date... guess so. It's here if you want to take a look.

LexManos · 2024-10-10T23:11:56Z

patches/minecraft/net/minecraft/world/inventory/BrewingStandMenu.java.patch

+ import net.minecraft.world.item.alchemy.PotionUtils;
+
+ public class BrewingStandMenu extends AbstractContainerMenu {
+   private static final net.minecraftforge.common.BrewingFuelValues BREWING_FUELS = net.minecraftforge.common.ForgeHooks.onBrewingFuelRegisterEvent();


This fires the event during the static initializer of the brewing stand menu, this is not a good place for that. A better place would be to make it part of the registry events, Or perhaps instead of making it a custom registry implementation. It could be data driven and use the syncing mechanics of the custom data registries?

I'll look into those and see if I can find a better way. One thing, though: you mentioned on Discord that using tags would not be the best idea - wouldn't making it data driven have the same problems with vanilla server/Forge client?

Quite possibly, it would need testing and a way to verify that connecting to vanilla servers behaves fine.

I'm having trouble figuring out how to use the custom data registries. Are there any existing examples I could take a look at?

Since I've been so far unable to figure that part out, I've been working on a different approach. I reworked the brewing fuel values class to pattern it off of the BrewingRecipeRegistry class; thus, instead of having its own event, mods can add values to a static map in the FMLCommonSetupEvent. This version does not require adding an event, requires only minimal changes to other Forge classes, and matches the existing way of adding behaviors to the brewing stand.

LexManos · 2024-10-10T23:13:38Z

patches/minecraft/net/minecraft/world/level/block/entity/BrewingStandBlockEntity.java.patch

+ import net.minecraft.world.level.block.state.BlockState;
+
+ public class BrewingStandBlockEntity extends BaseContainerBlockEntity implements WorldlyContainer {
+   private static final net.minecraftforge.common.BrewingFuelValues BREWING_FUELS = net.minecraftforge.common.ForgeHooks.onBrewingFuelRegisterEvent();


This again has the static initializer issue. But also you're firing the event twice, once here and once in the menu. Better is to find a way to fire it once and use it in both places.

I resolved this by making BrewingFuelValues work in a static way, like how BrewingRecipeRegistry does. This should make things simpler for both players and maintenance.

LexManos · 2024-10-10T23:14:18Z

src/main/java/net/minecraftforge/common/BrewingFuelValues.java

+
+    public BrewingFuelValues() 
+    {
+        this.fuelTimes = new Object2IntOpenHashMap<>();


As this takes no values from the constructor it can be done in the field initializer.

LexManos · 2024-10-10T23:14:48Z

src/main/java/net/minecraftforge/common/BrewingFuelValues.java

+     */
+    public int get(Item fuel) 
+    {
+        return this.fuelTimes.getOrDefault(fuel, VANILLA_FUEL_TIME);


This would make any item not registered return the default. I dont think this is good api...

The idea was that items tagged as brewing stand fuel would be whatever the default is, and can be overriden via this method.

Yes, but that is not what this method does, thats what the add(Item) overload does.
Change this to getOrDefault(fule, 0) and get rid of hasFuel in favor of get(item) != 0

Understood, I'll fix that. Thank you for your input.

Also drastically simplified the way it worked, and removed the (now unnecessary) event and hook.

Updated to address concerns.

Update ConstructorThrowTest.java (Oops)

Thomas1034 · 2024-10-11T17:36:42Z

I've updated the pull request to address the concerns mentioned above.

In the previous commit, patches were not generated correctly; apparently, it took in excess of five minutes for Git to realize the patch files had changed after the task had finished, so the previous commit did not include them.

Fixed Patches

Jonathing

Please update the PR description to account for the changes made in recent commits.

Thomas1034 · 2024-11-01T21:56:15Z

I have updated the description as Jonathing requested.

Jonathing

I love this PR. Incredibly clear, efficient, and does what it needs to do well. I have a few nitpicks, but after that I think it's good for merge. I will handle backports for relevant versions.

Jonathing · 2024-11-01T22:05:01Z

patches/minecraft/net/minecraft/world/level/block/entity/BrewingStandBlockEntity.java.patch

+-      if (p_155289_.f_58979_ <= 0 && itemstack.m_150930_(Items.f_42593_)) {
+-         p_155289_.f_58979_ = 20;
+      if (p_155289_.f_58979_ <= 0 && (itemstack.m_150930_(Items.f_42593_) || net.minecraftforge.common.BrewingFuelRegistry.get(itemstack.m_41720_()) != 0)) {
+         p_155289_.f_58979_ = net.minecraftforge.common.BrewingFuelRegistry.get(itemstack.m_41720_());


You should move BrewingFuelRegistry.get() to its own variable. This will avoid calling it twice, along with the parameters which also include method calls.

I've fixed that. I'm not sure why I didn't have it do that originally.

Jonathing · 2024-11-01T22:11:13Z

src/main/java/net/minecraftforge/common/BrewingFuelRegistry.java

Please add a way to clarify in this file that you can only add to this once FMLCommonSetup has been fired.

I've updated the comments to indicate that it should only be manipulated during the FMLCommonSetupEvent.

Updated comments and moved get() into variable

Thomas1034 added 16 commits June 28, 2024 20:37

Update HangingSignBlockEntity.java.patch

eea9b0b

Update HangingSignBlockEntity.java.patch

a5aa7cb

Adding Alternate Brewing Fuels

2708ad4

Partially done.

Relocated

a7d33f4

Merge branch '1.20.1-extensible-hanging-sign-be' of https://github.co…

e4595af

…m/Thomas1034/MinecraftForge into 1.20.1-extensible-hanging-sign-be

Merge pull request #1 from MinecraftForge/1.20.1

8cccbe7

1.20.1 - Import Changes

Merge branch '1.20.1' into 1.20.1-brewing-fuels

11c221a

Fixed rearrangements to ForgeHooks

f702da1

Also removed extra line in the event class.

Add import to ForgeHooks

eac8192

Generated patches + added jsons

178af67

Update ConstructorThrowTest.java

264bb41

Fixed accidental comment out

Fixed bizarre glitch. I hope.

6a08727

Why won't GitHub work right?

Spacing

39a0477

Slight renaming, changed stack size of item

86bda71

Merge pull request #2 from Thomas1034/1.20.1-brewing-fuels

6a0ebfd

(1.20.1) Adds Capability for Mods to Register Custom Brewing

autoforge bot added Triage This request requires the active attention of the Triage Team. Requires labelling or reviews. 1.20 Feature This request implements a new feature. New Event This request implements a new event. labels Oct 10, 2024

autoforge bot requested a review from a team October 10, 2024 02:17

LexManos reviewed Oct 10, 2024

View reviewed changes

Thomas1034 added 4 commits October 11, 2024 13:33

Updated to address concerns.

80f5092

Also drastically simplified the way it worked, and removed the (now unnecessary) event and hook.

Merge pull request #3 from Thomas1034/1.20.1-brewing-fuels

4e5b6fa

Updated to address concerns.

Update ConstructorThrowTest.java

265b578

Merge pull request #4 from Thomas1034/1.20.1-brewing-fuels

24f2499

Update ConstructorThrowTest.java (Oops)

Jonathing requested a review from LexManos October 14, 2024 00:28

Thomas1034 added 2 commits November 1, 2024 09:03

Fixed Patches

7008194

In the previous commit, patches were not generated correctly; apparently, it took in excess of five minutes for Git to realize the patch files had changed after the task had finished, so the previous commit did not include them.

Merge pull request #5 from Thomas1034/1.20.1-brewing-fuels

b23507f

Fixed Patches

Jonathing suggested changes Nov 1, 2024

View reviewed changes

Thomas1034 added 2 commits November 1, 2024 19:54

Updated comments and moved get() into variable

d7774a9

Merge pull request #6 from Thomas1034/1.20.1-brewing-fuels

17a5c8e

Updated comments and moved get() into variable

Jonathing approved these changes Nov 2, 2024

View reviewed changes

Jonathing added Assigned This request has been assigned to a core developer. Will not go stale. and removed Triage This request requires the active attention of the Triage Team. Requires labelling or reviews. labels Nov 2, 2024

Jonathing assigned LexManos Nov 2, 2024

Jonathing self-requested a review November 6, 2024 20:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

(1.20.1) Adds Capability for Mods to Register Custom Brewing Fuels #10123

(1.20.1) Adds Capability for Mods to Register Custom Brewing Fuels #10123

Thomas1034 commented Oct 10, 2024 •

edited

Loading

LexManos Oct 10, 2024

Thomas1034 Oct 11, 2024

LexManos Oct 10, 2024

Thomas1034 Oct 11, 2024

LexManos Oct 11, 2024

Thomas1034 Oct 11, 2024

LexManos Oct 10, 2024

Thomas1034 Oct 12, 2024

LexManos Oct 10, 2024

LexManos Oct 10, 2024

RealMangorage Oct 10, 2024

LexManos Oct 10, 2024

Thomas1034 Oct 11, 2024

Thomas1034 commented Oct 11, 2024

Jonathing left a comment

Thomas1034 commented Nov 1, 2024

Jonathing left a comment

Jonathing Nov 1, 2024

Thomas1034 Nov 1, 2024

Jonathing Nov 1, 2024

Thomas1034 Nov 1, 2024


		public class RegisterBrewingFuelEvent extends net.minecraftforge.eventbus.api.Event
		{

(1.20.1) Adds Capability for Mods to Register Custom Brewing Fuels #10123

Are you sure you want to change the base?

(1.20.1) Adds Capability for Mods to Register Custom Brewing Fuels #10123

Conversation

Thomas1034 commented Oct 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Thomas1034 commented Oct 11, 2024

Jonathing left a comment

Choose a reason for hiding this comment

Thomas1034 commented Nov 1, 2024

Jonathing left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Thomas1034 commented Oct 10, 2024 •

edited

Loading