Feature/#1966 friend in game indicator #2034

Atrocyte · 2020-11-13T11:35:51Z

Looks up online friends and matches these to the players inside the game lobby.
Games with friends are highlighted and have a smiley
fixes #1966

merge fix

Looks up online friends and matches these to the players inside the game lobby. Games with friends are highlighted Fixes FAForever#1966

src/main/java/com/faforever/client/game/GamesTableController.java

1-alex98 · 2020-12-01T09:45:39Z

src/main/java/com/faforever/client/game/GameTileController.java

@@ -110,6 +112,8 @@ public void setGame(Game game) {
    ));

    lockIconLabel.visibleProperty().bind(game.passwordProtectedProperty());
+    friendsLabel.visibleProperty().bind(new SimpleBooleanProperty(!playerService.friendsInGame(game).isEmpty()));


Is that updated if a friend joins a game?

yes, you can follow friends when they hop from lobby to lobby

my concern was if setGame is called again if a new player joins or if you add a new friend... the later I doubt

And binding makes no sense if the BooleanProperty
is never updated!!!

src/main/java/com/faforever/client/game/GameTileController.java

src/main/java/com/faforever/client/game/GamesTableController.java

1-alex98 · 2020-12-01T09:49:59Z

src/main/java/com/faforever/client/player/PlayerService.java

+          .flatMap(Collection::stream)
+          .collect(Collectors.toList());
+      List<Player> friendsInGame = this.friendsOnline().stream()
+          .filter(e -> playersInGame.contains(e.getUsername()))


That will result in some false positives

@axel1200 I may be missing something here, I am unable to find the false positives in the logic you mention.
This weekend I ran some tests and I did not see any false positives occur when running multiple hours with 1000+ users online.
Could you elaborate on the false positives?

I don't think this will have false positives as each username is assumed to be unique and this checks if the username is in the list correct?

yea that's right

src/main/resources/theme/style.css

Sheikah45 · 2021-01-06T01:06:27Z

Is this being worked on? If not will close

Atrocyte · 2021-01-11T11:34:37Z

Hey @Sheikah45 I'll get on with it again this month, did not have time to contribute to FAF in December.

Looks up online friends and matches these to the players inside the game lobby. Games with friends are highlighted Fixes FAForever#1966

Marc-Spector · 2021-02-27T03:58:59Z

Hi @Atrocyte
Is there any progress? I can help you finish your PR or implement it my own way. :)

Atrocyte · 2021-02-28T09:38:41Z

hey @ivan-gryzunov! The PR is stuck on a comment that my methods friendsOnline and friendInGame in the PlayerService cause false positives. I can't find any false positives in logic and in testing, so could you please comment/help on that?

Apart from that bit the change is ready to go IMO, thank you for participating :)

Sheikah45 · 2021-02-28T10:55:53Z

@Atrocyte it looks like there are also still open issues with the changing of the id rather than state as well

…icator' into feature/#1966_friend_in_game_indicator # Conflicts: # src/main/java/com/faforever/client/game/GameTileController.java # src/main/java/com/faforever/client/game/GamesTableController.java # src/main/resources/theme/style.css

…icator # Conflicts: # src/main/resources/theme/play/games_table.fxml # src/test/java/com/faforever/client/game/GamesTableControllerTest.java

…icator

codecov · 2021-03-09T10:25:53Z

Codecov Report

Merging #2034 (53a383a) into develop (e2a41ba) will increase coverage by 0.01%.
The diff coverage is 69.56%.

@@              Coverage Diff              @@
##             develop    #2034      +/-   ##
=============================================
+ Coverage      54.82%   54.84%   +0.01%     
- Complexity      3730     3739       +9     
=============================================
  Files            573      573              
  Lines          19883    19906      +23     
  Branches        1148     1152       +4     
=============================================
+ Hits           10901    10917      +16     
  Misses          8385     8385              
- Partials         597      604       +7

Impacted Files	Coverage Δ	Complexity Δ
.../com/faforever/client/game/GameTileController.java	`92.30% <0.00%> (-3.70%)`	`14.00 <0.00> (ø)`
...om/faforever/client/game/GamesTableController.java	`73.60% <50.00%> (-1.19%)`	`52.00 <5.00> (+5.00)`	⬇️
...ava/com/faforever/client/player/PlayerService.java	`63.50% <86.66%> (+1.87%)`	`39.00 <4.00> (+4.00)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e2a41ba...7548da5. Read the comment docs.

1-alex98 · 2021-03-09T13:36:52Z

src/main/java/com/faforever/client/game/GameTileController.java

@@ -110,6 +112,8 @@ public void setGame(Game game) {
    ));

    lockIconLabel.visibleProperty().bind(game.passwordProtectedProperty());
+    friendsLabel.visibleProperty().bind(new SimpleBooleanProperty(!playerService.friendsInGame(game).isEmpty()));


my concern was if setGame is called again if a new player joins or if you add a new friend... the later I doubt

1-alex98 · 2021-03-09T13:37:02Z

src/main/java/com/faforever/client/game/GameTileController.java

@@ -110,6 +112,8 @@ public void setGame(Game game) {
    ));

    lockIconLabel.visibleProperty().bind(game.passwordProtectedProperty());
+    friendsLabel.visibleProperty().bind(new SimpleBooleanProperty(!playerService.friendsInGame(game).isEmpty()));


my concern was if setGame is called again if a new player joins or if you add a new friend... the later I doubt

1-alex98 · 2021-03-09T13:37:32Z

src/main/java/com/faforever/client/game/GameTileController.java

@@ -110,6 +112,8 @@ public void setGame(Game game) {
    ));

    lockIconLabel.visibleProperty().bind(game.passwordProtectedProperty());
+    friendsLabel.visibleProperty().bind(new SimpleBooleanProperty(!playerService.friendsInGame(game).isEmpty()));


And binding makes no sense if the BooleanProperty
is never updated!!!

1-alex98 · 2021-03-09T13:38:15Z

src/main/java/com/faforever/client/game/GamesTableController.java

+    friendsColumn.setCellValueFactory(param -> new SimpleBooleanProperty(!playerService.friendsInGame(param.getValue()).isEmpty()));
+    friendsColumn.setCellFactory(param -> friendsCell());


also here not updated

1-alex98 · 2021-03-09T13:38:56Z

src/main/java/com/faforever/client/player/PlayerService.java

+        .collect(Collectors.toList());
+  }
+
+  public List<Player> friendsInGame(Game game) {


Should probably be an Observable list so you can Listen for changes

1-alex98 · 2021-03-09T13:40:30Z

Some of my comments are marked as resolved but they are not no idea how that happend

Sheikah45 · 2021-04-07T18:03:13Z

Is this still being worked on?

Sheikah45 · 2021-05-02T12:21:16Z

Gonna close if not being worked on

Sheikah45 · 2021-05-23T12:16:28Z

Stale

Atrocyte added 3 commits October 19, 2020 21:17

Merge pull request #1 from FAForever/develop

8444269

merge fix

Merge remote-tracking branch 'upstream/develop' into develop

ed9857e

Indicate friends in game

2661930

Looks up online friends and matches these to the players inside the game lobby. Games with friends are highlighted Fixes FAForever#1966

Sheikah45 reviewed Nov 29, 2020

View reviewed changes

src/main/java/com/faforever/client/game/GamesTableController.java Outdated Show resolved Hide resolved

1-alex98 requested changes Dec 1, 2020

View reviewed changes

Indicate friends in game

70b5ce8

Looks up online friends and matches these to the players inside the game lobby. Games with friends are highlighted Fixes FAForever#1966

Atrocyte added 3 commits March 1, 2021 13:55

Merge branch 'upstream_develop' into feature/#1966_friend_in_game_ind…

36fcb15

…icator # Conflicts: # src/main/resources/theme/play/games_table.fxml # src/test/java/com/faforever/client/game/GamesTableControllerTest.java

Merge branch 'upstream_develop' into feature/#1966_friend_in_game_ind…

7548da5

…icator

Atrocyte requested a review from 1-alex98 March 9, 2021 10:28

1-alex98 requested changes Mar 9, 2021

View reviewed changes

Sheikah45 closed this May 23, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/#1966 friend in game indicator #2034

Feature/#1966 friend in game indicator #2034

Atrocyte commented Nov 13, 2020 •

edited

1-alex98 Dec 1, 2020

Atrocyte Mar 1, 2021

1-alex98 Mar 9, 2021

1-alex98 Mar 9, 2021

1-alex98 Mar 9, 2021

1-alex98 Dec 1, 2020

Atrocyte Feb 8, 2021

Sheikah45 Feb 28, 2021

Atrocyte Mar 1, 2021

Sheikah45 commented Jan 6, 2021

Atrocyte commented Jan 11, 2021

Marc-Spector commented Feb 27, 2021

Atrocyte commented Feb 28, 2021

Sheikah45 commented Feb 28, 2021

codecov bot commented Mar 9, 2021

1-alex98 Mar 9, 2021

1-alex98 Mar 9, 2021

1-alex98 Mar 9, 2021

1-alex98 Mar 9, 2021

1-alex98 Mar 9, 2021

1-alex98 commented Mar 9, 2021

Sheikah45 commented Apr 7, 2021

Sheikah45 commented May 2, 2021

Sheikah45 commented May 23, 2021

		friendsColumn.setCellValueFactory(param -> new SimpleBooleanProperty(!playerService.friendsInGame(param.getValue()).isEmpty()));
		friendsColumn.setCellFactory(param -> friendsCell());

Feature/#1966 friend in game indicator #2034

Feature/#1966 friend in game indicator #2034

Conversation

Atrocyte commented Nov 13, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Sheikah45 commented Jan 6, 2021

Atrocyte commented Jan 11, 2021

Marc-Spector commented Feb 27, 2021

Atrocyte commented Feb 28, 2021

Sheikah45 commented Feb 28, 2021

codecov bot commented Mar 9, 2021

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

1-alex98 commented Mar 9, 2021

Sheikah45 commented Apr 7, 2021

Sheikah45 commented May 2, 2021

Sheikah45 commented May 23, 2021

Atrocyte commented Nov 13, 2020 •

edited