SlimefunItemStack reimplementation
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
NonnullandNullableannotations to my methods to indicate their behaviour for null values - [ ] I added sufficient Unit Tests to cover my code.
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! ๐
Really not a fan of getDelegate tbh, not sure a great other solution tho
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
Slimefun preview build
A Slimefun preview build is available for testing! Commit: 3adafcdc
https://preview-builds.walshy.dev/download/Slimefun/4251/3adafcdc
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!
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~~ Now creating defensive copies
- ~~Run delombok to remove the dependency that I put on SlimefunItemStack~~
tested this and everything seems to be working fine. no errors in console and all items working as intended.
WrongItemStackException isn't used anymore, should it be removed altogether? also SlimefunItem#isItemStackImmutable is now useless as I am returning defensive copies, should I remove that one too?
WrongItemStackException isn't used anymore, should it be removed altogether? also
SlimefunItem#isItemStackImmutableis now useless as I am returning defensive copies, should I remove that one too?
Search for the exception and the function, and the result is only Slimefun core and its fork. No other plugins use them, so I say just remove them.
Pull request made: https://github.com/Intybyte/Slimefun4-Fixes/pull/3 or you want to use patch: https://discord.com/channels/565557184348422174/565570276038017044/1309652382279401605
retested 11-24-24 all seems to be working fine on preview build 6d38511a
No need to retest from 0 the last commit as it just removes unused code
We've done a ton of testing, had no issues, everything is working perfectly
What is preventing this from being merged?
Starting to look like someone is too lazy to press merge
Moving bundle features to another PR as they are out of scope here #4278