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 Visualization chat commands #13328

Merged
merged 2 commits into from Jul 23, 2017

Conversation

Projects
None yet
5 participants
@rob-v
Contributor

rob-v commented May 21, 2017

New visualization chat commands - showcombatgeometry, showdebuggeometry, showdepthpreview, showactortags - available also for spectators.

Show Unit Paths (DevPathDebug chat command) is in Visualization section, but is also player command, so not available for spectators.
Were already available: terrainoverlay, debugcustomterrain.

To enable these 4 options in Debug window, World needs VisualizationMode trait. To enable also chat commands, World needs also VisualizationCommands trait.

Closes #13252

@rob-v rob-v changed the title from Add Visualization chat commands showcombatgeometry, showactortags to IGNORE Add Visualization chat commands showcombatgeometry, showactortags May 21, 2017

@rob-v rob-v changed the title from IGNORE Add Visualization chat commands showcombatgeometry, showactortags to Add Visualization chat commands May 21, 2017

@rob-v

This comment has been minimized.

Show comment
Hide comment
@rob-v

rob-v May 21, 2017

Contributor

Updated - new PR description and implementation.

Contributor

rob-v commented May 21, 2017

Updated - new PR description and implementation.

@abcdefg30

You're removing fields from DeveloperModeInfo, but aren't re-adding them to VisualizationModeInfo. Is that intentional/wanted?

@rob-v

This comment has been minimized.

Show comment
Hide comment
@rob-v

rob-v May 21, 2017

Contributor

It is intentional, not sure if wanted. I think (almost) always either you want those visualization commands or not. IMO it isn't needed/wanted to disable some of the visualization commands, you simply don't use them.

Contributor

rob-v commented May 21, 2017

It is intentional, not sure if wanted. I think (almost) always either you want those visualization commands or not. IMO it isn't needed/wanted to disable some of the visualization commands, you simply don't use them.

@reaperrr

This comment has been minimized.

Show comment
Hide comment
@reaperrr

reaperrr Jul 21, 2017

Contributor

Needs rebase.

Contributor

reaperrr commented Jul 21, 2017

Needs rebase.

@rob-v

This comment has been minimized.

Show comment
Hide comment
@rob-v

rob-v Jul 22, 2017

Contributor

Rebased.

Contributor

rob-v commented Jul 22, 2017

Rebased.

@pchote

pchote approved these changes Jul 22, 2017

This works well. Just a couple of naming suggestions that I think would make this a bit tidier.

One really great side effect of this is that the core engine no longer depends on the DeveloperMode trait. Would you mind adding a commit that moves this to Mods.Common?

@rob-v

This comment has been minimized.

Show comment
Hide comment
@rob-v

rob-v Jul 23, 2017

Contributor

Updated.

Contributor

rob-v commented Jul 23, 2017

Updated.

@rob-v

This comment has been minimized.

Show comment
Hide comment
@rob-v

rob-v Jul 23, 2017

Contributor

Reverted VS formatting.

Contributor

rob-v commented Jul 23, 2017

Reverted VS formatting.

@obrakmann

Looks good otherwise, 👍

@@ -6,6 +6,7 @@
MusicPlaylist:
VictoryMusic: win1
DefeatMusic: nod_map1
DebugVisualizations:

This comment has been minimized.

@obrakmann

obrakmann Jul 23, 2017

Contributor

It would probably make sense to add this via an upgrade rule. What to you think?

@obrakmann

obrakmann Jul 23, 2017

Contributor

It would probably make sense to add this via an upgrade rule. What to you think?

This comment has been minimized.

@pchote

pchote Jul 23, 2017

Member

Probably more trouble than its worth to stop it from adding these to maps that override World.

@pchote

pchote Jul 23, 2017

Member

Probably more trouble than its worth to stop it from adding these to maps that override World.

@obrakmann obrakmann merged commit 97306f2 into OpenRA:bleed Jul 23, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@obrakmann

This comment has been minimized.

Show comment
Hide comment
@obrakmann
Contributor

obrakmann commented Jul 23, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment