De-hardcode shears (again) #3477

AnAwesomGuy · 2023-12-21T10:24:14Z

Adds a boolean method to FabricItem and FabricItemStack, isShears. It functions as a stack-aware check that allows items to function as shears. By default, the method returns true if the item is either an instance of ShearsItem or in the fabric:shears tag.

If a stack isShears, it is able to shear pumpkins, de-arm tripwire, be enchanted with efficiency, have ShearsDispenserBehavior (if there isn't already a DispenserBehavior for the item registered), and shear snow golems, sheep, and mooshrooms.

This also mixes into MatchToolLootCondition and adds a custom predicate to it if its predicate contains shears. The custom predicate is a boolean value registered under fabric:allow_shears and defaults to true. If the value is false, the mixin will not add the custom predicate even if the predicate contains shears.

Supersedes/closes #1287 and closes #1226. Also depends on/includes #3493, so either merge that first or just merge this PR.

ZestyBlaze · 2023-12-21T12:04:23Z

Aren't tags saved under c:shears not fabric:shears? You might want to ammend the comments to correct that if that's the case

deirn · 2023-12-21T14:38:19Z

Apply the license header with gradlew spotlessApply. You also need to fix the style issues.

Juuxel · 2023-12-21T19:12:50Z

@ZestyBlaze Generally, fabric tags are used for behaviour and c tags just for categorisation. (Compare with FAPI's mining level + mineable tags etc)

AnAwesomGuy · 2023-12-22T00:35:30Z

this works from 23w32a onwards (MatchToolLootCondition became a record with an optional in that version)

for below that, you would remove DirectRegistryEntryListAccessor<T> and replace MatchToolLootConditionMixin with something like this. that will work down until somewhere between 1.18.1 and 1.18.2, when TagKey<T>s were added

fabric-item-api-v1/src/main/java/net/fabricmc/fabric/impl/item/ShearsHelper.java

...-api-v1/src/main/java/net/fabricmc/fabric/mixin/item/shears/MatchToolLootConditionMixin.java

...tem-api-v1/src/main/java/net/fabricmc/fabric/mixin/item/shears/BlockInteractShearsMixin.java

fabric-item-api-v1/src/main/java/net/fabricmc/fabric/mixin/item/shears/DispenserBlockMixin.java

...m-api-v1/src/main/java/net/fabricmc/fabric/mixin/item/shears/EfficiencyEnchantmentMixin.java

...em-api-v1/src/main/java/net/fabricmc/fabric/mixin/item/shears/EntityInteractShearsMixin.java

...-api-v1/src/main/java/net/fabricmc/fabric/mixin/item/shears/MatchToolLootConditionMixin.java

fabric-item-api-v1/src/main/java/net/fabricmc/fabric/mixin/item/shears/TripwireBlockMixin.java

…lizer also change the way all the shears are found also change the way the mixins work

fabric-item-api-v1/src/main/java/net/fabricmc/fabric/impl/item/ShearsHelper.java

fabric-item-api-v1/src/testmod/java/net/fabricmc/fabric/test/item/ShearsTest.java

fabric-item-api-v1/src/main/java/net/fabricmc/fabric/impl/item/ShearsHelper.java

...-api-v1/src/main/java/net/fabricmc/fabric/mixin/item/shears/MatchToolLootConditionMixin.java

fabric-item-api-v1/src/main/java/net/fabricmc/fabric/mixin/item/shears/DispenserBlockMixin.java

deirn

Not sure about modifying all loot tables, we should probably only do it for vanilla.

fabric-item-api-v1/src/main/java/net/fabricmc/fabric/impl/item/ShearsHelper.java

AnAwesomGuy · 2023-12-29T10:23:11Z

Not sure about modifying all loot tables, we should probably only do it for vanilla.

If someone could help me find a way to do that that would be amazing. Though it would be even harder now that we're moving towards making it stack-based, which means I'm mixing into MatchToolLootCondition#test, making it so there is basically no context about its creation.

I also don't really see why anyone would want to target only vanilla shears but ¯⁠\⁠_⁠(⁠ツ⁠)⁠_⁠/¯.

deirn · 2023-12-29T10:49:06Z

The way Forge does it is to just replace the loot tables with their own. Maybe we can move the modification into a datagen instead.

AnAwesomGuy · 2023-12-31T06:09:11Z

Then it wouldn't be stack-based, and that seems like what modmuss and some others want.

deirn · 2023-12-31T06:37:53Z

You can make a custom predicate that checks if the item is shears with #3493 😉

AnAwesomGuy · 2024-01-01T02:39:07Z

That looks pretty good, but how would I make my PR depend on yours?
Should I just wait for it to get merged?

use custom codec impl

…v1/CustomItemPredicateRegistry.java Co-authored-by: apple502j <33279053+apple502j@users.noreply.github.com>

# Conflicts: # fabric-item-api-v1/src/main/java/net/fabricmc/fabric/api/item/v1/FabricItemStack.java # fabric-item-api-v1/src/main/resources/fabric-item-api-v1.mixins.json # fabric-item-api-v1/src/testmod/resources/fabric.mod.json

fabric-item-api-v1/src/main/java/net/fabricmc/fabric/impl/item/ShearsItemPredicate.java

deirn · 2024-02-09T16:10:28Z

fabric-item-api-v1/src/main/java/net/fabricmc/fabric/impl/item/ShearsItemPredicate.java

+	/**
+	 * Allows Fabric API to replace any occurrences of {@link net.minecraft.item.Items#SHEARS minecraft:shears} with a custom predicate for stack-aware shears. This is the default if {@link #DENY} is not specified.
+	 *
+	 * @see net.fabricmc.fabric.api.item.v1.FabricItem#isShears
+	 */
+	ALLOW,
+	/**
+	 * Disallows Fabric API to replace any occurrences of {@link net.minecraft.item.Items#SHEARS minecraft:shears} with a custom predicate for stack-aware shears.
+	 *
+	 * @see net.fabricmc.fabric.api.item.v1.FabricItem#isShears
+	 */
+	DENY;


I'm a little confused by this, what's with the allow and deny? Why can't it be a boolean that checks if it is shears or not?

This also mixes into MatchToolLootCondition and adds a custom predicate to it if its predicate contains shears. The custom predicate is a boolean value registered under fabric:allow_shears and defaults to true. If the value is false, the mixin will not add the custom predicate even if the predicate contains shears.

true is ALLOW and false is DENY
this is made so mods can also target only shears using DENY if they want to for some reason

What I had in mind is a boolean predicate, maybe called fabric:is_shears and value && item.isShears() check that entirely replaces the vanilla items predicate. People who want to match strictly vanilla shears can do so like before without the deny shenanigans.

deirn · 2024-02-09T16:10:57Z

...-api-v1/src/main/java/net/fabricmc/fabric/mixin/item/shears/MatchToolLootConditionMixin.java

+		if (predicate.items().isPresent() && predicate.items().get().contains(Items.SHEARS.getRegistryEntry())) {
+			CustomItemPredicate[] custom = predicate.custom().toArray(new CustomItemPredicate[0]);
+			ImmutableList.Builder<CustomItemPredicate> builder = ImmutableList.builderWithExpectedSize(custom.length + 1);
+
+			for (CustomItemPredicate customPredicate : custom) {
+				if (customPredicate == ShearsItemPredicate.DENY) return;
+				builder.add(custom);
+			}
+
+			((ItemPredicateExtensions) (Object) predicate).fabric_setCustom(builder.add(ShearsItemPredicate.ALLOW).build());
+		}


Does this make it replace all predicates with the ShearsItemPredicate.ALLOW by default? If anything, it should be defaults to deny.

yes it does
i chose to make it default to ALLOW cuz im lazy and dont want to datagen
also i doubt anyone should want to make it only work for shears so its opt-out

Can't you change the conditions at runtime with LootTableEvents.MODIFY? CC: @Juuxel

cech12 · 2024-04-17T06:00:35Z

Thanks for all your work! I think it would be great if the changes were included as I own 2 mods that add shears to Minecraft. They would benefit greatly from this. Are there any plans to bring this PR to 1.20.5? What is needed to reach that? :)

AnAwesomGuy · 2024-04-17T06:51:40Z

it should already work on 1.20.5, but this pr is kinda in a dead place rn

cech12 · 2024-04-25T06:25:05Z

I think it needs a rework after #3493 was closed, because of the vanilla's sub predicate system.

cech12 · 2024-06-07T14:17:10Z

For porting this to 1.20.6 I think it would be easier to add a new boolean component "IS_SHEARS" or something similar and add it to vanilla shears and also to custom shears. This component could be checked in all your mixins instead of adding a new tag, a new isShears method and the whole custom predicate logic. That should be much cleaner and easier to use.

Edit: and don't forget the newly added functionalities: shearable Bogged entity & Wolf armor shearing

Patbox · 2024-06-07T15:51:28Z

Adding new component would make fabric api incompatible with vanilla

AnAwesomGuy · 2024-06-07T23:57:32Z

yes, that is why i havent ported this to 1.20.5+ yet
i could also go the old route and just add stuff to FabricItem (including the custom predicate) but i dont really know if thats the way to go yet

SHEARS!

2a147b9

modmuss50 added the enhancement New feature or request label Dec 21, 2023

shear away the accesswidener

f9eae7b

code cleanup

a01db88

deirn reviewed Dec 22, 2023

View reviewed changes

fabric-item-api-v1/src/main/java/net/fabricmc/fabric/impl/item/ShearsHelper.java Outdated Show resolved Hide resolved

...-api-v1/src/main/java/net/fabricmc/fabric/mixin/item/shears/MatchToolLootConditionMixin.java Outdated Show resolved Hide resolved

LlamaLad7 suggested changes Dec 22, 2023

View reviewed changes

move static initializer in MatchToolLootConditionMixin to a ModInitia…

63da6b4

…lizer also change the way all the shears are found also change the way the mixins work

AnAwesomGuy requested review from modmuss50, deirn, Syst3ms and LlamaLad7 December 24, 2023 05:49

small change

c3ee57f

modmuss50 requested changes Dec 27, 2023

View reviewed changes

AnAwesomGuy added 2 commits December 28, 2023 13:51

write comments and stuff like that (code cleanup x2)

955563e

fix a small problem with the dispenser mixin

b3e7cf3

deirn reviewed Dec 28, 2023

View reviewed changes

fabric-item-api-v1/src/main/java/net/fabricmc/fabric/impl/item/ShearsHelper.java Outdated Show resolved Hide resolved

stack-awareness (pt. 1 (loot doesnt work yet))

f28040a

custom item predicate

11f8fa1

deirn mentioned this pull request Dec 31, 2023

Custom item predicates #3493

Closed

replace $ in fmj with \u0024

27200b2

use static registry method

a6dd97c

use custom codec impl

deirn and others added 10 commits January 3, 2024 07:59

Update fabric-item-api-v1/src/main/java/net/fabricmc/fabric/api/item/…

6f2536e

…v1/CustomItemPredicateRegistry.java Co-authored-by: apple502j <33279053+apple502j@users.noreply.github.com>

!

9d922b4

gametest

5dfeed7

move the tag (also suppress some warnings)

8a847fa

actually test matching stacks :)

184aa22

tabs

4238571

gametest but it still doesnt work

324f2c1

use Codec#parse

9f390a7

use static block

a26b354

gametest except it actually works fr this time

5ba68c1

AnAwesomGuy requested review from deirn and modmuss50 January 11, 2024 02:41

AnAwesomGuy added 5 commits January 21, 2024 21:29

merge custom item predicates into my branch

e04af50

whoops

cc2639a

stack-awareness (pt. 2) (loot works now)

4929c92

delete the accessor, its not needed now

9eafc64

deirn reviewed Jan 24, 2024

View reviewed changes

fabric-item-api-v1/src/main/java/net/fabricmc/fabric/impl/item/ShearsItemPredicate.java Outdated Show resolved Hide resolved

AnAwesomGuy added 3 commits January 25, 2024 08:38

whoops x2

55f0e56

finished adding custom predicate

c2b9129

MY VPN WORKED!!!!

975798f

AnAwesomGuy requested a review from deirn January 30, 2024 08:23

deirn reviewed Feb 9, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

De-hardcode shears (again) #3477

De-hardcode shears (again) #3477

AnAwesomGuy commented Dec 21, 2023 •

edited

ZestyBlaze commented Dec 21, 2023 •

edited

deirn commented Dec 21, 2023

Juuxel commented Dec 21, 2023

AnAwesomGuy commented Dec 22, 2023 •

edited

deirn left a comment •

edited

AnAwesomGuy commented Dec 29, 2023 •

edited

deirn commented Dec 29, 2023

AnAwesomGuy commented Dec 31, 2023

deirn commented Dec 31, 2023

AnAwesomGuy commented Jan 1, 2024

deirn Feb 9, 2024

AnAwesomGuy Feb 13, 2024

deirn Feb 13, 2024

deirn Feb 9, 2024

AnAwesomGuy Feb 13, 2024

deirn Feb 13, 2024

cech12 commented Apr 17, 2024

AnAwesomGuy commented Apr 17, 2024

cech12 commented Apr 25, 2024

cech12 commented Jun 7, 2024 •

edited

Patbox commented Jun 7, 2024

AnAwesomGuy commented Jun 7, 2024

De-hardcode shears (again) #3477

Are you sure you want to change the base?

De-hardcode shears (again) #3477

Conversation

AnAwesomGuy commented Dec 21, 2023 • edited

ZestyBlaze commented Dec 21, 2023 • edited

deirn commented Dec 21, 2023

Juuxel commented Dec 21, 2023

AnAwesomGuy commented Dec 22, 2023 • edited

deirn left a comment • edited

Choose a reason for hiding this comment

AnAwesomGuy commented Dec 29, 2023 • edited

deirn commented Dec 29, 2023

AnAwesomGuy commented Dec 31, 2023

deirn commented Dec 31, 2023

AnAwesomGuy commented Jan 1, 2024

deirn Feb 9, 2024

Choose a reason for hiding this comment

AnAwesomGuy Feb 13, 2024

Choose a reason for hiding this comment

deirn Feb 13, 2024

Choose a reason for hiding this comment

deirn Feb 9, 2024

Choose a reason for hiding this comment

AnAwesomGuy Feb 13, 2024

Choose a reason for hiding this comment

deirn Feb 13, 2024

Choose a reason for hiding this comment

cech12 commented Apr 17, 2024

AnAwesomGuy commented Apr 17, 2024

cech12 commented Apr 25, 2024

cech12 commented Jun 7, 2024 • edited

Patbox commented Jun 7, 2024

AnAwesomGuy commented Jun 7, 2024

AnAwesomGuy commented Dec 21, 2023 •

edited

ZestyBlaze commented Dec 21, 2023 •

edited

AnAwesomGuy commented Dec 22, 2023 •

edited

deirn left a comment •

edited

AnAwesomGuy commented Dec 29, 2023 •

edited

cech12 commented Jun 7, 2024 •

edited