Add rotation to tp command - #3413
Conversation
|
Hmm, I don't really see any large batches of commits all working on the same issue in the commit-history. |
The pr commits are squashed on merge automatically |
2dfc5ff to
2c4991e
Compare
Wunka
left a comment
There was a problem hiding this comment.
Else this looks really good. But please rename your pr to something more meaningfull like: "Add rotation to tp command"
Wunka
left a comment
There was a problem hiding this comment.
Seems good to me. As a lot of code is pretty much the same to other code I put you into easy to review. Sorry also that you got hit by my command rewrite pr.
No problem, it wasn't that hard to fix my PR. Always making sure never to interfere with anything isn't possible anyways. |
IntegratedQuantum
left a comment
There was a problem hiding this comment.
I think this should wait until after #2414 so we don't need to introduce yet another needless network protocol for random command data.
I think I have the race condition fixed now, as for the issue of network protocols; Besides that I am not sure why you are opposed to this additional UpdateType. It may only be used by one command, but that command is useful and can only be expected to become more useful as Cubyz obtains more content (like command blocks for example) and gains more popularity and modded servers may in the future want to set a players rotation when teleporting into an area (like spawn or a lobby of sorts) as well (imagine if hypixel gave you a random yaw and pitch every time you entered the lobby) and the most significant bloat added by it is the camera mutex, which might have been needed anyways depending on how the clientside implementation would/will go. Again, I can bundle the setRotation packet into the teleport one. The mutex would still have to stay though. So that was my opinion and the arguments for it. I hope I convinced you that some method for the server to set player rotation is worth it. |
|
Yeah, right that is a good point. But it still should not be a random network command (which inherintly are not synchronized properly which you already discovered as a race condition). Instead you should use a SyncOperation in sync.zig (see also #1124).
It would not be needed, since the sync system already has its own mutex, and in the future will handle synchronization by putting everything onto the render thread. |
yaw and pitch parameters can be relative or absolute New UpdateType added to update player rotation Player rotation update packet is seperate from teleport packet to prevent repeated automated teleportations from messing with camera rotations. Might be useful in cases where modded servers want to prevent players from walking into a certain area or moving entirely. Update tp.zig Don't send a teleport packet if the target is already at the appropriate coordinates. It just seems right to not send a teleport packet if you are only changing the player rotation since the same is done the other way around as well. For consistency's sake. Is only intended to stop teleport packet sending in case of `/tp ~ ~ ~ <yaw> <pitch>`, but works in case of `/tp ~ ~ ~` as well. Does nothing to stop packet sending in case of something like `/tp 0 ~ ~`x2. Adapt to command source changes Adapt to command source changes in c4cf9ec Make requested changes
Adds optional yaw and pitch parameters to the /tp command.
Yaw and pitch are expected to be supplied in degrees as that is the unit most users can be expected to be familiar with and will automatically be converted to radians internally.
Yaw and pitch can be relative:
/tp @0 ~ ~ ~ ~ ~10Closes #2403