Opened 5 years ago

Closed 5 years ago

Last modified 4 years ago

#3242 closed enhancement (fixed)

[PATCH] Allow spectators to join the game after it started

Reported by: elexis Owned by: leper
Priority: Nice to Have Milestone: Alpha 19
Component: Network Keywords: patch
Cc: Patch:

Description

The gamesetup should have an option that allows new clients to join the game as observers, even if they were not connected before the game started.

Besides spectating games, this would allow them to estimate how long the game will take to finish and even communicate to the players as well. Sometimes people also miss to join the game before it starts by seconds. Since its optional, private matches can still be private.

Unwanted spectators should be kicked then (see #3241).

Attachments (8)

t3242_observer_late_join_v1.patch (8.1 KB ) - added by elexis 5 years ago.
Thanks again to Yves for helping me with the SpiderMonkey interaction! This patch implements the described function.
t3242_clear_player_assignments_when_starting_the_game.patch (846 bytes ) - added by elexis 5 years ago.
Fixes the other bug described in the comment above. Prevens players and observers from rejoining if they were not present when the game started.
t3242_observer_late_join_v2.patch (11.5 KB ) - added by elexis 5 years ago.
Prevents joins (and rejoins) while the game is loading. Uses std::find_if and lambda function. Increments PS_PROTOCOL_VERSION. Fixes the options window size in singleplayer and non-lobby multiplayer games. Fixes trailing whitespace.
t3242_observer_late_join_v2.1.patch (11.9 KB ) - added by elexis 5 years ago.
const & in lambda function. Hopefully improved the readability of the observerLateJoin initialization and containing CNetServerWorker::OnAuthenticate.
t3242_observer_late_join_v2.2.patch (11.9 KB ) - added by elexis 5 years ago.
Adds a newline and a check (HasProperty) in NetServer.cpp to suppress a warning.
t3242_observer_late_join_v2.3.patch (11.1 KB ) - added by elexis 5 years ago.
I'd prefer the lambda function over a loop, but I could change it.
t3242_observer_late_join_v2.4.patch (11.0 KB ) - added by elexis 5 years ago.
That what Itms said.
t3242_observer_late_join_v2.5.patch (11.9 KB ) - added by elexis 5 years ago.
Increments protocol version, changed caption to "Late Observer Joins", fixes whitespace.

Download all attachments as: .zip

Change History (19)

comment:1 by elexis, 5 years ago

Summary: Allow spectators rejoin after the game startedAllow spectators join the game after it started

by elexis, 5 years ago

Thanks again to Yves for helping me with the SpiderMonkey interaction! This patch implements the described function.

comment:2 by elexis, 5 years ago

Keywords: patch review added
Milestone: BacklogAlpha 19
Summary: Allow spectators join the game after it started[PATCH] Allow spectators join the game after it started
Type: defectenhancement

comment:3 by elexis, 5 years ago

You might encounter a bug when testing this patch. The bug applies to alpha 18 too, so don't blame my patch. This issue and the patch below was moved to #3305. Reproduce: (1) open a match (2) join and disconnect with the client (3) start the game (without the client) (4) the client can rejoin as player 2, although he was not present when the game started.

Although the player was not present when the game started, he was still assigned I guess.

Last edited 5 years ago by elexis (previous) (diff)

by elexis, 5 years ago

Fixes the other bug described in the comment above. Prevens players and observers from rejoining if they were not present when the game started.

by elexis, 5 years ago

Prevents joins (and rejoins) while the game is loading. Uses std::find_if and lambda function. Increments PS_PROTOCOL_VERSION. Fixes the options window size in singleplayer and non-lobby multiplayer games. Fixes trailing whitespace.

comment:4 by elexis, 5 years ago

Owner: set to leper
Summary: [PATCH] Allow spectators join the game after it started[PATCH] Allow spectators to join the game after it started

by elexis, 5 years ago

const & in lambda function. Hopefully improved the readability of the observerLateJoin initialization and containing CNetServerWorker::OnAuthenticate.

by elexis, 5 years ago

Adds a newline and a check (HasProperty) in NetServer.cpp to suppress a warning.

comment:5 by Itms, 5 years ago

Thanks for working on this.

  • Error messages: keep that simple ("Game has already started, no observers allowed")
  • gamesetup.js and following occurences: g_GameAttributes.settings.observerLateJoin should be ObserverLateJoin to stay consistent
  • I think we don't need to update the protocol version as it already was changed for A19 in r16614. SVN users should be up-to-date.
  • Is this block on line 819 of NetServer.cpp really more readable? And as far as I can see it won't be more efficient. A range-based for would already improve the style.

by elexis, 5 years ago

I'd prefer the lambda function over a loop, but I could change it.

comment:6 by Itms, 5 years ago

This looks nice. Three remarks:

  • I think the caption of the new game option is very long? What about "Allow late rejoin"? With the tooltip I think it's clear enough.
  • Your translation comments are useless here. They're meant for translating strings like "Yes", or "None" that need context.
  • The problem with the lambda function is that your current disposition makes it hard to read. I propose the following
isRejoining =
    observerLateJoin || 
    std::find_if(
        server.m_PlayerAssignments.begin(), server.m_PlayerAssignments.end(), 
 	[&username] (const std::pair<CStr, PlayerAssignment>& pair) 
 	{ return pair.second.m_Enabled && pair.second.m_Name == username; }) 
    != server.m_PlayerAssignments.end();

Notice that I removed one pair of parentheses.

by elexis, 5 years ago

That what Itms said.

by elexis, 5 years ago

Increments protocol version, changed caption to "Late Observer Joins", fixes whitespace.

comment:7 by Itms, 5 years ago

Resolution: fixed
Status: newclosed

In 16945:

Optionally allow observers to rejoin a game when they weren't here during the game setup.

Patch by elexis, fixes #3242.

comment:8 by Itms, 5 years ago

Keywords: review removed

Thanks for the patch :)

comment:9 by leper, 5 years ago

In 16955:

Fix player rejoins. Refs #3242.

comment:10 by elexis, 5 years ago

Yesterday someone disconnected from a game and rejoined. Somehow the server still thought the player was in the game, so he couldn't rejoin as that player but rejoined as a spectator.

Same can occur in the gamesetup without observer-late-join.

Maybe we should completely forbid joins if that playername is already connected instead of calling DeduplicatePlayername in CNetServerWorker::OnAuthenticate, introducing a new disconnect reason?

comment:11 by elexis, 4 years ago

Component: Core engineNetwork

(set component to network)

Note: See TracTickets for help on using tickets.