SlimefunItemStack reimplementation #4251

Intybyte · 2024-10-22T20:31:57Z

Description

We can't extend itemstack anymore we need to fix EVERYTHING

Proposed changes

Use a delegate instead of extending itemstack

Related Issues (if applicable)

Checklist

I have fully tested the proposed changes and promise that they will not break everything into chaos.
I have also tested the proposed changes in combination with various popular addons and can confirm my changes do not break them.
I have made sure that the proposed changes do not break compatibility across the supported Minecraft versions (1.16.* - 1.20.*).
I followed the existing code standards and didn't mess up the formatting.
I did my best to add documentation to any public classes or methods I added.
I have added Nonnull and Nullable annotations to my methods to indicate their behaviour for null values
I added sufficient Unit Tests to cover my code.

github-actions · 2024-10-22T20:32:23Z

Pro Tip!
You can help us label your Pull Requests by using the following branch naming convention next time you create a pull request. ❤️

Branch naming convention	Label
`feature/**`	🎈 Feature
`fix/**`	✨ Fix
`chore/**`	🧹 Chores
`api/**`	🔧 API
`performance/**`	💡 Performance Optimization
`compatibility/**`	🤝 Compatibility

If your changes do not fall into any of these categories, don't worry. You can just ignore this message in that case! 👀

ybw0014 · 2024-10-22T21:15:13Z

src/main/java/io/github/thebusybiscuit/slimefun4/api/items/SlimefunItemStack.java

@@ -306,13 +307,13 @@ public void lock() {
    }

    @Override
