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

Host can change Team and Spawn of any player. #12936 #13002

Merged
merged 1 commit into from May 4, 2017
Merged

Host can change Team and Spawn of any player. #12936 #13002

merged 1 commit into from May 4, 2017

Conversation

rob-v
Copy link
Contributor

@rob-v rob-v commented Mar 20, 2017

Host can change Team and Spawn of any player to make it simpler and quicker to setup new game.

@@ -25,6 +25,9 @@ namespace OpenRA.Mods.Common.Widgets.Logic
{
public class LobbyLogic : ChromeLogic
{
const string TeamDropdownWidgetName = "TEAM_DROPDOWN";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Defining these as constants is inconsistent with the rest of this file (and all of the other chrome logic), so this actually hurts readability. Please keep these inline.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

constants removed

dropdown.IsDisabled = () => s.LockTeam || orderManager.LocalClient.IsReady;
dropdown.OnMouseDown = _ => ShowTeamDropDown(dropdown, c, orderManager, map.PlayerCount);
dropdown.GetText = () => (c.Team == 0) ? "-" : c.Team.ToString();

if (widgetId != "TEAM")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can then drop these checks.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if I should take into account external mods in this phase. I would make it simpler if I didn't want to keep compatibility with external mods.

I think that check is needed - when it is called for host - EditableTemplate, I don't want to hide it. It is hidden only when it is called from NonEditableTemplate with TEAM_DROPDOWN. The TEAM is in boh sections as Label and as Dropdown. I didn't rename it for compatibility.

Copy link
Member

@pchote pchote Mar 20, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SetupEditableTeamWidget should only ever act on the TEAM widget.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The chrome logic classes are intimately tied with the yaml, so backwards compatibility is not a goal.

@rob-v
Copy link
Contributor Author

rob-v commented Mar 21, 2017

Updated, we don't need backward compatibility, so it is now simpler.

@@ -723,8 +724,16 @@ void UpdatePlayerList()
() => panel = PanelType.Kick, () => panel = PanelType.Players);
LobbyUtils.SetupColorWidget(template, slot, client);
LobbyUtils.SetupFactionWidget(template, slot, client, factions);
LobbyUtils.SetupTeamWidget(template, slot, client);
LobbyUtils.SetupSpawnWidget(template, slot, client);
if (isHost) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should move the brace to a new line.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

Copy link
Member

@pchote pchote left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works as advertised on all four default mods. Just one small fix to do for future compatibility:

dropdown.IsDisabled = () => s.LockTeam || orderManager.LocalClient.IsReady;
dropdown.OnMouseDown = _ => ShowTeamDropDown(dropdown, c, orderManager, map.PlayerCount);
dropdown.GetText = () => (c.Team == 0) ? "-" : c.Team.ToString();

HideChildWidget(parent, "TEAM");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You will also need to hide TEAM_DROPDOWN in SetupTeamWidget in case the template is reused when the player surrenders admin (as a future feature).

@@ -450,6 +450,15 @@ public static void SetupEditableSpawnWidget(Widget parent, Session.Slot s, Sessi
ShowSpawnDropDown(dropdown, c, orderManager, spawnPoints);
};
dropdown.GetText = () => (c.SpawnPoint == 0) ? "-" : Convert.ToChar('A' - 1 + c.SpawnPoint).ToString();

HideChildWidget(parent, "SPAWN");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likewise for SPAWN_DROPDOWN.

@rob-v
Copy link
Contributor Author

rob-v commented Apr 23, 2017

Updated

@@ -666,6 +666,7 @@ void UpdatePlayerList()
if (orderManager.LocalClient == null)
return;

var isHost = Game.IsHost;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you copying this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not changing, used in loop so retrieved only once.

@pchote pchote added this to the Next Release milestone Apr 30, 2017
@reaperrr
Copy link
Contributor

reaperrr commented May 4, 2017

👍

@reaperrr reaperrr merged commit ea29cce into OpenRA:bleed May 4, 2017
@reaperrr
Copy link
Contributor

reaperrr commented May 4, 2017

changelog

@rob-v rob-v deleted the LobbyOptionsEditableByHost branch May 4, 2017 17:26
@reaperrr reaperrr modified the milestones: Next Release, Next + 1 Sep 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants