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

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

Open
wants to merge 24 commits into
base: 1.20.1
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
04c7827
Create HangingSignBlockEntity.java.patch
Thomas1034 Jun 29, 2024
eea9b0b
Update HangingSignBlockEntity.java.patch
Thomas1034 Jun 29, 2024
a5aa7cb
Update HangingSignBlockEntity.java.patch
Thomas1034 Jul 4, 2024
2708ad4
Adding Alternate Brewing Fuels
Thomas1034 Oct 9, 2024
a7d33f4
Relocated
Thomas1034 Oct 9, 2024
e4595af
Merge branch '1.20.1-extensible-hanging-sign-be' of https://github.co…
Thomas1034 Oct 9, 2024
8cccbe7
Merge pull request #1 from MinecraftForge/1.20.1
Thomas1034 Oct 9, 2024
11c221a
Merge branch '1.20.1' into 1.20.1-brewing-fuels
Thomas1034 Oct 9, 2024
f702da1
Fixed rearrangements to ForgeHooks
Thomas1034 Oct 9, 2024
eac8192
Add import to ForgeHooks
Thomas1034 Oct 9, 2024
178af67
Generated patches + added jsons
Thomas1034 Oct 9, 2024
264bb41
Update ConstructorThrowTest.java
Thomas1034 Oct 9, 2024
6a08727
Fixed bizarre glitch. I hope.
Thomas1034 Oct 10, 2024
39a0477
Spacing
Thomas1034 Oct 10, 2024
86bda71
Slight renaming, changed stack size of item
Thomas1034 Oct 10, 2024
6a0ebfd
Merge pull request #2 from Thomas1034/1.20.1-brewing-fuels
Thomas1034 Oct 10, 2024
80f5092
Updated to address concerns.
Thomas1034 Oct 11, 2024
4e5b6fa
Merge pull request #3 from Thomas1034/1.20.1-brewing-fuels
Thomas1034 Oct 11, 2024
265b578
Update ConstructorThrowTest.java
Thomas1034 Oct 11, 2024
24f2499
Merge pull request #4 from Thomas1034/1.20.1-brewing-fuels
Thomas1034 Oct 11, 2024
7008194
Fixed Patches
Thomas1034 Nov 1, 2024
b23507f
Merge pull request #5 from Thomas1034/1.20.1-brewing-fuels
Thomas1034 Nov 1, 2024
d7774a9
Updated comments and moved get() into variable
Thomas1034 Nov 1, 2024
17a5c8e
Merge pull request #6 from Thomas1034/1.20.1-brewing-fuels
Thomas1034 Nov 1, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
--- a/net/minecraft/world/inventory/BrewingStandMenu.java
+++ b/net/minecraft/world/inventory/BrewingStandMenu.java
@@ -13,6 +_,7 @@
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();
Copy link
Member

Choose a reason for hiding this comment

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

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?

Copy link
Author

Choose a reason for hiding this comment

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

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?

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Author

Choose a reason for hiding this comment

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

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.

private static final int f_150488_ = 0;
private static final int f_150489_ = 2;
private static final int f_150490_ = 3;
@@ -75,7 +_,7 @@
if (!this.m_38903_(itemstack1, 3, 4, false)) {
return ItemStack.f_41583_;
Expand All @@ -9,6 +17,15 @@
if (!this.m_38903_(itemstack1, 0, 3, false)) {
return ItemStack.f_41583_;
}
@@ -132,7 +_,7 @@
}

public static boolean m_39112_(ItemStack p_39113_) {
- return p_39113_.m_150930_(Items.f_42593_);
+ return p_39113_.m_150930_(Items.f_42593_) || BREWING_FUELS.hasFuel(p_39113_.m_41720_());
}

