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

Add some custom command argument types #834

Merged
merged 9 commits into from
Jul 14, 2024

Conversation

Emirlol
Copy link
Collaborator

@Emirlol Emirlol commented Jul 11, 2024

Added an EggTypeArgumentType for the egg finder's location sharing command (not that anyone would use it manually, but I just wanted to) and ClientBlockPosArgumentType because the one minecraft has (BlockPosArgumentType) requires a ServerCommandSource, and since these are client-side commands we don't have an instance of ServerCommandSource. That led to this monstrosity:

context.getArgument("pos", PosArgument.class).toAbsoluteBlockPos(new ServerCommandSource(null, context.getSource().getPosition(), context.getSource().getRotation(), null, 0, null, null, null, null)

So now you can do this:

context.getArgument("pos", ClientPosArgument.class).toAbsoluteBlockPos(context.getSource())

or this for short:

ClientBlockPosArgumentType.getBlockPos(context, "pos")

I've updated the usages of BlockPosArgumentType to use the custom one instead. I've also noticed that the /skyblocker diana clearGriffinBurrow pos command wasn't working due to incorrect class given as the argument to the context.getArgument method, and fixed that along the way. Additionally, there are some minor code cleanups here and there.

@LifeIsAParadox LifeIsAParadox added the reviews needed This PR needs reviews label Jul 11, 2024
@Emirlol
Copy link
Collaborator Author

Emirlol commented Jul 11, 2024

As a side note, I've tested this on the egg finder command and the clearGriffinBurrow command. Perhaps not exhaustively, but it's better than nothing.

Also these argument types aren't registered to the map in ArgumentTypes class because I thought it was unnecessary, and it would add more stuff to maintain over new version changes as they need serializers and all that. It seems like that's only used for S2C command packets and dumping commands in a json, which isn't all that important. Though, if anyone knows a reason why they should be registered, let me know.

@AzureAaron AzureAaron added the bug Something isn't working label Jul 14, 2024
@LifeIsAParadox LifeIsAParadox added merge me please Pull requests that are ready to merge and removed reviews needed This PR needs reviews labels Jul 14, 2024
@kevinthegreat1 kevinthegreat1 merged commit 1d1cfea into SkyblockerMod:master Jul 14, 2024
1 check passed
@LifeIsAParadox LifeIsAParadox removed the merge me please Pull requests that are ready to merge label Jul 14, 2024
kevinthegreat1 added a commit that referenced this pull request Jul 14, 2024
Add some custom command argument types

(cherry picked from commit 1d1cfea)
@Emirlol Emirlol deleted the command-changes branch July 14, 2024 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants