Opened 8 years ago

Closed 8 years ago

#3804 closed enhancement (fixed)

[PATCH] Gamesetup - swapping players should also swap the civs

Reported by: elexis Owned by: Vladislav Belov
Priority: Nice to Have Milestone: Alpha 21
Component: UI & Simulation Keywords: patch
Cc: Patch:

Description

When the host swaps two players in the player-assignments of the gamesetup, their civs (but not team-numbers) should be swapped too.

Use-case:

  • Multiplayergames are typically played with two teams, i.e. team numbers are usually fixed to 1*2* throughout the gamesetup.
  • Typcially the gamesetup occurs iteratively. Repeatedly playerslots and civilizations are changed until every player is satisfied with the assignments. (Therefore we can't assume a linear gamesetup with only 16 dropdown interactions for 8 players).
  • With every team-change, the host has two recall the two players that changed slots and which civilizaitons they had before.
  • Implementing the task would eliminate that issue, thus ease the repetetive and stressful massively-multiplayer-online-strategy-gamesetup.

Attachments (3)

gamesetup.js.patch (963 bytes ) - added by Vladislav Belov 8 years ago.
gamesetup.js.2.patch (900 bytes ) - added by Vladislav Belov 8 years ago.
gamesetup.js.3.patch (1.9 KB ) - added by Vladislav Belov 8 years ago.

Download all attachments as: .zip

Change History (12)

by Vladislav Belov, 8 years ago

Attachment: gamesetup.js.patch added

comment:1 by Vladislav Belov, 8 years ago

We could swap not only civs, but colors/team too, is it needed?

comment:2 by Vladislav Belov, 8 years ago

Keywords: patch review added
Milestone: BacklogAlpha 21
Owner: set to Vladislav Belov
Status: newassigned
Summary: Gamesetup - swapping players should also swap the civs[PATCH] Gamesetup - swapping players should also swap the civs

by Vladislav Belov, 8 years ago

Attachment: gamesetup.js.2.patch added

comment:3 by Itms, 8 years ago

Keywords: review removed

This patch exposes a bug in the gamesetup, so putting it out of the queue while Vladislav works on it.

comment:4 by Vladislav Belov, 8 years ago

Keywords: review added

Fixed the Itms's issue.

Last edited 8 years ago by Vladislav Belov (previous) (diff)

by Vladislav Belov, 8 years ago

Attachment: gamesetup.js.3.patch added

comment:5 by elexis, 8 years ago

The persistmatchsettings bug mentioned by Itms and addressed in the patch seems to be #2982.

comment:6 by Itms, 8 years ago

Really sorry for the late review. Why do you add this outdatedAttributes boolean? Wouldn't it be possible to just call updateGameAttributes() at the end to be sure everything is correctly updated?

in reply to:  6 comment:7 by Vladislav Belov, 8 years ago

Replying to Itms:

Really sorry for the late review. Why do you add this outdatedAttributes boolean? Wouldn't it be possible to just call updateGameAttributes() at the end to be sure everything is correctly updated?

You can try the previous patch. And it has the bug (that you noticed): bot is empty. So it's necessary to have this bool.

comment:8 by elexis, 8 years ago

Keywords: simple review removed
Priority: Must HaveNice to Have
  • The persistmatchsettings part is wrong as it shouldn't write something to that file that wasn't actually set. If it was really the same bug as #2982, then it should be addressed differently.
  • The outdatedAttributes is not only weird but also wrong. For singleplayergames it is sufficient to change the g_GameAttributes. For multiplayer, the new attributes are sent via network right after the call to swapPlayers, rtfm. (The only difference between SetNetworkGameAttributes and updateGameAttributes is that the latter also calls sendRegisterGameStanza, which is not relevant with the current code.
  • One trailing whitespace after the equal character

comment:9 by elexis, 8 years ago

Resolution: fixed
Status: assignedclosed

In 18124:

Swap the civilizations when swapping players in the gamesetup. Based on patch by Vladislav, fixes #3804.

Note: See TracTickets for help on using tickets.