bytzo / sessility

AFK utilities for the server: Keeps track of sessile creatures, more often known as "AFK players".
https://modrinth.com/mod/sessility
MIT License
4 stars 4 forks source link

Player is still considered afk when moving around #3

Closed LookItsAndy closed 1 year ago

LookItsAndy commented 2 years ago

Hi, I have been playing around with the mod and realized that when a player is AFK they can freely jump and walk around while still labeled as AFK. The motile message is triggered when the player has either: crouched, sprinted, attacked, right clicked, put items in their offhand, and moved or dropped items in the inventory.

bytzo commented 2 years ago

This is actually intended behavior that prevents AFK pools and the like from working to bypass the sessile status.

Sessility uses the same code that vanilla uses to kick idle players (player-idle-timeout in server.properties), which will only consider players active when they perform explicit actions like sprinting or placing. While actions like moving are also explicitly called by the player, they can also be triggered by things in the world like other entities or water pushing the player. If things in the world could make a player active, mobs pushing a still player would incorrectly make them motile.

Would you find it useful to have an option for more sensitive active player detection, at the cost of some false positives?

LookItsAndy commented 2 years ago

I don't think I will find an option like that useful however would it be possible to make it so that jumping will make a player active?

bytzo commented 2 years ago

Unfortunately it doesn't look like there is a way for the server to reliably detect if a player jumps. The way the mod currently behaves is likely as sensitive as I can make it without risking false positives.

donington commented 1 year ago

I've looked a bit into this idea, and It brought me to:

hook world.entity.player.Player.awardStat(Stat<?> $$0, int $$1) for mixin
requires: net.minecraft.stat
          net.minecraft.stats

mixin target: public void awardStat(Stat<?> $$0, int $$1)

Then perform case statement against what matters.

The specific request of jumping is net.minecraft.stats.JUMP, but it could extend to movement with WALK_ONE_CM, CROUCH_ONE_CM, SPRINT_ONE_CM, etc.

Not sure if it would be more performant to make some kind of absolute whitelist of events that should flag motile, or a two pass strategy ignoring some and vetting some at a glance. Needs more thought. There's a lot of potential triggers in the stats that would require some sifting that I can't do in one sitting. That being said, it seems a lot easier than hooking a bunch of methods with a bunch of mixins if you can pull it off with one for a lot of result.

Cool mod! Love to help this improve if I have the time for it.

bytzo commented 1 year ago

Wow, that's actually a great solution for detecting jumps!

Before I was looking at it from the client's perspective–when the player presses the jump key, what gets executed? I saw that it only affected the player's vertical velocity, which didn't make it easy to determine when a player was jumping. But, looking at it from the server's perspective–specifically in advancements like you said–the server does actually have a way to detect jumping! In net.minecraft.server.network.ServerGamePacketListenerImpl.handleMovePlayer(), the net.minecraft.world.entity.Player.jumpFromGround() method is called when the vertical velocity is positive and the player was previously on the ground and is now in the air.

But rather than writing a Mixin here and in many other places, your idea is much more robust. There are tons of possibilities of detecting player actions through advancements, and like you said we would only need to maintain a whitelist/blacklist of triggers to count towards a player being motile. Furthermore, we could even make this configurable by the user!

Cool mod! Love to help this improve if I have the time for it.

I would love to work on this, but I have been personally busy in the past few months so it may take some time for me to get to. So if you want to, I'd gladly accept a contribution with this feature!

donington commented 1 year ago

I'm probably going to take a stab at this this weekend. Currently evaulating the easiest way to catch multiple types of events with little effort.

In terms of configuration, were you thinking being able to enable/disable specific types of actions (eg, jumping, movement, interaction with stations is even possible I believe) or just enabling and disabling the use of achievements and leaving the default behaviour?

bytzo commented 1 year ago

I was imagining a list of stat keys that would trigger motility on award/progression, but the current config format doesn't support lists. I made a poor decision to extend the vanilla dedicated server properties format. So for now, a boolean value of whether or not statistics should be used to update sessility could be used, while the specific stats that are used are hard-coded.

bytzo commented 1 year ago

Thanks to the contributions from @donington, Sessility 0.4 can now detect player rotation and jumping when determining motility!