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

Causing player selector in Minecraft command #874

Open
nguyenquyhy opened this issue Aug 6, 2016 · 25 comments
Open

Causing player selector in Minecraft command #874

nguyenquyhy opened this issue Aug 6, 2016 · 25 comments

Comments

@nguyenquyhy
Copy link

nguyenquyhy commented Aug 6, 2016

Currently Minecraft supports several target selectors such as @p, @r, @a. I wonder if Sponge can add a new selector to target the cause of the command.

For example, if I put a button on a command block, and set the command block to /tp @c 100 100 100, the command block will teleport the exact player that pressed the button. At the moment, the best I can do is to set tp @p 100 100 100, which is quite problematic in crowded area.

If Sponge can really override the player target mechanism of Minecraft, the same @c may also be used with other plugins such as Command Sign.

@octylFractal
Copy link
Contributor

I can look into writing API and implementation if the Sponge developers think it would be worthwhile.

@ryantheleach
Copy link
Contributor

Is it possible to implement this in a plugin?

@nguyenquyhy
Copy link
Author

An API that exposes Minecraft target resolution (for those new @x) would be great. However, since cause tracking is supported by Sponge, I think at least a cause target selector should be something built-in.

@gabizou
Copy link
Member

gabizou commented Aug 6, 2016

@kenzierocks It is within the realm of possibility, but I'm curious how it would be handled from the API standpoint. I'd imagine it would still be a special CommandElement, and @zml2008 did express some interest in such a thing. Implementation wise, it would be really simple with the CauseTracker system in place for both 1.8.9 and 1.10.2

@nguyenquyhy
Copy link
Author

I think on the API side, a new event or changing existing event might not be necessary. However, internal Sponge's resolution for player parameters (those that declared during command registration) can be changed to make use of this @c feature as well. Moreover, it would be great to add a function like resolveEntityTarget() which accepts those @ string.

@octylFractal
Copy link
Contributor

The SelectorResolver does that, but it has a ton of bugs that need fixing (I have an open PR for this).

@JBYoshi
Copy link
Member

JBYoshi commented Aug 11, 2016

@nguyenquyhy I added support for that some time ago in SpongePowered/SpongeAPI#780, but it does use SelectorResolver, so it will be affected by any bugs in that (see kashike's comment).

@nguyenquyhy
Copy link
Author

@JBYoshi it seems the PR you referenced is closed. Is it merged somewhere else in Sponge?

@JBYoshi
Copy link
Member

JBYoshi commented Aug 17, 2016

It was rebased into the master branch in SpongePowered/SpongeAPI@3476052.

@nguyenquyhy
Copy link
Author

I don't really get it. So how can I target causing player now? E.g. a player clicks a button on a command block and how can the command block do /tell ... to that exact player?

@JBYoshi
Copy link
Member

JBYoshi commented Aug 19, 2016

I meant that added support for custom selectors in commands. It does not add support for a cause selector.

@ghost
Copy link

ghost commented Aug 21, 2016

How would it works if there is a button and a complex redstone circuit with gate, ...?
I'm not sure it would be possible.

@ryantheleach
Copy link
Contributor

I too thought it wouldn't be possible at the start, then when the Sponge cause system was introduced I was pleasantly surprised.

At this stage it's just creating a selector that responds to causes, the cause system already exists, and works.

@nguyenquyhy
Copy link
Author

Yes that is why I give the suggestion. It seems to me that Sponge already has all neccessary facility to implement a cause selector (Selector resolver + Cause tracking). @c can become a new built-in selector for all commands that use selector resolver.

@nguyenquyhy
Copy link
Author

@advancid since the feature relies on cause tracker, it will have all limitation/ability of cause tracker.

I agree that this feature may not work accurately in complex redstone mechanics when the cause is not obvious. However, I think for most of the cases, the cause of an activation is quite obvious, and the lack of cause selector force server owner to employ complex mechanism to make sure they target the correct player. Moreover, with plugins, there is also fewer needs for server owners to build complex redstone mechanics for complex functions.

@ryantheleach
Copy link
Contributor

ryantheleach commented Aug 22, 2016

The only thing I don't necessarily agree with is using the prefix @ for Sponge-custom selectors, as it's plausible that minecraft introducing additional selectors would also use the @ prefix.

It's a minor gripe, but I could see it being a future migration pain point if @c were ever to be used by minecraft itself.

@octylFractal
Copy link
Contributor

Would it be too much of a pain to use @sponge:c, identifying like items and blocks do?

@nguyenquyhy
Copy link
Author

For me, @c or @sponge:c or anything else is ok as long as it is implemented.

@ghost
Copy link

ghost commented Aug 23, 2016

Sponge is "A community-driven open source Minecraft modding platform.", I don't think this should be imlemented in Sponge it self but maybe in a plugin.

@nguyenquyhy
Copy link
Author

I believe Sponge already has all facility to implement the function. In my opinion, having a another plugin just to merge two built-in Sponge functions feel a bit overkill.
I am aware that there can be compatibility issue when Minecraft moves forward. That's why I really hope to hear from Sponge team to know if they will go ahead with the implementation or I should build something on my own. This has been here for more than 2 weeks already.

@nguyenquyhy
Copy link
Author

Is there any update on this?

@mattysweeps
Copy link
Contributor

I also would be interested in seeing this added to Sponge by API 5.

@ghost
Copy link

ghost commented Oct 8, 2016

If this was implemented, I think it would not be in the API since it would make no sense, the API is for plugins.

@ST-DDT
Copy link
Member

ST-DDT commented Oct 8, 2016

@advancid It might be a useful default for some commands.

Example:
/tpto someplace (Teleport executor)
/tpto someplace @a (Teleport all)

@mattysweeps
Copy link
Contributor

Sorry to wake an old issue. Does anyone still want this?

Sponge Causes have come a long way since this was submitted. Does anyone have a better idea of whether or not it is feasible? Or a better idea whether or not it belongs as a part of Sponge?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants