Skip to content
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

[WIP] [RFC] Early WIP of extension slot API proof of concept. #1183

Draft
wants to merge 1 commit into
base: 1.21.x
Choose a base branch
from

Conversation

gigaherz
Copy link
Contributor

Nothing here is final. Even this description is just a braindump.

There have always been mods that extend the player inventory (and sometimes other entities). Usually, though, there was only one popular option for any major version. The situation may be changing.

Here is a very early proposal for what I would like an API that unifies access to these mod's extension slots. It is loosely based on my own code I wrote years ago with the idea of eventually making a PR to forge. https://github.com/gigaherz/ToolBelt/tree/1.20.1/src/main/java/dev/gigaherz/toolbelt/customslots

My use case is that my Tool Belt "finds" the belt slot when opening the GUI, while the gui is open, I require consistent access to the slot (meaning I can check every tick if the belt is still in that slot), and if I actually implemented a feature that has been in my TODO list, I would need to mutate the stack or replace it in the slot. So it would greatly benefit me if we had a shared API for accessing these slots.

I am sure there are many uses cases which I have not yet considered. I am prepared to make any changes necessary for them, or even to make way for someone else's design.

This API has been designed to fulfill these requirements:

  1. the ability to identify all "belt" slots from all mods, in one place
  2. the ability to uniquely identify one specific "belt" slot among the multiple
  3. the ability to re-query and set the contents of that slot

The design includes:

  • A capability to gather all the extension slots, and provide access to them by "type".
  • An interface slots implement to allow access to the slot.
  • An interface for items to provide special handling for equip, unequip, and ticking.
  • Querying a slot type returns a collection of slots matching that type.
  • The returned objects are to be consistent and valid across ticks (eg. while a GUI remains open).
  • The objects that fit in a slot are decided based on a tag, but can provide additional filtering in their handler.

Questions I have for the community:

  • Should slot types be a registry? this would allow having slot type tags, and it would allow having common tags for slot types so people can query all "c:belt" slots
  • Should be API to submit slots to the query handler be part of the query interface? or something external
  • Is it ok to expect and require that all inventory extensions use tags to decide if items go in the slot?

Additional considerations:

  • Should the API provide methods for storage of the slot contents? (IMO, no, or at least not mandatory, since slots could work differently, eg. someone could make a set of inventory extension slots that is shared among people of one team or some such)
  • Should the API provide methods for handling death in a proper way? (IMO, yes, but we mostly do that already https://github.com/gigaherz/ToolBelt/blob/master/src/main/java/dev/gigaherz/toolbelt/slot/BeltAttachment.java#L130-L170)
  • Should be API provide facilities for synchronizing slots to the client?
  • Should the API provide facilities for displaying the slots in a GUI?

@neoforged-pr-publishing
Copy link

  • Publish PR to GitHub Packages

@Matyrobbrt Matyrobbrt added enhancement New (or improvement to existing) feature or request help wanted Extra attention is needed request for comments For gathering opinions on some topic or subject 1.21 Targeted at Minecraft 1.21 labels Jun 25, 2024
@Caltinor
Copy link
Contributor

Caltinor commented Jul 2, 2024

Understanding that this implementation is a draft, I don't see how someone would query every slot from every extension. Conceptually, what would that look like?

For a use-case and example, Project MMO gives players bonuses based on the items they have equipped. With something like Curios, we could scan all the slots and their contained items and tally up the bonuses from there. Based on the implementation, as I read here, I would need to know the extension's registry name to access the slots. Would the correct approach here be to get all keys from the registry, then iterate over those with the getSlots(key) to nested iterate to check each slot?

@gigaherz
Copy link
Contributor Author

gigaherz commented Jul 3, 2024

Understanding that this implementation is a draft, I don't see how someone would query every slot from every extension. Conceptually, what would that look like?

That is indeed an important use case that I didn't consider in the initial draft since I assumed enumeration would happen for a fixed set of slots (eg. in ToolBelt I only care about belts, and in Elements of Power I have headband, necklace, bracelet and ring).

For a use-case and example, Project MMO gives players bonuses based on the items they have equipped. With something like Curios, we could scan all the slots and their contained items and tally up the bonuses from there. Based on the implementation, as I read here, I would need to know the extension's registry name to access the slots. Would the correct approach here be to get all keys from the registry, then iterate over those with the getSlots(key) to nested iterate to check each slot?

While it would be possible to enumerate if the slot types became a registry, it would probably be best to still provide access to the keyset of the multimap. 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.21 Targeted at Minecraft 1.21 enhancement New (or improvement to existing) feature or request help wanted Extra attention is needed request for comments For gathering opinions on some topic or subject
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants