-
-
Notifications
You must be signed in to change notification settings - Fork 55
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, take 3 or something #223
Merged
Merged
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
fixes CommandSourceStack#getServer being null during permission checks for arguments with custom suggestions
…oud commands with '/execute' on fabric
jpenilla
added
enhancement
New feature or request
implementation
Platform implementations
minecraft
labels
Mar 13, 2021
zml2008
commented
Mar 13, 2021
...aft/cloud-fabric/src/main/java/cloud/commandframework/fabric/FabricClientCommandManager.java
Show resolved
Hide resolved
broccolai
approved these changes
Mar 15, 2021
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.
LGTM, test mod seems to work from my limited fabric knowledge too
jpenilla
force-pushed
the
feature/fabric
branch
from
March 16, 2021 00:49
8d003d8
to
97bd549
Compare
jpenilla
approved these changes
Mar 17, 2021
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.
Ready for merge in my opinion, pending review from @Citymonstret and @zml2008
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.
Re-opening based on the previous PR
This is a basic implementation that can register commands, but doesn't have any fancy argument types. I'm hoping to add that eventually. I'll probably go about that by allowing argument parsers to delegate to Brigadier types, since that will be easier, and allow for easier integration with mods that provide their own Brigadier argument types.
Argument type support
This PR aims to implement all Vanilla argument types (besides the development-only ones), primarily through wrapping the Brigadier parsers. Current progress:
Exist in Cloud
UUIDArgumentType
(network:minecraft:uuid
)Directly mapped
ColorArgument
(network:minecraft:color
)CompoundTagArgument
(network:minecraft:nbt_compound_tag
)NbtTagArgument
(network:minecraft:nbt_tag
)NbtPathArgument
(network:minecraft:nbt_path
) todo: the actual implementation for this is an inner class of the brigadier type, does it make sense to wrap that?ScoreboardCriteriaArgument
(network:minecraft:objective_criteria
)ScoreboardOperationArgument
(network:minecraft:operation
)ParticleEffectArgument
(network:minecraft:particle
)AngleArgument
(network:minecraft:angle
) todo: the vanilla impl is logical server-only, but doesn't need to beAxisArgument
(network:minecraft:swizzle
)IdentifierArgument
(network:minecraft:resource_location
)EntityAnchorArgument
(network:minecraft:entity_anchor
)IntRangeArgument
(network:minecraft:int_range
)FloatRangeArgument
(network:minecraft:float_range
)ItemStackArgument
(network:minecraft:item_stack
) todo: this might want a wrapper -- it's not a full item stack, just the item type and NBT tagNeed wrapper type
These Brigadier parsers only return an intermediate type, and require a ServerCommandSource to be fully resolved. We need to evaluate which ones actually need a server, which can be made client-friendly. I expect everything that uses selectors will have to be server-only, but others can be client-friendly
These would also be bound to a ServerCommandSource sender type, or would need to provide a mapper function.
Using selectors
ScoreHolderArgument
(network:minecraft:score_holder
) -- single vs multiple, anything that can be on a scoreboard (all wildcarding is done based on selector matches)EntityArgument
(network:minecraft:entity
) -- supports entity/player, and single/multiple propertiesMessageArgument
(network:minecraft:message
) -- this is just a greedy string that resolves selectorsGameProfileArgument
(network:minecraft:game_profile
) - parses selectorsUsing relative positions
BlockPosArgument
(network:minecraft:block_pos
) - can be relativeColumnPosArgument
(network:minecraft:column_pos
) - can be relativeVec3dArgument
(network:minecraft:vec3
) - can be relativeVec2fArgument
(network:minecraft:vec2
) - can be relativeOthers/to be sorted
BlockPredicateArgument
(network:minecraft:block_predicate
)BlockStateArgument
(network:minecraft:block_state
)ItemPredicateArgument
(network:minecraft:item_predicate
)ScoreboardObjectiveArgument
(network:minecraft:objective
)RotationArgument
(network:minecraft:rotation
)ScoreboardSlotArgument
(network:minecraft:scoreboard_slot
)TeamArgument
(network:minecraft:team
)ItemSlotArgument
(network:minecraft:item_slot
)CommandFunctionArgument
(network:minecraft:function
)TimeArgument
(network:minecraft:time
) -- time in ticksCombined to one registry values argument
These are all specialized argument types that can be exposed as one single argument type in-API. We'll be able to send the appropriate argument type/suggestion provider to be used for completions though, falling back to querying the server.
EntityTypeArgument
(network:minecraft:entity_summon
)AVAILABLE_SOUNDS
suggestion providerALL_BIOMES
suggestion providerSUMMONABLE_ENTITIES
(all entities but player and fishing bobber)EnchantmentArgument
(network:minecraft:item_enchantment
) (needs to be re-implemented)ServerWorldArgument
(network:minecraft:dimension
)StatusEffectArgument
(network:minecraft:mob_effect
) (needs to be re-implemented)Doesn't exist as an argument type, but is a client-local completion provider
ALL_RECIPES
Entirely custom
Some parsers that might be useful for mod developers but aren't included in base Cloud:
RegistryEntryArgument
-- any entry in a registry, should support both static and dynamic registriesOther odds and ends