Essentials icon indicating copy to clipboard operation
Essentials copied to clipboard

Add log config option + socialspy for /tpr

Open JRoy opened this issue 4 years ago • 9 comments

Adds a config option to tpr.yml to log teleport locations in the console

log-tprs: true

Adds a custom social spy handler for /tpr which gives the destination location.

Att #4613

JRoy avatar Nov 06 '21 14:11 JRoy

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 /whois after the player teleports. For debugging purposes, I think it would be sufficient if the exact location was simply logged to the /ess debug output.

If @Bobcat00 wants to they could also add a listener for UserRandomTeleportEvent to set up custom logging / monitoring.

pop4959 avatar Nov 06 '21 20:11 pop4959

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 /whois after the player teleports. For debugging purposes, I think it would be sufficient if the exact location was simply logged to the /ess debug output.

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.

JRoy avatar Nov 06 '21 20:11 JRoy

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 /home, /tpaccept, or so on.

pop4959 avatar Nov 06 '21 21:11 pop4959

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 /home, /tpaccept, or so on.

I'm not opposed to adding those special cases for socialspy inside of #3801

JRoy avatar Nov 06 '21 21:11 JRoy

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 /home, /tpaccept, or so on.

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.

Bobcat00 avatar Nov 06 '21 21:11 Bobcat00

I'm not opposed to adding those special cases for socialspy inside of #3801

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.

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.

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 /whois, social spy, or teleporting to the player to see their location). In that case it probably does make sense to do custom logging to file, or hook into Prism if it supports custom logging, using the UserRandomTeleportEvent.

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.

+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.

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 don't hate it but I agree that it probably doesn't need to be part of social spy.

pop4959 avatar Nov 06 '21 22:11 pop4959

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.

mdcfe avatar Nov 20 '21 14:11 mdcfe

Adding to 2.20 milestone pending a decision on whether this is considered a worthwhile addition.

mdcfe avatar Feb 28 '22 23:02 mdcfe

Well here's a use case mentioned by @pop4959(!) in #3499:

If players are constantly teleporting to the edge of the world border instead of an actual random location, it's a huge hint to the server owner that they have something set up incorrectly.

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.

Bobcat00 avatar Mar 01 '22 12:03 Bobcat00