-
Notifications
You must be signed in to change notification settings - Fork 68
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
Add GUI Registry pre/post hooks #215
Open
MattSturgeon
wants to merge
7
commits into
shedaniel:v8
Choose a base branch
from
MattSturgeon:register-hooks
base: v8
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
6e3893e
to
9e3f6e0
Compare
d789d6c (debug prints) is included in this PR for demonstrative purposes only. It shouldn't actually be merged. For additional example usage, you could refer to my in-progress autoconfig-dependencies branch: public class DefaultGuiHooks {
public static GuiRegistry apply(GuiRegistry registry) {
registry.registerPostHook((guis, i18n, field, config, defaults, guiProvider) ->
guiProvider.getLookupTable().register(field, guis));
registry.registerPostHook((guis, i18n, field, config, defaults, guiProvider) ->
guiProvider.getRequirementManager().registerRequirements(guis, field));
return registry;
}
} |
Rather than allocating a large hash table and hashing the keys, we can use a specialized map that is far more efficient in both speed and size. EnumMap is implemented using a simple array, whose size matches the number of enum values.
Inline `firstPresent()` and take advantage of streams being lazy to make things more readable, without loosing any efficiency. Also, take advantage of `EnumMap` being ordered; we don't need to map `Priority.values()` using `providers.get()` to maintain order anymore. The stream will go one by one through the provider lists (in order), testing each entry using the filter. Once an entry matches, the stream will short-circuit and run the mapping function.
Using a stream reduction to transform the GUI list. Used 3-arg `reduce` because the 2-arg version requires that input & output are the same type. Since our use case doesn't support parallel reduction, an exception is thrown if the combinator is invoked.
Allow registering event handler "hooks" that are triggered either before or after generating a GUI using `GuiRegistry`. Intended to enable autoconfig requirements.
9e3f6e0
to
d789d6c
Compare
I've pushed some changes and clarified the PR description. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
This PR adds the ability to register pre & post hooks with the
GuiRegistry
. Registered hooks are run before and after eachField
is transformed into GUIs, allowing users to (for example) get a reference to the final GUIs.Motivation
A user could achieve a similar effect using a
GuiTransformer
, however there is a risk that another transformer may "pull the rug out" and remove/replace the GUI in question. Hooks solve this issue with an immutable GUI list and not using a return value. Therefore a post hook is guaranteed to run after all destructive changes have happened.Without this PR, a user could register their own requirements using the transformer API, which will work for most cases.
With this PR, a user can register requirements without having to make any assumptions.
This PR also starts the ball rolling for a dedicated AutoConfig Requirements API using declarative annotations.
Question
The above motivation hopefully justifies having a "post hook" API, however I've only included a "pre hook" for completeness.
Is there any reasom to include or not include a "pre hook" in this PR? If not, should "post hook" be renamed?
Related changes
This PR includes a number of small related changes to cleanup and modernize
GuiRegistry
's internal implementation. Namely:These changes could be dropped or moved to a different PR, however I felt it was simpler to include them here as they lay the foundation for this PR's core changes.
You may find it easier to review this PR one commit at a time.
A temporary "example" commit is also included, to aid in testing & reviewing. This commit should be dropped before merging (d789d6c).
Original description
Autoconfig requirement support will require operating with finished Config Entry GUIs in order to a) keep track of what GUIs have been created in a lookup table and b) register GUIs that have dependencies declared with a
RequirementManager
.This could be done internally by injecting additional code into
GuiRegistry
or by adding defaultGuiTransformers
, however the former seems short-sighted and the latter isn't very robust, seeing as there's no guarantee thegui
list won't change (or be entirely replaced) by another transformer running after us.IMO the best solution is a "post" event hook. The handler can be passed an
Unmodifiable List
, ensuring that the GUI objects the handler sees are the same ones that will actually be used.It seems silly to do this entirely internally, and strange to have a public "post" hook without a "pre" hook, so this PR follows the above to its logical conclusion and adds an experimental public API to register pre & post event hooks on
GuiRegistry
.I wasn't sure whether or not it made sense for event hooks to be subject to predicates (in the same way that registered
GuiProvider
andGuiTransformer
s are). This can be added easily if desired, but will increase the number of new experimental API methods onGuiRegistry
a lot...This PR is blocking Requirement support for autoconfig (as per #195 & #204).