Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add restart button to in-game menu for single player (skirmish, misison, replays) #16672

Merged
merged 3 commits into from Dec 28, 2019

Conversation

@dragunoff
Copy link
Contributor

dragunoff commented Jun 9, 2019

This PR:

  • Adds a "Restart" button to the ingame menu for single player (skirmish, misison, replays)
  • Removes the "Restart" button from the prompt for "Abort Mission" / "Leave" as it is redundant
  • Removes the "Surrender" button for single player games (the screenshots are outdated in this regard)

game-in-progress
Game in progress (skirmish, mission, replay)

game-over
Game over (skirmish, mission, replay)

restart-prompt
Prompt on "Restart" button

The second commit of this PR changes the behavior of "Leave" button when a game is over. The behavior was inconsistent because it did not prompt players that won the game but sent them straight out. I know this is a bit out of scope here and may be controversial but I was tempted to change it to make the behavior of all buttons ("Restart", "Surrender", "Leave") consistent (i.e. always prompt).

Closes #11318

@dragunoff

This comment has been minimized.

Copy link
Contributor Author

dragunoff commented Jun 9, 2019

Related issues for multiplayer games and replays: #15181, #14104

@pchote pchote added this to the Next Release milestone Aug 17, 2019
@pchote

This comment has been minimized.

Copy link
Member

pchote commented Aug 17, 2019

Scope creeping onto the milestone - this goes well (both code and feature wise) with the savegame changes.

@pchote

This comment has been minimized.

Copy link
Member

pchote commented Aug 18, 2019

Unfortunately this overflows our minimum supported window width (1024px) in the TD ingame menu in skirmish mode.

I suspect that the best fix here (which has been on my low priority TODO list for a long time now) will be to remove the Music button, and have the music options as a tab on the central panel instead.

This is too much for the playtest, so we'll have to defer this to Next + 1.

@pchote pchote modified the milestones: Next Release, Next+1 Aug 18, 2019
@dragunoff

This comment has been minimized.

Copy link
Contributor Author

dragunoff commented Aug 19, 2019

Unfortunately this overflows our minimum supported window width (1024px) in the TD ingame menu in skirmish mode.

Ah, that's a bummer... But I thought the minimum supported resolution is 1280x720 and this is what I tested on.

I suspect that the best fix here (which has been on my low priority TODO list for a long time now) will be to remove the Music button, and have the music options as a tab on the central panel instead.

You mean as a tab on the score/chat panel?

@reaperrr reaperrr mentioned this pull request Nov 17, 2019
@tovl

This comment has been minimized.

Copy link
Contributor

tovl commented Dec 8, 2019

I think we should just remove the surrender button for single player skirmish.

@dragunoff

This comment has been minimized.

Copy link
Contributor Author

dragunoff commented Dec 10, 2019

I think we should just remove the surrender button for single player skirmish.

I can get behind that. I can't think of a use case for the surrender button for single player other than for testing purposes but that can be achieved with a chat command.

@pchote

This comment has been minimized.

Copy link
Member

pchote commented Dec 10, 2019

I use the surrender button a lot while testing, but I guess I can live with the chat command for that use case.

@dragunoff

This comment has been minimized.

Copy link
Contributor Author

dragunoff commented Dec 10, 2019

I use the surrender button a lot while testing, but I guess I can live with the chat command for that use case.

Yeah, I use it a lot too. But if there is no other value in keeping it I think we should sacrifice it.

And with regards improving the usability of chat commands - an auto-complete sounds like a good addition (if it's not already there, haven't tried).

@abmyii

This comment has been minimized.

Copy link
Contributor

abmyii commented Dec 10, 2019

And with regards improving the usability of chat commands - an auto-complete sounds like a good addition (if it's not already there, haven't tried).

There is already one for player names, though it is a bit annoying when you want to toggle team/all chat - you have to erase everything (I believe pressing Home would also work though I haven't tested it). Perhaps we should change either toggle or autocomplete (Ctrl+Tab/Space?) to another key. I'll make a separate issue if it is worth it.

@abcdefg30

This comment has been minimized.

Copy link
Member

abcdefg30 commented Dec 10, 2019

Chat commands do support auto-completion.

@dragunoff dragunoff force-pushed the dragunoff:feature/restart-button-single-player branch from 36629b9 to 1249018 Dec 11, 2019
@dragunoff

This comment has been minimized.

Copy link
Contributor Author

dragunoff commented Dec 11, 2019

Update:

  • Remove "Surrender" button for single player games
  • Add a fixup commit making isSinglePlayer readonly (will squash when ready to merge)

The TD menu now fits in 1024px:
изображение

@abcdefg30

This comment has been minimized.

Copy link
Member

abcdefg30 commented Dec 12, 2019

Adding Fixup requested so we don't forget to squash the extra commit.

@dragunoff dragunoff force-pushed the dragunoff:feature/restart-button-single-player branch from 1249018 to b7fdda9 Dec 12, 2019
@dragunoff

This comment has been minimized.

Copy link
Contributor Author

dragunoff commented Dec 12, 2019

Squashed.

@pchote
pchote approved these changes Dec 28, 2019
Copy link
Member

pchote left a comment

LGTM, and since this is a relatively simple polish PR I'm going to take the liberty of picking this onto prep for the upcoming hotfix.

@pchote pchote modified the milestones: Next+1, Next Release Dec 28, 2019
@pchote pchote merged commit 3d1b4c2 into OpenRA:bleed Dec 28, 2019
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.