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

Feature/command selectors #298

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

DarkEyeDragon
Copy link
Contributor

Adds command selector handling to BukkitCommandContexts#getOnlinePlayer However i'm not sure how to deal with @A or selectors that return more than one entity. So in this draft i just return the first entry. However this isn't an optimal solution, and also currently fails if there are no players

//Check if mc version is higher than 1.13.1 (added in 1.13.2)
if ((manager.mcMinorVersion > 13) || (manager.mcMinorVersion == 13 && manager.mcPatchVersion > 1)) {
try {
Entity entity = Bukkit.selectEntities(issuer.getIssuer(), lookup).get(0);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rather than blind look up 0 relying on errors, iterate and return on first player match instead, and lack of finding should fall through to the lines below.

And we can only run this selectEntities if the input starts with @

@@ -241,6 +241,20 @@ OnlinePlayer getOnlinePlayer(BukkitCommandIssuer issuer, String lookup, boolean
Player player = ACFBukkitUtil.findPlayerSmart(issuer, lookup);
//noinspection Duplicates
if (player == null) {
if (issuer.getManager() instanceof BukkitCommandManager) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is already a bukkit context, this isn't really needed.

this.manager should already exists.

Copy link
Contributor Author

@DarkEyeDragon DarkEyeDragon Oct 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this.manager returns a CommandManager which does not have the version fields, however, i should probably be able to cast that safely right?

Iterate over the entitylist properly now
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants