Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#3970 closed enhancement (fixed)

[PATCH] Last man standing - gamesetup option to disable allied victory

Reported by: elexis Owned by: elexis
Priority: Should Have Milestone: Alpha 21
Component: UI & Simulation Keywords: patch
Cc: Patch:

Description

Diplomacy games (teams not locked) would become more entertaining if it were possible to disable the allied-victory mechanism.

This would result in a "last man standing" gametype, forcing players to break the alliance in order to win. Players were incentivized to ally the weaker players, in order to counterbalance the player who is most likely to win (instead of allying that one and be done with the game while making it even harder to beat the strongest).

It should be implemented as a separate boolean option, not a victory condition. This way the mode can be played with all gametypes like wonder-games.

As players might assume allying all players means having ended the game, it might be an idea to show a messagebox that the game isn't over yet.

If teams are set to locked, the gamesetup checkbox should be hidden and the allied-victory mechanism must be enabled.

The EndGameManager.js has a method to disable allied-victory already:

// Allied victory means allied players can win if victory conditions are met for each of them
// Would be false for a "last man standing" game (when diplomacy is fully implemented)

The feature should be possible to implement with changing probably less than 5 files. Feel free to ask on IRC for details (@wiki:GettingStartedProgrammers#QuestionsandSuggestions)!

Attachments (4)

lastmanstanding.patch (6.2 KB ) - added by Sandarac 8 years ago.
Adds check box in More Options
lastmanstanding_v2.patch (6.7 KB ) - added by Sandarac 8 years ago.
All code in EndGameManager.js is called in OnGlobalPlayerDefeated()
lastmanstanding_v3.patch (8.4 KB ) - added by Sandarac 8 years ago.
Thank you for the tips, this should do it all, but the code could be cleaner
lastmanstanding_v4.patch (8.4 KB ) - added by Sandarac 8 years ago.

Download all attachments as: .zip

Change History (17)

by Sandarac, 8 years ago

Attachment: lastmanstanding.patch added

Adds check box in More Options

comment:1 by Sandarac, 8 years ago

Keywords: review patch added
Milestone: BacklogAlpha 21
Summary: Last man standing - gamesetup option to disable allied victory[PATCH] Last man standing - gamesetup option to disable allied victory

comment:2 by elexis, 8 years ago

Thanks, it's a must have for alpha 21 in my opinion! Didn't test the patch yet, logic looks good from reading

Logic:

  • What should happen if only allies are remaining? Nothing? I see players either wondering why the game didn't end yet or assuming that all of them won. We could show the notification as long as all are allied to reminding them. (Setting diplomacies back to neutral or enemy might be a bit too harsh).

Codestyle remarks:

  • Either use LastManStanding or disableAlliedVictory, but not both (and also not LastMan). Those optionFoo things should likely be consistent with the controls it contains (so we have optionFoo which contains foo and fooText). Should probably be LastManStanding as that's the string that would also be displayed to the user.
  • The first part of the tooltip Toggle the "Last Man Standing" option (only one player can win). is redundant (it's obvious that it triggers that option since it's called like that). So only mention the non-obvious things that it does.
  • when diplomacy is fully implemented -> it is as far as I know, so nuke that
  • [1,2,3,4,5,6,7,8] -> oh no, you don't. If you need that, construct it using g_MaxPlayers (#4004), see also playersArray in gamesetup.js. But it also doesn't take observers into account, so maybe you're just better off not passing the players argument?
  • Last civilization remaining wins -> it's the player that wins (there might be 2 players with the same civ). Not a big issue, but for wonder-games the first player to build a wonder wins. (That message is only shown for some seconds after the gamestart?)
  • L112 unneeded parenthesis as && has a higher operator precedence than || (just like * and +): https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Operator_Precedence You could also add a linebreak after the second ||

by Sandarac, 8 years ago

Attachment: lastmanstanding_v2.patch added

All code in EndGameManager.js is called in OnGlobalPlayerDefeated()

comment:3 by elexis, 8 years ago

Keywords: review removed
  • Trailing whitespace in the first changed XML line
  • optionLastManStanding should be hidden if teams are locked
  • "Last Man Standing: No" seems to be counterintuitive in comparison to "Last Man Standing: Yes" and "Allied Victory: Yes", this could only be solved by using a dropdown with two choices "Last Man Standing" and "Allied Victory". But as we don't use dropdowns for binary choices, we'll have to go with only one title, so it's okay this way I guess.
  • delete g_GameAttributes.settings.LastManStanding; that line totally needs to be wrapped in braces, otherwise it's not part of the if-statement.
  • !this.alliedVictory && cmpPlayers.filter(pData => pData.GetState() == "active").length != 1) -> use 4 spaces instead of a tab so it's right below that other condition, regardless of how the tabs are displayed
  • When starting the match with allies only, I don't get a notification. Neither do I get one when changing the diplomacies so that all are allied.
  • Ideally the notification would be shown as long as all players are allied, as 5 seconds are over fast and people often don't read the notification in that time frame. You can probably do it the way the wonder victory does it (AddTimeNotification with maybe a 90 minute limit, not showing the time in that string and then calling DeleteTimeNotification)

comment:4 by elexis, 8 years ago

Any updates? You can identify diplomacy changes in OnDiplomacyChanged (maybe OnGlobalDiplomacyChanged) and then just call the same code. You could add a function to the EndGameManager that does the onlyAlliesLeft check and adding of a timeNotification and call that from the DiplomacyChanged / PlayerDefeated events.

by Sandarac, 8 years ago

Attachment: lastmanstanding_v3.patch added

Thank you for the tips, this should do it all, but the code could be cleaner

comment:5 by Sandarac, 8 years ago

Keywords: review added

comment:6 by elexis, 8 years ago

Keywords: review removed

Just wanted to make sure you didn't abort ;)

You are right, few more cleaning and the code should be done:

  • gamesetup.js L513 newline
  • Toggle if only one player can win (disables allied victory).: "allied victory" seems a bit technical, maybe `Toggle whether the last remaining player or the last remaining set of allies wins" ?

About DisableAlliedVictoryCheck:

  • Rename DisableAlliedVictoryCheck to LastManStandingCheck
  • Any reason for not using this instead of cmpEndGameManager?
  • Add an early return !cmpGuiInterface || !cmpPlayerManager
  • Add an 'early return' function this.alliedVictory (and also delete the notification, so that the code supports changing the setting in a running game, although that is not being used yet)
  • That i == msg.playerId check seems wrong, since you call the function also for the diplomacy changed message. Is that check needed for the defeat message actually or is cmpPlayers[i].GetState() == "active" already the case OnGlobalPlayerDefat`? (In that case you could just delete it)
  • Don't copy code. Reuse it by moving the shared part to a function. For example you could add a function OnlyAlliesLeft if you need that check in multiple places (it should return false or that allies array).
  • I would say merge L111 L112, but L112 is actually not needed if you take a look at GUIInterface.js
  • L106 newline (same goes for L104)
  • L107 following, wrong indentation
  • 10 * 60 * 9000 -> that was split because it ought to be readable, if you want 90min, make that 90 * 60 * 1000 instead (minutes * seconds/minute * milliseconds/second)
  • messages is not needed, just access cmpEndGameManager.disabledAlliedVictoryMessages
  • cmpEndGameManager.disabledAlliedVictoryMessages will never contain anything more than noAlliesLeftMessage, so just use cmpEndGameManager.noAlliesLeftMessage
  • Last player remaining wins -> add an exclamation mark or period
  • Unneeded variable numPlayers
  • OnGlobalPlayerDefeated should just call the LastManStandingCheck instead of introducing any duplicate lines
  • Since both the LastManStandingCheck and OnGlobalPlayerDefeated need the allies array, you could just do LastManStandingCheck(OnlyAlliesLeft())) in the init and diplomacy-changed function and let allies = OnlyAlliesLeft(); LastManStandingCheck(allies); and then the SetWon part.

by Sandarac, 8 years ago

Attachment: lastmanstanding_v4.patch added

comment:7 by Sandarac, 8 years ago

Keywords: review added

comment:8 by elexis, 8 years ago

Owner: set to elexis
Resolution: fixed
Status: newclosed

In 18342:

Last Man Standing mode. Based on patch by Sandarac, fixes #3970.

Also fix a simulation bug (check for allied victory on game start).
The mode is not a victory condition as it applies to all victoriy conditions.

comment:9 by elexis, 8 years ago

Keywords: simple review removed

Thanks for the patch!

I had to change few things though, mostly style issues but also a bug in the simulation (it doesn't mark players as won if the game is started with allies only).

gamesetup.js

  • !this.checked ? this.checked : !this.checked is equal to !this.checked However I set it to false instead, so it's unchecked by default when disabling locked teams.
  • Moved that optionLastManStanding to the optionWonderDuration.hidden part

gamesetup.xml

  • The hideMoreOptions button also needs to be moved 30 pixels down (though that error is corrected by resizeMoreOptionsWindow anyway) Apparently #3234 also forgot that.

EndGameManager.js:

  • let instead of var
  • added those functions to the protoype, so it's consistent and uses this instead of cmpEndGameManager
  • noAlliesLeftMessage -> lastManStandingMessage
  • Removed the two alliedVictory checks since we already have that early return ensuring the value above, thus end up with if-else,
  • moved the DeleteTimeNotification out of the if-statement since it's done in both cases
  • fixed indentation of AddTimeNotification
  • Removed unused cmpPlayerManager from LastManStandingCheck
  • Not really needed to have an array cmpPlayers, just using cmpPlayer (after swapping allies[0] and i when checking for IsMutualAlly)
  • The i == msg.playerId check is still wrong as you pass msg.playerId also if that player changed the diplomacy but wasn't defeated. It is also not needed for checking the defeated message as the playerstate is already "defeated" once the message arrives (found by testing)
  • Removed unused variable onlyAlliesLeft
  • Using cmpPlayerManager.GetNumPlayers() directly to get rid of numPlayers which is only used once
  • Last player remaining wins -> Last remaining player wins
  • Actually there is a bug with the current code - it should also do the LastManStandingCheck test on diplomacy change and on init too, otherwise it would wait until a player has resigned to decide whether the game is won due to only allies remaining. Thus the OnlyAlliesLeft can be merged with LastManStandingCheck (Yeah, could have noticed that before)
  • Furthermore for clarity I renamed "skip" to "skipAlliedVictoryCheck"
  • Added lastManStandingMessage to Init. not strictly required but I think it's nice to have all variables used defined there.
  • That cmpPlayers.filter loops over the players while you just did a whole loop before (in OnlyAlliesLeft in your patch). That check should be equivalent to allies.length == 1.

comment:10 by elexis, 8 years ago

In 18348:

Ensure in the simulation that locked teams and last man standing aren't enabled simultaneously. Suggested by leper, refs #3970.

comment:11 by elexis, 8 years ago

In 18441:

Win and defeat cleanup, fixes #4013.

Add a new simulation message and chat notification for players who won.
Avoid duplicate playerstate messages in the sim and GUI by triggering changes with a function instead of a message.
Reveal the map on defeat/win exclusively in the player component (instead of having a silly GUI proxy and doing it also in the EndGameManager sometimes).
Remove the skipAlliedVictory argument from the player component, since that shouldn't contain references to the EndGameManager, refs #3970.

Show a proper message box on win/loss and add the hint for hosts disconnecting other players.
Do defeat/win procedure in the GUI when such a message arrives, instead of checking onTick for playerstate changes.
Thus don't show that confirmation again on every rejoin.
Don't show a pointless message box if IsAtlasRunning.

Explain that the session.js variable is needed to avoid an order-of-execution bug.
Select "observer" item when rejoining as a defeated player.
Remove an unneeded call to updateTopPanel.

comment:12 by elexis, 8 years ago

In 18540:

Reset Last Man Standing flag when enabling rated games, to avoid the warn, refs #3970.

comment:13 by elexis, 8 years ago

In 18603:

Show the last-man-standing option regardless of whether it is possible to set it. Reviewed by fatherbushido, refs #3970.

Note: See TracTickets for help on using tickets.