public int m_6641_() {
@@ -146,7 +_,7 @@
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,24 @@
--- a/net/minecraft/world/level/block/entity/BrewingStandBlockEntity.java
+++ b/net/minecraft/world/level/block/entity/BrewingStandBlockEntity.java
@@ -25,6 +_,7 @@
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();
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Author

Choose a reason for hiding this comment

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

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.

private static final int f_155280_ = 3;
private static final int f_155281_ = 4;
private static final int[] f_58972_ = new int[]{3};
@@ -91,8 +_,8 @@

public static void m_155285_(Level p_155286_, BlockPos p_155287_, BlockState p_155288_, BrewingStandBlockEntity p_155289_) {
ItemStack itemstack = p_155289_.f_58975_.get(4);
- 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_) || BREWING_FUELS.hasFuel(itemstack.m_41720_()))) {
+ p_155289_.f_58979_ = BREWING_FUELS.get(itemstack.m_41720_());
itemstack.m_41774_(1);
m_155232_(p_155286_, p_155287_, p_155288_);
}
@@ -148,6 +_,7 @@

private static boolean m_155294_(NonNullList<ItemStack> p_155295_) {
Expand Down Expand Up @@ -44,7 +63,8 @@
- return PotionBrewing.m_43506_(p_59018_);
+ return net.minecraftforge.common.brewing.BrewingRecipeRegistry.isValidIngredient(p_59018_);
} else if (p_59017_ == 4) {
return p_59018_.m_150930_(Items.f_42593_);
- return p_59018_.m_150930_(Items.f_42593_);
+ return p_59018_.m_150930_(Items.f_42593_) || BREWING_FUELS.hasFuel(p_59018_.m_41720_());
} else {
- return (p_59018_.m_150930_(Items.f_42589_) || p_59018_.m_150930_(Items.f_42736_) || p_59018_.m_150930_(Items.f_42739_) || p_59018_.m_150930_(Items.f_42590_)) && this.m_8020_(p_59017_).m_41619_();
+ return net.minecraftforge.common.brewing.BrewingRecipeRegistry.isValidInput(p_59018_) && this.m_8020_(p_59017_).m_41619_();
Expand Down
61 changes: 61 additions & 0 deletions src/main/java/net/minecraftforge/common/BrewingFuelValues.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
package net.minecraftforge.common;

import it.unimi.dsi.fastutil.objects.Object2IntMap;
import it.unimi.dsi.fastutil.objects.Object2IntOpenHashMap;
import net.minecraft.world.item.Item;

/**
* Stores a map of items to integers representing the fuel value of the items
* when used in a brewing stand. If a fuel is not assigned a value, it will use
* the fuel value of blaze power. Once added, items cannot be removed from the
* fuel values table.
*/
public class BrewingFuelValues {

/**
* The value of the only vanilla fuel, blaze powder.
*/
public static final int VANILLA_FUEL_TIME = 20;
private Object2IntMap<Item> fuelTimes;

public BrewingFuelValues()
{
this.fuelTimes = new Object2IntOpenHashMap<>();
Copy link
Member

Choose a reason for hiding this comment

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

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

}

/**
* Adds the given item as a fuel for blaze rods, and assigns it the given fuel
* value.
*/
public void add(Item fuel, int fuelTime)
{
this.fuelTimes.put(fuel, fuelTime);
}

/**
* Adds the given item as a fuel for brewing stands, and assigns the default
* fuel value of blaze powder.
*/
public void add(Item fuel)
{
this.add(fuel, VANILLA_FUEL_TIME);
}

/**
* Retrieves the brewing stand fuel value of the given item. If the item is not
* in the map, this function returns the fuel value of blaze powder.
*/
public int get(Item fuel)
{
return this.fuelTimes.getOrDefault(fuel, VANILLA_FUEL_TIME);
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Author

Choose a reason for hiding this comment

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

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

}

/**
* Returns true if and only if this object's map has the given item as a fuel.
* Note that this map will not hold blaze powder by default.
*/
public boolean hasFuel(Item fuel)
{
return this.fuelTimes.containsKey(fuel);
}
}
8 changes: 8 additions & 0 deletions src/main/java/net/minecraftforge/common/ForgeHooks.java
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@
import net.minecraftforge.event.ServerChatEvent;
import net.minecraftforge.event.RegisterStructureConversionsEvent;
import net.minecraftforge.event.VanillaGameEvent;
import net.minecraftforge.event.brewing.RegisterBrewingFuelEvent;
import net.minecraftforge.event.entity.EntityAttributeCreationEvent;
import net.minecraftforge.event.entity.EntityAttributeModificationEvent;
import net.minecraftforge.event.entity.EntityEvent;
Expand Down Expand Up @@ -1635,4 +1636,11 @@ public static void onCreativeModeTabBuildContents(CreativeModeTab tab, ResourceK
for (var entry : entries)
output.accept(entry.getKey(), entry.getValue());
}

public static BrewingFuelValues onBrewingFuelRegisterEvent()
{
RegisterBrewingFuelEvent event = new RegisterBrewingFuelEvent();
MinecraftForge.EVENT_BUS.post(event);
return event.getBrewingFuelValues();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package net.minecraftforge.event.brewing;

import net.minecraft.world.item.Item;
import net.minecraftforge.common.BrewingFuelValues;

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

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Author

Choose a reason for hiding this comment

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

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.

private BrewingFuelValues brewingFuelValues;

public RegisterBrewingFuelEvent()
{
super();
this.brewingFuelValues = new BrewingFuelValues();
}

public void register(Item fuel, int fuelValue)
{
this.brewingFuelValues.add(fuel, fuelValue);
}

/**
* Is this unsafe? Should we allow outside access to the values?
* This could result in unexpected changes, which might cause problems.
*/
public BrewingFuelValues getBrewingFuelValues() {
return this.brewingFuelValues;
}

}
47 changes: 47 additions & 0 deletions src/test/java/net/minecraftforge/debug/BrewingFuelTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package net.minecraftforge.debug;

import net.minecraft.world.item.Item;
import net.minecraft.world.item.Items;
import net.minecraft.world.item.Rarity;
import net.minecraftforge.event.brewing.RegisterBrewingFuelEvent;
import net.minecraftforge.eventbus.api.IEventBus;
import net.minecraftforge.eventbus.api.SubscribeEvent;
import net.minecraftforge.fml.common.Mod;
import net.minecraftforge.fml.javafmlmod.FMLJavaModLoadingContext;
import net.minecraftforge.registries.DeferredRegister;
import net.minecraftforge.registries.ForgeRegistries;
import net.minecraftforge.registries.RegistryObject;

@Mod(BrewingFuelTest.MOD_ID)
@Mod.EventBusSubscriber(modid=BrewingFuelTest.MOD_ID)
public class BrewingFuelTest
{
static final String MOD_ID = "brewing_fuel_test";

private static final DeferredRegister<Item> ITEMS = DeferredRegister.create(ForgeRegistries.ITEMS, MOD_ID);

private static final RegistryObject<Item> TEST_BREWING_FUEL = ITEMS.register("test_brewing_fuel",
() -> new Item(new Item.Properties().rarity(Rarity.UNCOMMON)));

private static final boolean ENABLED = true;

public BrewingFuelTest()
{
if (ENABLED) {
final IEventBus modEventBus = FMLJavaModLoadingContext.get().getModEventBus();
ITEMS.register(modEventBus);
}
}

@SubscribeEvent
public static void addBrewingFuel(RegisterBrewingFuelEvent event)
{
if (ENABLED)
{
// Register vanilla item as fuel.
event.register(Items.COAL, 5);
// Register custom item as fuel.
event.register(TEST_BREWING_FUEL.get(), 10);
}
}
}
3 changes: 3 additions & 0 deletions src/test/resources/META-INF/mods.toml
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ modId="mdk_datagen"
[[mods]]
modId="calculate_normals_test"

[[mods]]
modId="brewing_fuel_test"

#[[mods]]
# modId="constructor_throw_test"

Expand Down
3 changes: 3 additions & 0 deletions src/test/resources/assets/brewing_fuel_test/lang/en_us.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"item.brewing_fuel_test.test_brewing_fuel": "Brewing Fuel"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"parent": "minecraft:item/generated",
"textures": {
"layer0": "brewing_fuel_test:item/test_brewing_fuel"
}
}
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading