-
-
Notifications
You must be signed in to change notification settings - Fork 213
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
Refactor inventory #1585
Refactor inventory #1585
Conversation
7b8d009
to
cd09f70
Compare
1abba2a
to
93c7543
Compare
c343a42
to
bf2beff
Compare
1a47e05
to
f2fce41
Compare
I think I'm done with it for now. |
@SimonFlash @codeHusky @TheoKah @ustc-zzzz @Zerthick @GreWeMa @DosMike As developers with plugins that interact with inventories, I'd appreciate it if you guys managed to test these changes, and see if the inventories structure makes sense. @Faithcaio Is a build able to be made for these guys to assist (if they wish to), or is there API changes that go along with this that they might need? |
API is changed SpongePowered/SpongeAPI#1710 (but not much) |
@Faithcaio I've edited it into the OP. |
All of my inventory operations are functional on SpongeForge with 2555. The subset I use covers basic inventory operations, queries, and retrieving slots from indexes, so anything beyond that I can't confirm. |
76fa6f9
to
7d55c45
Compare
reduce code duplication make some lenses reusable make internal inventory structure more consistent
7d55c45
to
9eaa7b5
Compare
SpongeAPI | SpongeCommon | SpongeForge
Restructure internal Inventory classes:
InventoryAdapter
Adapter interface (implements Inventory)MinecraftInventoryAdapter
default implement most ofInventory
AbstractInventoryAdapter
Adapter implementation (all types)VanillaAdapter
(IInventory)OrderedInventoryAdapter
etc.SlotAdapter
(Wrapper for inventories without realSlot
)FilteringSlotAdapter
etc.MinecraftQueryResultAdapter
IItemHandlerAdapter
(Forge)MixinTraitInventory
Implement Inventory interface using Adapter InterfaceMixinTraitAdapter
Implement Adapter interface (based on Reusable/LensProvider)MixinInvWrapper
,MixinItemStackHandler
IItemHandlerAdapter
MixinSlot
, (Container Slots)MixinSlotItemHandler
(Forge)MixinCustomInventory
,MixinSpongeUserInventory
Lens
AbstractLens
RealLens
Lenses for real inventories (gets real inventories if mixed in)ContainerLens
constructed with lenses of sub-inventoriesContainerPlayerInventoryLens
CustomLens
(for Sponge Inventories)FurnaceInventoryLens
etc.ConceptualLens
Lenses for inventory conceptsCompoundLens
used for union operationOrderedInventoryLensImpl
etc.DelegatingLens
(used in Containers) spanning child is orderedlens ; child is delegateDefaultIndexedLens
fallbackQueryLens
for query resultsSlotLens
->SlotLensImpl
,FakeSlotLensImpl
etc.DefaultEmptyLens
Fabric
MinecraftFabric
(IInventory based)CompoundFabric
(based on 2 other MinecraftFabric)ContainerFabric
(based on Set of IInventory in Container)IInventoryFabric
(based on a single IInventory)SlotFabric
(based on a single Slot)Move
Adapter.Logic
into its own class.Move as much implementation into the same place instead of duplicating it.
Lazy Lens creation (MixinTraitAdapter) based on LensProvider interfaces
Reusable Lenses (lenses for TileEntities only need to be constructed once)
A nice sideeffect of this whole thing is that you can now query for TileEntity.class on Containers and you get it (for vanilla / TileEntities we applied mixin to).
Also Container structure is more uniform (Spanning Children are always VanillaAdapters with an OrderedInventory ; VanillaAdapter with delegating lens can forward to the real inventory + we can reuse the inventory lens in the container)