Standardize executor input type for locations
gerzytet opened this issue · 12 comments
For executors that deal with location, some use Location instances while other use x y and z arguments, causing needless conversions and inconsistency.
Location values usually result in more concise code, but x y z arguments are better understood and more widely supported by other executors and placeholders. In the interest of consistency, executors that support location instances should deprecate that support in favor of xyz.
The following executors would be affected:
#DROPITEM
#FALLINGBLOCK
#SETBLOCK
#TP
No placeholders support Location instances
@wysohn
Can we drop this issue? All of the executors I mentioned support xyz in one form or another, TP just wasn’t properly documented, and DROPITEM is due for a full replacement anyway.
If you want to avoid confusion about location instances, the best thing you can do is remove the executor documentation for them along with the location-saving command. That way, locations are purely an advanced feature
@gerzytet
I think we can close it since no one is working on it anyway.
As we can save Location instance in the global variable, it sometimes is handy to use Location instance itself directly
For example, I could save Location in-game with /trg var Location to save the location where I am currently standing. /trg var Location spawnLocation
Then, all I have to do is #TP {"spawnLocation"}
Though, it's true that it's hard to grasp how it works
If we want to keep this feature useful for executors that don’t support support location instances, the command should save the x, y, and z co-ordinates separately.
#TP {"spawn.x"}, {"spawn.y"}, {"spawn.z"}
It’s verbose, but it’s the easiest way
There could also be an “unpacking” operator to do this for you
#TP ...{"spawn"}
But that seems like too much work just to support a single use case
I think the best way would just be to make that command copy the co-ordinates to your clipboard so you can paste them into your code
Unpacking is automatically done by the ConficurationSerialization, and that's a good part of Executors accepting Location instance directly. You can directly save Location into the global variable and load it from the global variable without any effort, since the Location is ConfigurationSerializable.
Not only the {"spawnLocation"} will be fully functional Location instance but also it automatically serialize/deserialize Location instance as needed.
This is same for ItemStack, and it's very useful since ConfigurationSerialization also supports the item metadata, which would be very painful to implement the unpacking operator ourselves.
The problem here isn’t that working with Locations or storing them is difficult.
The problem is that there’s a divide between what supports locations and what doesn’t.
Since xyz and Location instances represent the same thing, this issue is about deciding which one we want to make consistent across all the executors.
Right now, it’s looking like xyz will be the path of least resistance.
The main deal is that if you have a stored location and you want to use it in an xyz trigger, or the other way around, you have to do a big conversion.
The other problem is to manipulate a location, you have to read javadocs.
The only argument you've made for keeping around locations is the convenient saving abilities.
We can still have that; it would remain useful for java methods that take locations.
As for xyz, saving can be emulated through the clipboard (which, by the way, has already been somewhat implemented through the F3 + C hotkey, which copies your co-ordinates)
However, the main issue isn't save-ability. It's that while locations can be better in some situations (interfacing with java, keeping the data in 1 object), many users won't really understand what the difference is between the 2 forms. I think they're worth keeping around, but we at least need to ensure any programming task using trg builtins can be done using xyz just as easily as locations.
Minecraft's own command system has proven its success in providing an intuitive programming interface with only xyz. Why don't we do the same?
In my opinion, both use cases are useful.
What's the main deal of some Executors supporting Location instance and some don't?
We have to put ourselves in the users' shoes, not how amazing this plugin will look.
I can't really see why it would be practical to just support either Location instance or x,y,z
Though it's true that it would be nice if all the Executors support both use cases, yet that will take quite an effort, for small use cases.
Now that's more clear. I agree on your idea.
So we can keep our Executors as simple as possible for better accessibility, and those who understand TR a bit better can directly call methods (like player.teleport({"spawnLocation"}))
What would be the format for F3 + C though?
really, all the executors I mentioned have full support for xyz anyway (except for #DROPITEM, but dropitem has bigger problems)
We just have to decide what to do with the existing location support.
keep, undocument, deprecate, or remove?
I think we should keep all the Java instance-based arguments out of Executors and Placeholders. So I would vote for remove.
People who rely on Executor or Placeholder usually do not understand the difference between the literal values and the instance values, so if they do understand the differences, they can use advanced method, like directly executing the methods of Bukkit API.