feat: Add AFK state for players in multiplayer #3571

iHDeveloper · 2018-12-04T06:22:30Z

Contains

As a gamer sometimes I have to go away from the keyboard for some reason and I want to tell my friends who I'm playing with that I'm away so they won't get mad at me or something. So, I made an afk command to do that goal.

How to test

Pull from My Fork from branch afk-command
Run the game from the source code
Make sure you in multiplayer mode like (Hosting game or Playing online)
Show the game console
Type afk
You can now go away without telling your friends that I'm AFK in the chat 😄

Issues

No more issues finally 😁

Client can't send request to the server
Rebuild the way that client and server communicate in the AFKSystem
Auto disable the AFK when the player moves
Test it

GooeyHub · 2018-12-04T06:33:43Z

Hooray Jenkins reported success with all tests good!

GooeyHub · 2018-12-04T16:03:13Z

Hooray Jenkins reported success with all tests good!

GooeyHub · 2018-12-04T16:14:13Z

Hooray Jenkins reported success with all tests good!

GooeyHub · 2018-12-04T16:25:09Z

Hooray Jenkins reported success with all tests good!

syntaxi

Substantially better this time round. I've a few minor comments but they're trivial and the GCI task can be approved without them.
Just need to test it out in game and we should be good to go.

engine/src/main/java/org/terasology/logic/afk/AFKSystem.java

kaen · 2018-12-05T05:44:23Z

I tested this out locally. First very minor note is that the message in the console uses a curly brace where a square bracket is expected: [AFK} You are AFK!

More importantly, it seems that this only works as intended for the host going afk. Here are some scenarios I tested by hosting with (no saves) launch configuration, then launching 2nd and 3rd client from the configuration dropdown and connecting to localhost server.

Scenario 1 (works as intended)

host types afk
host shows afk on their own player list
host shows afk on 2nd client
host shows afk on 3rd client
host returns from afk
host shows not afk on both client lists

Scenario 2 (not working as intended)

2nd client types afk
2nd client shows afk on their own list
2nd client does not show afk on 3rd client's list
2nd client does not show afk on host's list

Scenario 3 (not working as intended)

host, 2nd, and 3rd client type afk
host list only shows host afk
2nd client shows host and 2nd client afk
3rd client show host and 3rd client afk
clients are not shown afk on other client's lists

Cervator

Can confirm as well - AFK shows for the player running the command, but that's it :-)

Also, I get the impression the player is meant to see the "Welcome back, you are no longer AFK!" message in the little chat message bar at the bottom of the screen? It pops up when a player starts moving, but appears blank to me, while the message gets logged in the regular console OK. Unsure if that's bug here or if any input to the console will trigger a blank chat notification as well.

A few times it seemed like I went un-afk without really doing anything. Maybe it is overly sensitive? Perhaps instead of using the move event use the one where the player has sent an actual message? Then they're certainly no longer afk. But if a cat poked their mouse - no more afk! :-)

Can also validate that the afk command doesn't work in single player, where it would be kinda silly. But on the other hand having extra logic for that may not be needed.

I suspect you need one more layer to make this work. See how AFKSystem (which should really be AfkSystem, likewise the component should be AfkComponent) is set to ALWAYS ? That means it'll do the same thing both on clients and the server. Right now it works solely locally.

You may well want to have a Client only system that registers and handles the command, but sends a note to the server "Hey I'm going AFK" (you send an event that's being listened to in an Authority system) and then the server can send a BroadcastEvent that the client systems catch with the name of the now-AFK player in it. Then each client can update their local state to know that player X is AFK and show that in the overlay. When the player comes back you repeat the process to let everybody know that player is back now

So in other words:

AfkClientSystem registers the command and when triggered sends an event (that's replicated / sent to the server somehow, I forget how) "Hey I'm going AFK, my player name is X"
AfkAuthoritySystem listens to the event indicating that player X is going AFK, re-issues a BroadcastEvent to everybody with that information
AfkClientSystem has a listener for the broadcast event, when received you now know to mark that player AFK in the overlay

You should be able to look at the players online overlay itself to see how it transfers state around to the different clients connected with their ping so it all shows up correctly everywhere :-)

GooeyHub · 2018-12-05T16:41:20Z

Hooray Jenkins reported success with all tests good!

iHDeveloper · 2018-12-05T16:50:02Z

I think we are ready to merge it. I tested it with my friend Falcoph. Thanks, @Cervator for your notes It wasn't gonna happen now without them. Also, Thanks, @kaen for the test scenarios and now this pull can pass all of those scenarios 😄

GooeyHub · 2018-12-05T20:25:26Z

Hooray Jenkins reported success with all tests good!

skaldarnar · 2018-12-06T10:21:08Z

I will test this out when I get a bit free time ;-)
One thing I'm wondering about is whether you need the boolean field on AFKComponent, or whether the presence of the component is enough to indicate that a player is actually afk, and the absence denotes the opposite (not afk).

iHDeveloper · 2018-12-06T13:19:14Z

Thanks for your help @skaldarnar. I implemented new features in the list below:

Shows Idle in the Discord RPC when the player is afk
Auto afk with no active for like 5 minutes
Auto unafk with any move or even touch your keyboard

You can test them and give us the result of your test. That's will be helpful from you 😄
Note: You can also try the scenarios that @kaen did in the last test result from him. 😉

GooeyHub · 2018-12-06T13:23:01Z

Hooray Jenkins reported success with all tests good!

syntaxi · 2018-12-06T14:37:04Z

I probably should have been clearer with my explanation.
This is mostly a rehash of my discord message, but here it won't scroll away.

What I meant was that the DelayManager listens for changes made to PeriodicActionComponent
So rather than re-implementing the delay logic from DelayManager I was intending for you to look at the logic only in addPeriodicAction and then copy parts of it.
You'll still use DelayManager behind the scenes, just changing how you access it.

To get more technical and specific, you should delete all the methods you copied except for addPeriodicAction, the PeriodicActionTriggeredEvent handler and getPeriodicComponent.
Then call the add method in postBegin and that should work.

If you can't get it to work then you could either just do the auto afk because it's not strictly needed for the task, or try and move it to the Authority method you have.

GooeyHub · 2018-12-07T07:11:20Z

Hooray Jenkins reported success with all tests good!

GooeyHub · 2018-12-07T07:22:05Z

Hooray Jenkins reported success with all tests good!

GooeyHub · 2018-12-07T08:32:38Z

Hooray Jenkins reported success with all tests good!

skaldarnar

Tests out fine with two clients in a local setup after updating this PR according to NUI extraction.

I'm not sure whether this should live directly in the engine, or whether it should be extracted to a module, but that can be done in a follow-up.

So, better late than never - LGTM!

skaldarnar · 2020-11-01T19:30:05Z

engine/src/main/java/org/terasology/logic/afk/AfkAuthoritySystem.java

+    @In
+    private NetworkSystem networkSystem;
+
+    private int count;


I'm not sure whether this is actually required. I'm wondering whether it would also work to just rely on the entity information we get on ConnectedEvent and DisconnectedEvent events.

It feels a bit too bulky to keep track of the count and look through all connected players additionally.

I've created #4242 to keep track of this. @iHDeveloper might this be your next contribution? 😉

@skaldarnar Sure! I'm currently busy with exams. But, I will work on it during the weekend 😁

Changes have been addressed

iHDeveloper added 9 commits Dec 2, 2018

AFK: Build the basics

c6fd06b

Print the AFK in the OnlinePlayersOverlay & Fix the network stuff

f4eac0f

AFK: Fix some issues and test some stuff

829ab27

AFK: Apply CheckStyle

c5b39b9

AFK: Try to fix the send request problem

a896a55

AFK: Fix some issues in the afk request

22e60f8

AFK: Sperate instead of having alot of things in one line

1b551dc

AFK: Add a target to the event to handle what we are looking for

307fadd

AFK: Register the events when the system initialised

Loading status checks…

f1f516e

syntaxi added UI Multiplayer Api WIP labels Dec 4, 2018

iHDeveloper added 3 commits Dec 4, 2018

AFK: Test stuff

a113705

AFK: Rebuild the system

Loading status checks…

d787000

AFK: Add auto unafk when the player moves

Loading status checks…

9cf51d5

iHDeveloper changed the title ~~[W.I.P] Add new AFK flag and display it on the online players overlay~~ Add new AFK flag and display it on the online players overlay Dec 4, 2018

AFK: Apply checkstyle

Loading status checks…

b9cfee2

syntaxi reviewed Dec 4, 2018

View changes

engine/src/main/java/org/terasology/logic/afk/AFKSystem.java Outdated Show resolved Hide resolved

engine/src/main/java/org/terasology/logic/afk/AFKSystem.java Outdated Show resolved Hide resolved

engine/src/main/java/org/terasology/logic/afk/AFKSystem.java Outdated Show resolved Hide resolved

Cervator reviewed Dec 5, 2018

View changes

iHDeveloper added 2 commits Dec 5, 2018

AFK: Rebuild the system with new way of communication to kill the bugs

Loading status checks…

149ba24

AFK: Fix issue with the client side communication

Loading status checks…

550357e

AFK: Add auto afk/unafk with AFK Screen

Loading status checks…

d180f5b

iHDeveloper added 2 commits Dec 6, 2018

AFK: Implement 'idle' state in the Discord RPC

1778243

AFK: Never use 'DelayManager' and create own delay

Loading status checks…

8eaff4e

iHDeveloper added 2 commits Dec 7, 2018

AFK: Rebuild the auto afk/unafk system

Loading status checks…

aa1762d

AFK: Fix the init issue

Loading status checks…

0630456

AFK: Fix issue with serverside manager and add smooth feature

Loading status checks…

966d844

skaldarnar added 4 commits Nov 1, 2020

Merge remote-tracking branch 'origin/develop' into afk-command

358cb87

fix: adjust to NUI extraction

Loading status checks…

5f384ba

chore: update copyright headers and add some comments

Loading status checks…

dd33e29

chore: update copyright headers and add some comments

Loading status checks…

57584fb

skaldarnar approved these changes Nov 1, 2020

View changes

Merge branch 'develop' into afk-command

Verified

This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.

GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits

Loading status checks…

fe1650b

skaldarnar mentioned this pull request Nov 11, 2020

Remove 'count' from 'AfkAuthoritySystem' #4242

Open

0 of 2 tasks complete

skaldarnar removed the WIP label Nov 11, 2020

skaldarnar added this to the v4.1.0 milestone Nov 11, 2020

skaldarnar changed the title ~~Add new AFK flag and display it on the online players overlay~~ feat: Add AFK state for players in multiplayer Nov 11, 2020

jdrueckert mentioned this pull request Nov 11, 2020

Investigate / Fix closing of AFK screen #4243

Closed

pollend mentioned this pull request Nov 14, 2020

fix: remove afk screen #4248

Merged

Oct	NOV	Dec
	29
2019	2020	2021

MovingBlocks / Terasology

feat: Add AFK state for players in multiplayer #3571

feat: Add AFK state for players in multiplayer #3571

iHDeveloper commented Dec 4, 2018 •

edited

GooeyHub commented Dec 4, 2018

GooeyHub commented Dec 4, 2018

GooeyHub commented Dec 4, 2018

GooeyHub commented Dec 4, 2018

syntaxi left a comment

kaen commented Dec 5, 2018

Cervator left a comment

GooeyHub commented Dec 5, 2018

iHDeveloper commented Dec 5, 2018

GooeyHub commented Dec 5, 2018

skaldarnar commented Dec 6, 2018

iHDeveloper commented Dec 6, 2018

GooeyHub commented Dec 6, 2018

syntaxi commented Dec 6, 2018

GooeyHub commented Dec 7, 2018

GooeyHub commented Dec 7, 2018

GooeyHub commented Dec 7, 2018

skaldarnar left a comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

MovingBlocks / Terasology

Join GitHub today

GitHub is where the world builds software

feat: Add AFK state for players in multiplayer #3571

feat: Add AFK state for players in multiplayer #3571

Conversation

iHDeveloper commented Dec 4, 2018 • edited

Contains

How to test

Issues

GooeyHub commented Dec 4, 2018

GooeyHub commented Dec 4, 2018

GooeyHub commented Dec 4, 2018

GooeyHub commented Dec 4, 2018

syntaxi left a comment

kaen commented Dec 5, 2018

Scenario 1 (works as intended)

Scenario 2 (not working as intended)

Scenario 3 (not working as intended)

Cervator left a comment

GooeyHub commented Dec 5, 2018

iHDeveloper commented Dec 5, 2018

GooeyHub commented Dec 5, 2018

skaldarnar commented Dec 6, 2018

iHDeveloper commented Dec 6, 2018

GooeyHub commented Dec 6, 2018

syntaxi commented Dec 6, 2018

GooeyHub commented Dec 7, 2018

GooeyHub commented Dec 7, 2018

GooeyHub commented Dec 7, 2018

skaldarnar left a comment

This comment has been minimized.

skaldarnar Nov 1, 2020 Member

This comment has been minimized.

skaldarnar Nov 11, 2020 Member

This comment has been minimized.

iHDeveloper Nov 11, 2020 • edited Author Contributor

Essential cookies

Always active

Analytics cookies

iHDeveloper commented Dec 4, 2018 •

edited

skaldarnar Nov 1, 2020
Member

skaldarnar Nov 11, 2020
Member

iHDeveloper Nov 11, 2020 •

edited

Author Contributor