Rework of AttachCapabilitiesEvent #9718

RealMangorage · 2023-09-20T07:18:17Z

PR for reworking the AttachCapabillitiesEvent.

Currently in a prototype/testing stage.
What is currently made is not final product. Changes may occur.

Documentation: Proposal (WIP)

We add a new class which handles all event posts for AttachCapabilitiesEvent (Which now extends Event, and not GenericEvent)

> CapabilitySystem.java

CapabilitySystem.addListener(Class<T> yourClassToListenFor, Consumer<AttachCapabilitiesEvent<T>> eventConsumer);

CapabilitySystem.addWrappedListener(Class<T> yourClassToListenFor, Class<W> classYourActuallyGonnaGet Consumer<AttachCapabilitiesEvent<W>> eventConsumer);

Examples:

CapabilitySystem.addListener(Player.class, e -> {}); // This will only be called and supplied with objects that are Player.class or extend Player.class.
 CapabilitySystem.addWrappedListener(ItemStack.class, BlockItem.class, ItemStack::getItem,  (event, item) -> {}); // This will only be called and supplied with objects who use BlockItem.class or extend BlockItem.class, and the objects it returns in this case will be ItemStack. Use .getItem() to get the item or use "item" to get it without needing to cast it.

Jonathing · 2023-09-20T14:11:39Z

Relevant conversation from Discord.

KnightMiner · 2023-09-21T04:10:18Z

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

    protected final BlockPos f_58858_;
    protected boolean f_58859_;
    private BlockState f_58856_;
+-


This patch change looks wrong to me, looks like this PR added a trivial whitespace change to the patch

Agreed. Please remove it so we can keep the patch surface as low as possible.

autoforge · 2023-09-21T17:57:53Z

@RealMangorage, this pull request has conflicts, please resolve them for this PR to move forward.

Jonathing

Effectively needs a rebase.

More info: https://discord.com/channels/1129059589325852724/1129059590156337154/1155564202853937283

Jonathing · 2023-09-25T11:33:10Z

src/main/java/net/minecraftforge/common/capabilities/CapabilitySystem.java

+import java.util.function.BiConsumer;
+import java.util.function.Consumer;
+
+public class CapabilitySystem {


Bracket on new line, please

Jonathing · 2023-09-25T11:33:42Z

src/main/java/net/minecraftforge/common/capabilities/CapabilitySystem.java

+    public static <T, W> void addWrappedListener(Class<T> type, Class<W> wrappedClass, BiConsumer<AttachCapabilitiesEvent<T>, W> eventConsumer)
+    {
+        if (!Item.class.isAssignableFrom(type)) throw new IllegalStateException("Unable to add Listener for Items. Use CapabilitySystem.addListener(Class, Consumer) for non Item related stuff");
+        LISTENER_LIST.computeIfAbsent(type, (e) -> new ArrayList<>()).add((cls) -> {


I don't remember if lambda brackets also go on new line. @LexManos?

Jonathing · 2023-09-25T20:11:18Z

src/main/java/net/minecraftforge/common/extensions/IForgeEntity.java

@@ -448,4 +448,4 @@ default boolean shouldUpdateFluidWhileBoating(FluidState state, Boat boat)
    {
        return boat.shouldUpdateFluidWhileRiding(state, self());
    }
-}


Unnecessary whitespace edit

Jonathing · 2023-09-25T20:11:26Z

src/main/java/net/minecraftforge/common/extensions/IForgeLevelChunk.java

@@ -10,4 +10,4 @@
 public interface IForgeLevelChunk extends ICapabilityProvider
 {

-}


Unnecessary whitespace edit

Jonathing · 2023-09-25T20:11:41Z

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

@@ -1250,5 +1250,4 @@ public static void handleClientConfigurationComplete(Connection connection) {
        } else
            LOGGER.info("Connected to a modded server.");
    }
-
 }


Unnecessary whitespace edit

Jonathing · 2023-09-25T20:11:55Z

src/main/java/net/minecraftforge/fluids/capability/wrappers/FluidBucketWrapper.java

@@ -174,4 +174,4 @@ public <T> LazyOptional<T> getCapability(@NotNull Capability<T> capability, @Nul
    {
        return ForgeCapabilities.FLUID_HANDLER_ITEM.orEmpty(capability, holder);
    }
-}


Unnecessary whitespace edit

Jonathing · 2023-09-25T20:12:17Z

src/main/java/net/minecraftforge/event/AttachCapabilitiesEvent.java

-        if (caps.containsKey(key))
-            throw new IllegalStateException("Duplicate Capability Key: " + key  + " " + cap);


Unnecessary whitespace edit (unless this is an intentional part of the refactor)

Jonathing · 2023-09-25T20:13:49Z

src/test/java/net/minecraftforge/debug/CapabilitiesTest.java

Similar formatting issues to CapabilitySystem.java. Inconsistent usage of new-line brackets.

Jonathing · 2023-09-25T20:14:10Z

src/test/resources/META-INF/mods.toml

+[[mods]]
+modId="captest"

 [[mods]]
 modId="creative_mode_tab_test"
-
+[[mods]]
+modId="captest"


I think the test mod only needs to be declared in this file once.

Jonathing · 2023-09-25T20:14:30Z

src/test_old/java/net/minecraftforge/debug/CapabilitiesTest.java

@@ -40,26 +42,44 @@ public class CapabilitiesTest
 {
    public static final String MODID = "capabilities_test";

-    private static final boolean ENABLED = false;
+    private static final boolean ENABLED = true;


Don't forget to disable this after the PR is out of draft state

Jonathing

Good shit so far. It works a lot better than the horse shit we had before, especially with the explicit class listening. Obviously it needs a lot more testing, but it's headed in the right direction. Like I said in VC, I am happy to help with remedial shit like javadocs and the primer.

Jonathing · 2023-09-28T00:49:26Z

src/main/java/net/minecraftforge/common/capabilities/CapabilitySystem.java

This class needs javadocs, especially since it looks like modders are going to be interfacing with this.

Jonathing · 2023-09-28T00:50:52Z

src/test/java/net/minecraftforge/debug/CapabilitiesTest.java

+{
+    public static final String MOD_ID = "captest";
+    private static final ConcurrentHashMap<Class<?>, AtomicInteger> TRACK = new ConcurrentHashMap<>();
+    private static final boolean ENABLED = true;


Remember to set this to false before marking the PR as ready!

…r.getAttachCapabilitiesEventFactory for getting the Event releated to AttachCapabilitiesEvent.java

…xception when attempting to setClassType() after being already set. Made AttachCapabilitiesEvent.java final, so it cannot be extended. Refactored CapabilitySystem field find -> LISTENER_LIST and static func find -> getListenerList.

…tener.

…istener

autoforge · 2023-10-07T04:02:21Z

@RealMangorage, this pull request has conflicts, please resolve them for this PR to move forward.

autoforge · 2023-10-08T02:23:36Z

@RealMangorage, this pull request has conflicts, please resolve them for this PR to move forward.

autoforge bot added Triage This request requires the active attention of the Triage Team. Requires labelling or reviews. 1.20 labels Sep 20, 2023

autoforge bot requested a review from a team September 20, 2023 07:18

RealMangorage marked this pull request as draft September 20, 2023 14:09

KnightMiner reviewed Sep 21, 2023

View reviewed changes

autoforge bot added the Needs Rebase This PR requires a rebase to implement upstream changes (usually to fix merge conflicts). label Sep 21, 2023

autoforge bot removed the Needs Rebase This PR requires a rebase to implement upstream changes (usually to fix merge conflicts). label Sep 22, 2023

Jonathing suggested changes Sep 24, 2023

View reviewed changes

RealMangorage force-pushed the 1.20.x-AttachCapEventRework branch from e8ed973 to 37a373a Compare September 25, 2023 07:12

Jonathing suggested changes Sep 25, 2023

View reviewed changes

Jonathing suggested changes Sep 28, 2023

View reviewed changes

RealMangorage added 17 commits October 5, 2023 02:26

Inital Commit for Rework of AttachCapabilitiesEvent

dee9c0e

Another update. Removed EventFinder.class. Added a ICapabilityProvide…

a1ea28a

…r.getAttachCapabilitiesEventFactory for getting the Event releated to AttachCapabilitiesEvent.java

Cleaned up CapabilityProvider and ForgeEventFactory

b82e926

Initial Update to 1.20.2

258f067

Inital Commit for Rework of AttachCapabilitiesEvent

a062261

Another update. Removed EventFinder.class. Added a ICapabilityProvide…

88feabe

…r.getAttachCapabilitiesEventFactory for getting the Event releated to AttachCapabilitiesEvent.java

Fixed BlockEntity.java.patch

630622c

Cleaned up CapabilityProvider and ForgeEventFactory

08229ec

Inital Commit for Rework of AttachCapabilitiesEvent

8cf4a4e

Another update. Removed EventFinder.class. Added a ICapabilityProvide…

97b5fc9

…r.getAttachCapabilitiesEventFactory for getting the Event releated to AttachCapabilitiesEvent.java

Try to fix smtn

f7a2bf2

Updated CapabilitiesTest.java

0c5104f

Updated Again. Still WIP

a512e47

Inital Commit for Rework of AttachCapabilitiesEvent

9a9c4f1

Another update. Removed EventFinder.class. Added a ICapabilityProvide…

838e438

…r.getAttachCapabilitiesEventFactory for getting the Event releated to AttachCapabilitiesEvent.java

Updated CapabilitiesTest.java

0ae4691

Updated Again. Still WIP

44bc729

RealMangorage added 15 commits October 5, 2023 02:27

Cleaned up some further stuff.

e46c7cd

Cleaned up some further stuff.

f0b63fb

Cleaned up stuff.

f1f19ea

Minor Fixes

2ec41ab

Minor Fixes

4b5a6d5

Minor Fixes

4ab110f

Minor Fixes

5f66655

Minor Fixes

2bde82c

Minor mods.toml fixes for Test module.

56179a4

Further improvements to CapabilitySystem.addWrappedListener

aa0965b

Updated CapabilitySystem to have a CapabilitySystem.addWrappedItemLis…

ade04d7

…tener.

Implemented a Caching system for CapabilitySystem's listeners.

9fbf209

Updated Again. Still WIP

c7e709f

Added proper caching system for CapabilitySystem.java listeners.

7743132

RealMangorage force-pushed the 1.20.x-AttachCapEventRework branch from 28a0bda to 7743132 Compare October 5, 2023 09:38

RealMangorage added 6 commits October 5, 2023 03:12

Fixes

45b65aa

Fixes

d76dcc1

Fixes

d13e91b

Fixes

11280c0

Removed un-needed stuff

78f1f8f

Made it so you can now use Interfaces to filter CapabilitySystem.addL…

f931293

…istener

autoforge bot added the Needs Rebase This PR requires a rebase to implement upstream changes (usually to fix merge conflicts). label Oct 7, 2023

autoforge bot removed the Needs Rebase This PR requires a rebase to implement upstream changes (usually to fix merge conflicts). label Oct 8, 2023

RealMangorage force-pushed the 1.20.x-AttachCapEventRework branch from 38f5be7 to f931293 Compare October 8, 2023 02:23

autoforge bot added the Needs Rebase This PR requires a rebase to implement upstream changes (usually to fix merge conflicts). label Oct 8, 2023

forgedev-teamcity force-pushed the 1.20.x branch from cb455e6 to 2d5ba40 Compare March 7, 2024 01:05

LexManos force-pushed the 1.20.x branch from c9560ee to a6978ee Compare March 7, 2024 01:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rework of AttachCapabilitiesEvent #9718

Rework of AttachCapabilitiesEvent #9718

RealMangorage commented Sep 20, 2023 •

edited

Jonathing commented Sep 20, 2023

KnightMiner Sep 21, 2023

Jonathing Sep 21, 2023

autoforge bot commented Sep 21, 2023

Jonathing left a comment

Jonathing Sep 25, 2023

Jonathing Sep 25, 2023

Jonathing Sep 25, 2023

Jonathing Sep 25, 2023

Jonathing Sep 25, 2023

Jonathing Sep 25, 2023

Jonathing Sep 25, 2023

Jonathing Sep 25, 2023

Jonathing Sep 25, 2023

Jonathing Sep 25, 2023

Jonathing left a comment

Jonathing Sep 28, 2023

Jonathing Sep 28, 2023

autoforge bot commented Oct 7, 2023

autoforge bot commented Oct 8, 2023

		if (caps.containsKey(key))
		throw new IllegalStateException("Duplicate Capability Key: " + key + " " + cap);

Rework of AttachCapabilitiesEvent #9718

Are you sure you want to change the base?

Rework of AttachCapabilitiesEvent #9718

Conversation

RealMangorage commented Sep 20, 2023 • edited

> CapabilitySystem.java

Jonathing commented Sep 20, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

autoforge bot commented Sep 21, 2023

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

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

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

autoforge bot commented Oct 7, 2023

autoforge bot commented Oct 8, 2023

RealMangorage commented Sep 20, 2023 •

edited