-
Notifications
You must be signed in to change notification settings - Fork 436
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
Fabric API Provider #1072
Fabric API Provider #1072
Conversation
…ic/impl/component/access/ComponentTypeImpl.java Co-authored-by: i509VCB <i509vcb@gmail.com>
naming, final, use DFU pair, simplify error check
…fabric into fabric-component-access
...rovider-api-v1/src/main/java/net/fabricmc/fabric/api/provider/v1/BlockApiProviderAccess.java
Outdated
Show resolved
Hide resolved
...ovider-api-v1/src/main/java/net/fabricmc/fabric/api/provider/v1/EntityApiProviderAccess.java
Outdated
Show resolved
Hide resolved
...provider-api-v1/src/main/java/net/fabricmc/fabric/api/provider/v1/ItemApiProviderAccess.java
Outdated
Show resolved
Hide resolved
...ovider-api-v1/src/main/java/net/fabricmc/fabric/impl/provider/ApiProviderAccessRegistry.java
Outdated
Show resolved
Hide resolved
...provider-api-v1/src/main/java/net/fabricmc/fabric/api/provider/v1/ItemApiProviderAccess.java
Outdated
Show resolved
Hide resolved
...rovider-api-v1/src/main/java/net/fabricmc/fabric/api/provider/v1/BlockApiProviderAccess.java
Outdated
Show resolved
Hide resolved
...rovider-api-v1/src/main/java/net/fabricmc/fabric/api/provider/v1/BlockApiProviderAccess.java
Outdated
Show resolved
Hide resolved
...provider-api-v1/src/main/java/net/fabricmc/fabric/api/provider/v1/ItemApiProviderAccess.java
Outdated
Show resolved
Hide resolved
...vider-api-v1/src/main/java/net/fabricmc/fabric/impl/provider/BlockApiProviderAccessImpl.java
Outdated
Show resolved
Hide resolved
...vider-api-v1/src/main/java/net/fabricmc/fabric/impl/provider/BlockApiProviderAccessImpl.java
Outdated
Show resolved
Hide resolved
...ovider-api-v1/src/main/java/net/fabricmc/fabric/impl/provider/ItemApiProviderAccessImpl.java
Outdated
Show resolved
Hide resolved
...ider-api-v1/src/main/java/net/fabricmc/fabric/impl/provider/EntityApiProviderAccessImpl.java
Outdated
Show resolved
Hide resolved
fabric-provider-api-v1/src/main/java/net/fabricmc/fabric/api/provider/v1/ApiProviderAccess.java
Outdated
Show resolved
Hide resolved
...ovider-api-v1/src/main/java/net/fabricmc/fabric/impl/provider/ApiProviderAccessRegistry.java
Outdated
Show resolved
Hide resolved
…provider/EntityApiProviderAccessImpl.java Co-authored-by: i509VCB <i509vcb@gmail.com>
…provider/BlockApiProviderAccessImpl.java Co-authored-by: i509VCB <i509vcb@gmail.com>
…provider/ItemApiProviderAccessImpl.java Co-authored-by: i509VCB <i509vcb@gmail.com>
for (final ItemConvertible item : items) { | ||
Objects.requireNonNull(item, "encountered null item in API provider mapping"); | ||
|
||
if (map.putIfAbsent(item.asItem(), mapping) != null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make sure asItem
does not return null also
* @param <P> Identifies the API provider type | ||
* @param <A> Identifies the API type | ||
*/ | ||
@FunctionalInterface |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this really be declared as a functional interface if mods are supposed to add more methods ? It seems that the fact it can be implemented as a lambda is coincidental rather than really planned.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is an intentional suggestion that sub-types remain lambda-friendly, with the existing method and any overloads getting default implementations. If a provider function is going to be very complex it will likely need a context object and in that case a functional interface should also be viable.
That said, implementations aren't forced to maintain the functional interface contract if it doesn't work for them.
Another option I had considered was to remove ApiProvider entirely and allow any type at all. The system seemed more difficult to understand that way but it would be even more flexible - would make it easier to have providers that a "single hop" - the mapping function returns the API instance directly if the minimal built-in access patterns are sufficient.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems reasonable to keep the annotation then.
...ovider-api-v1/src/main/java/net/fabricmc/fabric/api/provider/v1/EntityApiProviderAccess.java
Outdated
Show resolved
Hide resolved
…rovider/v1/EntityApiProviderAccess.java Co-authored-by: Pyrofab <redstoneinfire@gmail.com>
This probably needs some form of pre-release to easily test it with potential candidate APIs that might use it (i.e. fluid, item transfer or similar) |
* @param mapping function that derives a provider instance from an entity | ||
* @param entityType type for which the mapping will apply | ||
*/ | ||
void registerProviderForEntity(Function<Entity, P> mapping, EntityType<?> entityType); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to convert function to an EntityProviderFunction
* @param mapping function that derives a provider instance from an item stack | ||
* @param items one or more types for which the mapping will apply | ||
*/ | ||
void registerProviderForItem(Function<ItemStack, P> mapping, ItemConvertible... items); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also an ItemStackProviderFunction here
* | ||
* @return instance to be returned when the API is not present or not available. | ||
*/ | ||
A absentApi(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to call this getAbsentApi
? getBlah
allows some languages like kotlin to infer the method as a val
and makes access in those languages much nicer
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, especially since Yarn uses bean prefixes for accessor methods.
* | ||
* @return the class for instances of the provided API | ||
*/ | ||
Class<A> apiType(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar concern to absentApi
* | ||
* @return an immutable, non-allocating {@code ApiProvider} instance that always returns {@link #absentApi()} | ||
*/ | ||
P absentProvider(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar concern to absentProvider
Objects.requireNonNull(apiType, "encountered null API type"); | ||
Objects.requireNonNull(absentProvider, "encountered null API absentProvider"); | ||
|
||
absentApi = absentProvider.getApi(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Be consistent here, qualify with this or don't qualify other fields
There is another PR being worked in FabLabs that picks up where this one left off. No reason to have it clutter up the place. |
# Fabric API Lookup API v1 ## Introduction This module allows Api instances to be associated with game objects without specifying how the association is implemented. This is useful when the same Api could be implemented more than once or implemented in different ways. Many thanks to @grondag for providing the original concept (#1072). Thanks also go to @i509VCB, @Pyrofab, @sfPlayer1 and the others who were involved with the design of this module. This is the foundation upon which can be built for example a fluid transfer api (#1166). Closes #1199. ## Flexible Api Querying ## Block Api Usage example ## Building blocks This PR was changed a lot, please have a look at the README, the package info, and the javadoc for `BlockApiLookup` and `ApiLookupMap` for up-to-date documentation. ## More usage examples FastTransferLib (https://github.com/Technici4n/FastTransferLib) is an experiment to build an item, fluid and energy transfer api on top of this module. (Which was until recently called `fabric-provider-api-v1`.) ## Missing things? ~~I could add an overload of `BlockApiLookup#find` with nullable `BlockState` and `BlockEntity` parameters, so that the caller can directly provide them if they are available for some reason.~~ Added in later commits. There is no module to retrieve apis from items or entities yet because there were unsolved issues with those. The community can use the provided building blocks to experiment with their own implementations of `ItemStackApiLookup` and `EntityApiLookup` until the way forward becomes clear, but let's please not delay the `BlockApiLookup` because of that. Co-authored-by: i509VCB <git@i509.me> Co-authored-by: PepperBell <44146161+PepperCode1@users.noreply.github.com>
# Fabric API Lookup API v1 ## Introduction This module allows Api instances to be associated with game objects without specifying how the association is implemented. This is useful when the same Api could be implemented more than once or implemented in different ways. Many thanks to @grondag for providing the original concept (#1072). Thanks also go to @i509VCB, @Pyrofab, @sfPlayer1 and the others who were involved with the design of this module. This is the foundation upon which can be built for example a fluid transfer api (#1166). Closes #1199. ## Flexible Api Querying ## Block Api Usage example ## Building blocks This PR was changed a lot, please have a look at the README, the package info, and the javadoc for `BlockApiLookup` and `ApiLookupMap` for up-to-date documentation. ## More usage examples FastTransferLib (https://github.com/Technici4n/FastTransferLib) is an experiment to build an item, fluid and energy transfer api on top of this module. (Which was until recently called `fabric-provider-api-v1`.) ## Missing things? ~~I could add an overload of `BlockApiLookup#find` with nullable `BlockState` and `BlockEntity` parameters, so that the caller can directly provide them if they are available for some reason.~~ Added in later commits. There is no module to retrieve apis from items or entities yet because there were unsolved issues with those. The community can use the provided building blocks to experiment with their own implementations of `ItemStackApiLookup` and `EntityApiLookup` until the way forward becomes clear, but let's please not delay the `BlockApiLookup` because of that. Co-authored-by: i509VCB <git@i509.me> Co-authored-by: PepperBell <44146161+PepperCode1@users.noreply.github.com> (cherry picked from commit dc716ea)
# Fabric API Lookup API v1 ## Introduction This module allows Api instances to be associated with game objects without specifying how the association is implemented. This is useful when the same Api could be implemented more than once or implemented in different ways. Many thanks to @grondag for providing the original concept (FabricMC#1072). Thanks also go to @i509VCB, @Pyrofab, @sfPlayer1 and the others who were involved with the design of this module. This is the foundation upon which can be built for example a fluid transfer api (FabricMC#1166). Closes FabricMC#1199. ## Flexible Api Querying ## Block Api Usage example ## Building blocks This PR was changed a lot, please have a look at the README, the package info, and the javadoc for `BlockApiLookup` and `ApiLookupMap` for up-to-date documentation. ## More usage examples FastTransferLib (https://github.com/Technici4n/FastTransferLib) is an experiment to build an item, fluid and energy transfer api on top of this module. (Which was until recently called `fabric-provider-api-v1`.) ## Missing things? ~~I could add an overload of `BlockApiLookup#find` with nullable `BlockState` and `BlockEntity` parameters, so that the caller can directly provide them if they are available for some reason.~~ Added in later commits. There is no module to retrieve apis from items or entities yet because there were unsolved issues with those. The community can use the provided building blocks to experiment with their own implementations of `ItemStackApiLookup` and `EntityApiLookup` until the way forward becomes clear, but let's please not delay the `BlockApiLookup` because of that. Co-authored-by: i509VCB <git@i509.me> Co-authored-by: PepperBell <44146161+PepperCode1@users.noreply.github.com>
# Fabric API Lookup API v1 ## Introduction This module allows Api instances to be associated with game objects without specifying how the association is implemented. This is useful when the same Api could be implemented more than once or implemented in different ways. Many thanks to @grondag for providing the original concept (FabricMC#1072). Thanks also go to @i509VCB, @Pyrofab, @sfPlayer1 and the others who were involved with the design of this module. This is the foundation upon which can be built for example a fluid transfer api (FabricMC#1166). Closes FabricMC#1199. ## Flexible Api Querying ## Block Api Usage example ## Building blocks This PR was changed a lot, please have a look at the README, the package info, and the javadoc for `BlockApiLookup` and `ApiLookupMap` for up-to-date documentation. ## More usage examples FastTransferLib (https://github.com/Technici4n/FastTransferLib) is an experiment to build an item, fluid and energy transfer api on top of this module. (Which was until recently called `fabric-provider-api-v1`.) ## Missing things? ~~I could add an overload of `BlockApiLookup#find` with nullable `BlockState` and `BlockEntity` parameters, so that the caller can directly provide them if they are available for some reason.~~ Added in later commits. There is no module to retrieve apis from items or entities yet because there were unsolved issues with those. The community can use the provided building blocks to experiment with their own implementations of `ItemStackApiLookup` and `EntityApiLookup` until the way forward becomes clear, but let's please not delay the `BlockApiLookup` because of that. Co-authored-by: i509VCB <git@i509.me> Co-authored-by: PepperBell <44146161+PepperCode1@users.noreply.github.com> (cherry picked from commit dc716ea)
Replaces #1067
This module allows API instances to be associated with game objects without specifying how the association is implemented. This is useful when the same API could be implemented more than once or implemented in different ways.
For example, a fluid API might be implemented for a block in the following ways:
This variation is managed by a mapping function given when an API provider is registered. The provider interface
is meant to be extended as needed to handle a given use case. (Sidedness, instance ID, permissions, etc)
Once registered, API providers can be retrieved without any knowledge of the API or provider implementations, and then used according to the contract of the provider to obtain an API instance.
Note that this model allows API providers to be associated with game objects that don't natively support them. This will require a wrapper or data attachment facility (or mixins) to be effective in most cases, all of which are outside the scope of this module. However, the decoupling of retrieval from implementation affords maximum flexibility for implementors.
This module could easily extended to cover additional game objects and probably should be if accepted.
This module could be implemented with slightly more efficiency if a data attachment facility were available, but that is left for another day.
Here is a simple (and silly) example of usage: