smogon / pokemon-showdown

Pokémon battle simulator.
https://pokemonshowdown.com
MIT License
4.77k stars 2.79k forks source link

AFK/Status Follow-up #5590

Open scheibo opened 5 years ago

scheibo commented 5 years ago

Proposal

The initial launch of AFK/statuses has revealed some design inconsistencies and opportunities for improvements.

*This is up for debate. PMs currently work this way with the Away state - users can still chat and be colored in the chat but show up as gray in the userlist. This proposal would extend this to regular chat rooms as well.

Tweaks

Moderation

*Alternatively, Global Voice and above.

UI


@Zarel @bumbadadabum @HoeenCoder @whalemer

Zarel commented 5 years ago

A user can be in one of 4 states on the server: Online, Idle, Away, Offline. A user becomes Idle after a period of inactivity as determined by the server and can return from the Idle state by performing any action whatsoever. The Away state only occurs manually, through use of the /away command (and its aliases: /afk, /brb, /busy), and a user only leaves this state through the use of /back. No other action in the Away state causes a user to leave Away (including joining/leaving a room or chatting in a chat room*, though these would cause you to leave the Idle state).

I dislike making it easier for people to use the "Away" status to lie about being away. At least rename "Away" to "Busy". /afk and /brb can be used to manually set the "Idle" status (which will still be automatically cleared).

  • The server protocol will reserve the first character after the @ to be a symbol denoting the state of the user, which will currently either be '!' for Idle/Away and ' ' for Online, followed by the user's status message. The user's status is either inferred from the command used to mark the user as Away (/brb -> !BRB) or is whatever the user specified (/away eating -> !eating).

Reserving a space character, while possible, seems a bit subtle to me. I don't have a problem with /status !BRB or something as a way to manually set an away status. Could be useful for bots.

Statuses should not be filtered using the name filter (which disallows things like commas), and should probably have a more lenient filter based on the chat filter which pretty much only excludes '|' for protocol reasons, banwords/spoilers, and anything which could be confused with a rank (eg. /(global|room|upper|senior)\s*(staff|admin|administrator|owner|operator|mod|moderator|driver|voice|creator)/gi)

I did this a while ago: 4d80d4b1a93

And I already talked about this in https://github.com/Zarel/Pokemon-Showdown/pull/5587#issuecomment-508640635

This is up for debate. PMs currently work this way with the Away state - users can still chat and be colored in the chat but show up as gray in the userlist. This proposal would extend this to regular chat rooms as well.

PMs should probably also clear the Idle state.

"Someone is using an offensive username or pokemon nickname" -> "Someone is using an offensive username, status or pokemon nickname" (needs translation support?)

Don't we already distinguish these?

https://github.com/Zarel/Pokemon-Showdown/commit/4d80d4b1a934ff9c77831f880489d21d8c0ffa65#diff-1b8e46b915ffe29285ad921677061fc9R275

Policy TBD: Allow Room Staff* to /clearstatus of users in their room. While /clearstatus affects a user globally (clears their status in all rooms), it is much easier for users to recover from than being forcerenamed as status is not tied to to identity. Making it much easier for room auth to clear offensive statuses empowers them and means the burden of policing these statuses is more spread out and does not fall exclusively on global staff. If a user continues to require their status to be cleared, Global Staff can namelock/lock the user to prevent them from continuing to abuse the feature, but we would imagine this would occur less frequently.

Probably not a bad idea, but also probably should be restricted to public rooms.

/showjoins can include status changes (perhaps only when a custom message is used, to reduce spamminess), or perhaps a separate /showstatuses command can be added if we wish to not alter the existing behavior of /showjoins.

I'd prefer for it to be a part of showjoins. Maybe custom message only.

  • Should we display the '(Tag)' as a colored status orb/icon instead a la Discord/MSN ?

That's probably a good idea.

  • Can we better differentiate statuses from rank/group display such that its more difficult to confuse the two?

Rank/group could be pills? Like how Discord does them.

  • Support setting status and Away/Online state from the Options menu instead of purely through commands.

Would make more sense to have it settable via usercard.

  • Idle timeout should be configurable (/idletimer 900 = become Idle after 15 minutes instead of 30/60minutes). This should be a persistent setting, and also settable in the Options menu. /idletimer 0 = disable. Policy TBD: Should Global Staff be allowed to lengthen/disable their Idle timer (similar to not being able to disable PMs indefinitely).

Seems like excessive configuration. Ideally we let it be 15 minutes, and give another solution for the "still online but PS's tab isn't focused" situation.

  • Include a count of Away|Idle users in the userlist (eg. "29 users" -> "29 users (5 away)")

I don't think any other chat client does this. Seems unnecessarily complicated.

Asheviere commented 5 years ago

I agree with the comments Zarel made, except I really think splitting away into two hard types is overcomplicating the issue. While I did add some extra things that remove the automatic idle, I saw this mostly as a courtesy. If we're making a busy status type (which we could) it should focus on blocking PMs/challenges/highlights/tournaments and that sort of thing, and not be a different kind of away.

Regarding making pms should you as back, this was how I had it originally but pre wanted to be a lurker and still PM people.

Asheviere commented 5 years ago

Actually I guess I suppose it's fine to have a specified difference between manual and auto away. But if we do that, I would also want to make the busy status behave like the afk statuses with a enum character for consistency. That was we could also add checks for users having a busy status elsewhere to suppress some notifications and messages.

Asheviere commented 5 years ago

Maybe Busy -> Do not Disturb to make it clear that one should actually block messages (which was the intent)

thejetou commented 5 years ago

Maybe Busy -> Do not Disturb to make it clear that one should actually block messages (which was the intent)

Maybe Do not disturb / Busy should disable notifications (highlights, tournaments, PMs, etc..) instead of blocking them?

Zarel commented 5 years ago

DND blocking challenges I can understand, but I'm not sure if it should block PMs. Discord's doesn't, after all.

Zarel commented 5 years ago

I like JetOU's idea of disabling notifications.

Asheviere commented 5 years ago

Yeah personally I would say disable notifs on top of blocking challenges. What do you all think?

scheibo commented 5 years ago

I dislike making it easier for people to use the "Away" status to lie about being away. At least rename "Away" to "Busy".

Perhaps 'Away' is the wrong name for the state. 'Unavailable' /'Busy' / 'Away' / 'Do Not Disturb' - I don't really care what we call it, the concept we're going for is: "This user has explicitly indicated theyre perhaps not watching chat/going to respond in a prompt manner, maybe try back later or find someone else"

/afk and /brb can be used to manually set the "Idle" status (which will still be automatically cleared).

Possibly? One advantage people like about '(Idle)' status is that it is the one status that definitely guarantees a user was not active for a fixed time. I'm not super opinionated that we need to preserve that, just something to keep in mind.

The point is - I think we want two 'unavailable' states - mark me back immediately and mark me back only when I say so.

Reserving a space character, while possible, seems a bit subtle to me. I don't have a problem with /status !BRB or something as a way to manually set an away status. Could be useful for bots.

Its for symmetry with group/ranks. And if we change the filters to exclude symbols from the first character of statuses, you can still have bots use /status !BRB or /stats BRB and be able to infer the meaning correctly.

Statuses should not be filtered using the name filter (which disallows things like commas), and should probably have a more lenient filter based on the chat filter which pretty much only excludes '|' for protocol reasons, banwords/spoilers, and anything which could be confused with a rank (eg. /(global|room|upper|senior)\s*(staff|admin|administrator|owner|operator|mod|moderator|driver|voice|creator)/gi)

I did this a while ago: 4d80d4b

And I already talked about this in #5587 (comment)

Um, yeah. This point was motivated by those comments...

This isn't complete though, we still want a rank filter.

PMs should probably also clear the Idle state.

'Idle' sure (does it not already?). 'Unavailable', I'd rather not.

"Someone is using an offensive username or pokemon nickname" -> "Someone is using an offensive username, status or pokemon nickname" (needs translation support?)

Don't we already distinguish these?

This is for language around reporting a name (help tickets). We haven't updated that.

Policy TBD: Allow Room Staff* to /clearstatus of users in their room. Probably not a bad idea, but also probably should be restricted to public rooms.

Yup, SG. We discussed this briefly over VC with some US members but this need to be vetted more broadly.

I'd prefer for it to be a part of showjoins. Maybe custom message only.

Should showjoins also include renames? Should they all be condensed on one line?

  • Should we display the '(Tag)' as a colored status orb/icon instead a la Discord/MSN ?

That's probably a good idea.

Any thoughts around colors or icons?

Rank/group could be pills? Like how Discord does them.

Doh. Beauty! <3 Will implement.

  • Support setting status and Away/Online state from the Options menu instead of purely through commands.

Would make more sense to have it settable via usercard.

Good point. I didn't realize you could set avatar that way, I agree the text in the usercard should be content-editable as well.

  • Idle timeout should be configurable (/idletimer 900 = become Idle after 15 minutes instead of 30/60minutes). This should be a persistent setting, and also settable in the Options menu. /idletimer 0 = disable. Policy TBD: Should Global Staff be allowed to lengthen/disable their Idle timer (similar to not being able to disable PMs indefinitely).

Seems like excessive configuration. Ideally we let it be 15 minutes,

Discord is 10 minutes (600s) FWIW. I'd support 15. Bumba was the one who wanted 1hr+ before I talked him down to 30/60.

and give another solution for the "still online but PS's tab isn't focused" situation.

Such as?

  • Include a count of Away|Idle users in the userlist (eg. "29 users" -> "29 users (5 away)")

I don't think any other chat client does this. Seems unnecessarily complicated.

OK. It was a suggestion that came up in chat, doesn't seem too important, just wanted to be thorough.

I agree with the comments Zarel made, except I really think splitting away into two hard types is overcomplicating the issue.

Right now the types are amorphous and don't play well with each other. I think we need to decide what we want to achieve and better tailor our statuses to that.

While I did add some extra things that remove the automatic idle, I saw this mostly as a courtesy. If we're making a busy status type (which we could) it should focus on blocking PMs/challenges/highlights/tournaments and that sort of thing, and not be a different kind of away.

Regarding making pms should you as back, this was how I had it originally but pre wanted to be a lurker and still PM people.

Many devs want this. If we have distinction between auto and manual idle this no longer becomes an issue - auto idle marks you back on PMs, manual doesnt.

I have two concerns with 'Busy' status:

Screen Shot 2019-07-07 at 14 15 58

makes it super obvious who TF is actually around to talk to...

Actually I guess I suppose it's fine to have a specified difference between manual and auto away. But if we do that, I would also want to make the busy status behave like the afk statuses with a enum character for consistency. That was we could also add checks for users having a busy status elsewhere to suppress some notifications and messages.

I agree, if we have a 'Busy' state it would make sense to have a different symbol than 'Away'/'Idle'. However, I feel '!' is already a better symbol for Busy, and something like '?' would be better for Away/Idle/AFK (ie. in the latter case we're not sure if the user is there or not, whereas in the first case the user is explicitly warning theyre busy)

Maybe 'manual-Away' is really just a Busy-status that only gets cleared on /back + set the away bit so that there are more noticeable UI differences. I'd be fine with that. Online/Busy/Away/Offline (Away = brb/idle/afk, all of which clear on any activity).

scheibo commented 5 years ago

Consolidating what was discussed on our Discord and Zarel/Pokemon-Showdown-Client#1316:

RESOLVED

POSSIBLY RESOLVED?

UNRESOLVED

scheibo commented 5 years ago

@Zarel @bumbadadabum @TheJetOU @whalemer

Can we please hash out the remaining unresolved points (5-6 bullets above) so I can make a check list of what still needs to be done on the server and client and actually implement this?

thejetou commented 5 years ago

only the /clearstatus command clears the status message, /back just removes the Busy/Idle state, not whatever message the user had set

Agree. in the code base they're also treated separately, so I don't see why it shouldn't be the same in the commands. Also it doesn't make sense for /back to clear a status, and is just confusing.

@? and @! for Idle and Busy in the protocol (bumba/pre/jumbowhales preference) vs. @! and @ (Zarel)

Former. being explicit is almost always better.

do we want to sort Busy and Idle differently? ie/ is Busy > Idle or Idle > Busy or Idle === Busy?

Idle === Busy. It's really unnecessary to sort them, maybe if you really want the color coordination.

do we need to find a new name for the /clearstatus command when being used to clear other user's statuses? /clearstatus (and a possible /cs alias) is intuitive, and while from a code POV it may be a little awkward, it ends up reducing cognitive load on users

Maybe /clearuserstatus? That seems a bit verbose though. Something like /removestatus would just be downright confusing.

scheibo commented 5 years ago

maybe if you really want the color coordination.

Yeah, this is mostly what I want. My OCD wants all the symbols to line up. my instinct is idle then busy, because yellow then red. which makes me ask - should we have a green symbol for online (will it look odd that only 2 out of the 3 visible states in the user list have symbols?)

Zarel commented 5 years ago

Former. being explicit is almost always better.

I am really annoyed at people for not understanding that this has nothing to do with explicitness. They're both equally explicit.

@? and @! convey that Idle and Busy are different statuses.

@! and @ convey that Idle is a type of Busy status.

I think Idle is a type of Busy status. Therefore I prefer @! and @. That's all there is to it.

syntax for preset messages

Did we even decide we needed presets? This seems premature.

box sprites being part of status. I really don't think this works given theyre going to be 68x56 in the new gen and theyre already 40x30. I think they are a separate feature, like a second avatar. Not sure where we'd place them on the usercard...

This also seems premature.

Yeah, this is mostly what I want. My OCD wants all the symbols to line up. my instinct is idle then busy, because yellow then red. which makes me ask - should we have a green symbol for online (will it look odd that only 2 out of the 3 visible states in the user list have symbols?)

I'm fine with sorting them so the color-icon doesn't look weird.

I don't want a green symbol for online. It would make sense if we also show offline users in the list, but we don't.

do we need to find a new name for the /clearstatus command when being used to clear other user's statuses? /clearstatus (and a possible /cs alias) is intuitive, and while from a code POV it may be a little awkward, it ends up reducing cognitive load on users

I don't understand the question here. Why are we considering a rename of this command?

scheibo commented 5 years ago

I am really annoyed at people for not understanding that this has nothing to do with explicitness. They're both equally explicit.

@? and @! convey that Idle and Busy are different statuses.

@! and @ convey that Idle is a type of Busy status.

I think Idle is a type of Busy status. Therefore I prefer @! and @. That's all there is to it.

Maybe if no one agrees/understands it's because its not a good notation...

Idle isnt a type of Busy status - you're not Busy because youre Idle. Idle doesnt blockpms/blockchallenges like Busy does. They denote two different states/ideas.

Did we even decide we needed presets? This seems premature.

There is still an interest from US to pursue this, and while I would only consider implementing it after everything else, I'd rather we carve out a place in our design now so that if we do end up implementing them it can be done quickly as opposed to having to go back to the drawing board.

[Box Sprites] also seems premature.

Once again, I'd rather design with the future in mind. Obviously they are going to come later on, but let's design with them in mind of now.

I'm fine with sorting them so the color-icon doesn't look weird.

Online -> Idle -> Busy?

I don't want a green symbol for online. It would make sense if we also show offline users in the list, but we don't.

OK.

I don't understand the question here. Why are we considering a rename of [/clearstatus]?

Because this is literally something you requested, though no one else really feels like we need to/should. If you also don't remember/don't want to rename it, I'm more than happy to call this resolved.

Zarel commented 5 years ago

Maybe if no one agrees/understands it's because its not a good notation...

Idle isnt a type of Busy status - you're not Busy because youre Idle. Idle doesnt blockpms/blockchallenges like Busy does. They denote two different states/ideas.

You totally are busy because you're idle. It makes sense to me.

I'm really modeling it as:

It's also relevant that I feel like there should be an "Away (reason: unspecified)" status, and that Busy is that status.

(presets) There is still an interest from US to pursue this, and while I would only consider implementing it after everything else, I'd rather we carve out a place in our design now so that if we do end up implementing them it can be done quickly as opposed to having to go back to the drawing board.

It's more that it doesn't seem like it would block anything. It wouldn't need protocol changes, just a command.

(box sprites) Once again, I'd rather design with the future in mind. Obviously they are going to come later on, but let's design with them in mind of now.

I still think "emoji in status" is a good model, but I'm open to alternatives.

Online -> Idle -> Busy?

I'd put Busy first.

Because this is literally something you requested, though no one else really feels like we need to/should. If you also don't remember/don't want to rename it, I'm more than happy to call this resolved.

I'm not a fan of /clearstatus, but it's not like I have any better ideas at the moment. It's also not the sort of thing that blocks anything else.

Like, my model here is, "would doing everything else here make it harder to come back to this later?" and the answer for presets and a /clearstatus rename seems like a resounding no. There's no reason to block an AFK protocol rework on them because they're entirely orthogonal.

scheibo commented 5 years ago

OK, here's what I think the remaining work is:


Server

Client

Zarel commented 5 years ago
  • denote Busy with @! and Idle with @? (4 vs. 1 here...) and remove the status message from the protocol

This isn't a democracy. I'm not going to allow this without a bottom status unless someone gives me a good reason why we shouldn't have an "Away (unspecified reason)" status.

If you don't think "Busy" is a good name for it, we can call it something else.

  • return the custom message set by /status in /cmd userinfo

Isn't this already done?

  • only show a popup (or even a non-notifying PM) when the user goes Idle once per session
  • change idle timeout to 15 minutes (for both reg and staff. Discord is 10...)

If we change the idle timeout, we should probably allow all activity to wake PS, instead of needing to specifically send a message. I'm sure we've already realized that clicking a username should be enough, but so should just mousing over the window.

I'm not particularly happy with the popup in general, but it would definitely be really bad if it were 15 minutes, and also would sort of defeat the purpose if it only happened once per session instead of every time. The popup exists to deal with a problem that some users don't want to be seen as idle if they're at their computer and responding to highlights. Do we have a better solution?

  • allows users to set their status from the usercard directory (contenteditable) and busy/idle/online state similarly (icon dropdown?)

This definitely wouldn't need contenteditable, just a regular textarea. Heck, just an Edit button would probably be best.

  • /showjoins should include custom message changes (not busy -> idle -> online etc)

Well, this part is pretty hard to implement with the newly specced protocol. I'd be okay with punting it.

Everything else sounds good to me.

scheibo commented 5 years ago

Spoiler: my previous post was meant to be a forcing function. Seems to have worked :)

I'm not going to allow this without a bottom status unless someone gives me a good reason why we shouldn't have an "Away (unspecified reason)" status.

Bottom status is Online though, no? As for a bottom AFK status - thats kind of what I was originally going for with the Away vs. Busy vs. Idle split from the very first design. IDRC though, I just want to ship. Changed to @ and @!.

Isn't this already done?

You're right.

If we change the idle timeout, we should probably allow all activity to wake PS, instead of needing to specifically send a message. I'm sure we've already realized that clicking a username should be enough, but so should just mousing over the window.

Any activity on the connection causes a wake. Do we want mousing over the window to send a command to the server to wake?

The popup exists to deal with a problem that some users don't want to be seen as idle if they're at their computer and responding to highlights. Do we have a better solution?

?

How does the popup affect this in any way? The popup was intended to explain to users why the heck they just went grey, it doesnt prevent them from going idle or anything.

This definitely wouldn't need contenteditable, just a regular textarea. Heck, just an Edit button would probably be best.

Is contenteditable seems to be analogous to how the avatar-in-usercard design works (theres no 'change avatar' button). I was trying to keep the same design language.

Well, this part is pretty hard to implement with the newly specced protocol. I'd be okay with punting it.

Ah true.

Zarel commented 5 years ago

Any activity on the connection causes a wake. Do we want mousing over the window to send a command to the server to wake?

Probably throttled by "five minutes since last network activity", but yes.

?

How does the popup affect this in any way? The popup was intended to explain to users why the heck they just went grey, it doesnt prevent them from going idle or anything.

The way @bumbadadabum explained it to me, it was also to remind them they needed to do something if they didn't want to go idle. Actually, his original idea was to do it a few minutes before going idle, for that reason.

Is contenteditable seems to be analogous to how the avatar-in-usercard design works (theres no 'change avatar' button). I was trying to keep the same design language.

I would probably have added an "edit" icon over the avatar if I had that kind of free time. I'd definitely want to add an Edit button/icon next to the status, although yes, clicking the status should also initiate editing. That's still different from actual contenteditable (I would honestly lean towards an invisible textarea over literally contenteditable).

iscke commented 5 years ago

Any activity on the connection causes a wake. Do we want mousing over the window to send a command to the server to wake?

Probably throttled by "five minutes since last network activity", but yes.

At this point why not just move setting idle to the client? Would also deal with the code to stop bots going idle in (imo) a much nicer way.

Zarel commented 5 years ago

At this point why not just move setting idle to the client? Would also deal with the code to stop bots going idle in (imo) a much nicer way.

That's probably a pretty good idea, but I've also wanted to add activity pings to PS, too. We have a lot of zombie connections because SockJS says they have a heartbeat but they actually don't.