Contextual API - #972
Merged
Merged
Conversation
📖 Documentation previewPreview: https://pr-972-controlify.isxander.workers.dev Updated for |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 114 out of 114 changed files in this pull request and generated 12 comments.
Suppressed comments (5)
src/main/java/dev/isxander/controlify/contextual/RuleSetManager.java:144
- Same logging issue here: the caught exception is currently being used to fill the '{}' placeholder, so you lose the stack trace in logs. Put placeholder args first and pass the Throwable last.
src/main/java/dev/isxander/controlify/contextual/ContextualDomainImpl.java:129 - The exception is being passed as a formatting parameter, so the stack trace is dropped. Swap the argument order so the domain id fills the '{}' placeholder and the Throwable is last.
src/main/java/dev/isxander/controlify/contextual/ContextualDomainImpl.java:159 - Same logging issue: the Throwable is currently treated as a message argument, so you won't get a stack trace in logs. Pass the domain id first and the exception last.
src/main/java/dev/isxander/controlify/contextual/ContextualDomainImpl.java:186 - This warn log also passes the exception as a placeholder argument, which drops the stack trace. Put the fact/domain arguments first and the Throwable last.
src/main/java/dev/isxander/controlify/driver/sdl/SDLCommonDriver.java:195 - Typo in exception message: "require it's already closed" should be "when it's already closed".
Comment on lines
+104
to
+110
| CUtil.LOGGER.error( | ||
| "Failed to load contextual {} rules for domain '{}' from pack '{}'; skipping this layer", | ||
| e, | ||
| this.directory, | ||
| domainId, | ||
| ruleSetResource.sourcePackId() | ||
| ); |
Comment on lines
+103
to
+108
| CUtil.LOGGER.error( | ||
| "Failed to load contextual facts for domain '{}' from pack '{}'; skipping this layer", | ||
| e, | ||
| this.id, | ||
| resource.sourcePackId() | ||
| ); |
This file contains hidden or 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
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.
Rewritten APIs for Trigger Effect and Guides, now under one unified contextual domain / fact / rule system.
Using mostly-vanilla predicates, facts can be defined in resource packs, querying game-state such as held items, hit result, etc.
The guides are now highly contextual, for example, when looking at a villager L2 would be annotated by "Trade" instead of the default "Interact".
Because facts can be data-driven, third party mods and servers can integrate with Controlify without code.