-    public ItemStack clone() {


should return ItemStack

That wouldn't be good logic wise though

src/main/java/io/github/thebusybiscuit/slimefun4/api/items/SlimefunItemStack.java

ybw0014 · 2024-10-22T22:25:25Z

pom.xml

@@ -410,6 +410,13 @@
            </exclusions>
        </dependency>

+        <dependency>
+            <groupId>org.projectlombok</groupId>
+            <artifactId>lombok</artifactId>


i wonder if introducing a new dep is a good idea.

It is temporary for now, it makes use of all of itemstacks method so we don't have to reimplement it, once it works we can reimplement everything ourself i guess

...io/github/thebusybiscuit/slimefun4/implementation/items/electric/machines/ElectricPress.java

JustAHuman-xD · 2024-10-25T08:18:36Z

Really not a fan of getDelegate tbh, not sure a great other solution tho

JustAHuman-xD · 2024-10-25T08:21:18Z

src/main/java/io/github/thebusybiscuit/slimefun4/api/items/SlimefunItemStack.java

+    /**
+     * @return underlying ItemStack used
+     */
+    public @Nonnull ItemStack getDelegate() {
+        return delegate;
+    }


Also pretty sure we don't want to do this because then they can just modify delegate which we previously prevent, it'll probably be the case regardless of what we do but gonna point it out anyways

src/main/java/io/github/thebusybiscuit/slimefun4/core/commands/subcommands/BackpackCommand.java

JustAHuman-xD · 2024-10-25T08:24:27Z

src/main/java/io/github/thebusybiscuit/slimefun4/api/items/SlimefunItem.java

+    public static @Nullable SlimefunItem getByItem(@Nullable SlimefunItemStack slimefunItemStack) {
+        if (slimefunItemStack == null) {
+            return null;
+        }
+
+        var delegate = slimefunItemStack.getDelegate();
+        if (delegate.getType() == Material.AIR) {
+            return null;
+        }
+
+        return getById(slimefunItemStack.getItemId());
+    }


Not sure why the material check is here, can use isEmpty but regardless it's unnecessary, also if we add this method we should probably add the others as well, though its a lot more fluff so ehh

Yeah i am adding these methods where i can for the time being, it is not like i can refactor everything with a simple replace either so this saves from time compared to just replacing stuff normally by hand

@DeleGate

NOTE TO FUTURE SELF: Use ForwardingObject over lombok's @DeleGate

Intybyte · 2024-10-25T11:40:37Z

Once this works i will try replacing it with google's ForwardingObject rather than adding a new dependency, similar to how many paper classes are implemented so to not get surprises later on

(Edit) this breaks a lot of compatibility compared to using lombok as you need to first get the delegate and then run the methods on the delegate, instead lombok generates code to access said delegate methods directly

github-actions · 2024-11-09T19:38:58Z

Slimefun preview build

A Slimefun preview build is available for testing!
Commit: e362999

https://preview-builds.walshy.dev/download/Slimefun/4251/e362999f

Note: This is not a supported build and is only here for the purposes of testing.
Do not run this on a live server and do not report bugs anywhere but this PR!

# Conflicts: # pom.xml # src/main/java/io/github/thebusybiscuit/slimefun4/api/gps/GPSNetwork.java # src/main/java/io/github/thebusybiscuit/slimefun4/implementation/items/electric/machines/CarbonPress.java # src/main/java/io/github/thebusybiscuit/slimefun4/implementation/items/electric/machines/ElectricPress.java # src/main/java/io/github/thebusybiscuit/slimefun4/implementation/items/electric/machines/HeatedPressureChamber.java # src/main/java/io/github/thebusybiscuit/slimefun4/implementation/items/electric/reactors/Reactor.java # src/main/java/io/github/thebusybiscuit/slimefun4/implementation/items/magical/talismans/EnderTalisman.java # src/main/java/io/github/thebusybiscuit/slimefun4/implementation/items/magical/talismans/Talisman.java

Intybyte · 2024-11-12T10:23:49Z

No idea why it still says Merge Conflicts when i already solved those, it looks like there are some problems, rewriting them here for consistency

SoulBound tests don't pass yet
Locked items needs a new implementation, their tests are temporarily disabled
Run delombok to remove the dependency that I put on SlimefunItemStack

Intybyte added 2 commits October 22, 2024 21:56

Fix tests

6f2f30d

Use delegation... this is gonna be a long pr

a62ecff

WalshyDev changed the base branch from master to walshy/mc-1.21 October 22, 2024 20:33

ybw0014 suggested changes Oct 22, 2024

View reviewed changes

Intybyte added 2 commits October 23, 2024 13:15

Address review

75d9c75

Refactor some more stuff

c8efc79

JustAHuman-xD requested changes Oct 25, 2024

View reviewed changes

Intybyte added 2 commits October 25, 2024 13:19

More fixed, most notably ChestMenuUtils

70c96e0

NOTE TO FUTURE SELF: Use ForwardingObject over lombok's @DeleGate

More refactoring

e7aefc6

Refactor getDelegate to item, and some small changes

b646376

This was referenced Nov 7, 2024

Update to 1.21 #4248

Open

1.21 support #4254

Open

TheBusyBot added the ⚡ Merge Conflicts This Pull Request has merged conflicts which need to be resolved! label Nov 9, 2024

Intybyte force-pushed the walshy/mc-1.21-sfi branch from 7e5f09a to d12ae85 Compare November 9, 2024 19:36

Intybyte force-pushed the walshy/mc-1.21-sfi branch from d12ae85 to b646376 Compare November 9, 2024 19:42

Intybyte added 11 commits November 9, 2024 20:54

Small error on merge

35396d0

Merge fixes + more fixes

dfa4e94

GrindStone fix

3435390

MagicEyeOfEnder fix

3512cd3

NetherStarReactor fix

af27545

NuclearReactor fix

1b73f98

OilPump fix

1f28097

OreCrusher fix

cc45f7d

OrganicFertilizer fix

5e25492

OrganicFood fix

8969397

Intybyte added 26 commits November 9, 2024 22:11

TreeGrowthAccelerator fix

3497af0

Add method for SlimefunItemStack

7749d43

Smeltery Fix

69ecf93

Smeltery Fix

810ad17

SlimefunItemInteractListener Fix

a5094b1

SaltResource Fix

5d1952d

Yes. I wrote a parser just for this.

e10da88

Couldn't write a parser for the finishing touches tho...

8c25fff

Fix slimefunItem + TODO

a85442f

Fix RecipeType

e94b355

Fix AncientAltarListener

64c5e67

Fix ArmorTask

e0e850b

Fix DeathpointListener

81c600d

Fix DefaultItemGroups

8d940bd

Fix GadgetsListener

45d3231

Fix Resources

0cd88ca

Fix Resources 2

e141d41

Fix SlimefunUtils

eec08ce

Fix Some tests to call .item();

ff3b5ec

Ok lets fire those tests

b013127

Forgot those

f409400

Looks like there is no reason to keep that there

2683be5

TODOs

f4c0864

SlimefunItemStack can't be casted to ItemStack anymore

861d303

Fix null throwing

26b5a43

Fix init

60f183b

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SlimefunItemStack reimplementation #4251

SlimefunItemStack reimplementation #4251

Intybyte commented Oct 22, 2024

github-actions bot commented Oct 22, 2024

ybw0014 Oct 22, 2024

Intybyte Oct 23, 2024

ybw0014 Oct 22, 2024

Intybyte Oct 23, 2024

JustAHuman-xD commented Oct 25, 2024

JustAHuman-xD Oct 25, 2024

JustAHuman-xD Oct 25, 2024

Intybyte Oct 25, 2024

Intybyte commented Oct 25, 2024 •

edited

Loading

github-actions bot commented Nov 9, 2024

Intybyte commented Nov 12, 2024

SlimefunItemStack reimplementation #4251

Are you sure you want to change the base?

SlimefunItemStack reimplementation #4251

Conversation

Intybyte commented Oct 22, 2024

Description

Proposed changes

Related Issues (if applicable)

Checklist

github-actions bot commented Oct 22, 2024

ybw0014 Oct 22, 2024

Choose a reason for hiding this comment

Intybyte Oct 23, 2024

Choose a reason for hiding this comment

ybw0014 Oct 22, 2024

Choose a reason for hiding this comment

Intybyte Oct 23, 2024

Choose a reason for hiding this comment

JustAHuman-xD commented Oct 25, 2024

JustAHuman-xD Oct 25, 2024

Choose a reason for hiding this comment

JustAHuman-xD Oct 25, 2024

Choose a reason for hiding this comment

Intybyte Oct 25, 2024

Choose a reason for hiding this comment

Intybyte commented Oct 25, 2024 • edited Loading

github-actions bot commented Nov 9, 2024

Slimefun preview build

Intybyte commented Nov 12, 2024

Intybyte commented Oct 25, 2024 •

edited

Loading