-
-
Notifications
You must be signed in to change notification settings - Fork 992
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 log config option + socialspy for /tpr #4617
Conversation
These changes feel a bit much to me for how niche of a use case this is. It's already possible to see a player's location in-game using commands such as If @Bobcat00 wants to they could also add a listener for UserRandomTeleportEvent to set up custom logging / monitoring. |
I thought about adding this as a debug print but I feel like it's something that should be allowed outside of spamming the console full of the other debug prints. |
I can understand that argument but it just feels a bit odd to me that this would justify adding a special case to social spy, which has no other special cases outside of messaging commands for which it was primarily designed to be used with. In addition, with the same line of reasoning it would make a whole lot of sense to add logging of locations for all other teleportation commands as well, rather than just this one. I don't see how the use case is much different from wanting to know where a player went when they executed a similar command like |
I'm not opposed to adding those special cases for socialspy inside of #3801 |
With /tpa, /tpahere, and /tpaccept, you have a player issuing a command at the destination. So that can be logged by a logging plugin like Prism, allowing admins to determine the destination after the fact. With /tpr, there's no player issuing a command at the destination for it to be logged. Also, the logging could be useful to server owners who want to debug their /tpr setup. Maybe people are ending up in places that he didn't intend. (It's random!) The logging is how he could figure out what's wrong. But anyway, that's just me. If you guys hate it, that's OK. I'd just add a one-line output to the log after the teleport, and leave it at that. No SS stuff. |
I should have been more specific but I'm not sure if this really belongs in social spy at all, although I guess that might be a bit subjective. I think it'd be worth having another opinion on this, but I think a different solution (such as the notify permissions Bobcat mentioned) would probably make more sense.
It doesn't make sense for Essentials to be designed around what features other plugins do or don't have. It sounds like the real issue here is that you want to see the location later, not in real time (e.g. with
+1 as to why it might make sense to add to ess debug, but personally I've usually just used the F3 to debug it.
I don't hate it but I agree that it probably doesn't need to be part of social spy. |
I have no real objections to adding this, but I'm also not convinced that this has that much purpose. I'm open to other people's opinion on whether this is worth adding or not, but I'm dropping this from the 2.19.1 milestone until we have a consensus. |
@@ -39,6 +39,10 @@ public void reloadConfig() { | |||
cachedLocations.clear(); | |||
} | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make more grammatical sense to have this as “shouldLogTprs”?
Adding to 2.20 milestone pending a decision on whether this is considered a worthwhile addition. |
Well here's a use case mentioned by @pop4959(!) in #3499:
Since the TPR destination is not logged, there's no reliable way for the server owners to know that players are constantly teleporting to the edge of the world border instead of an actual random location. Logging each TPR destination would have helped in that case, and it's certainly possible it could help in investigating future problems. Although as I said above, I'd just do a one-line write to the log file instead of adding it to Social Spy, where it arguably doesn't belong. |
Adds a config option to
tpr.yml
to log teleport locations in the consoleAdds a custom social spy handler for /tpr which gives the destination location.
Att #4613