Page MenuHomeWildfire Games

Show the servername in gamesetup
Needs ReviewPublic

Authored by Imarok on Sep 7 2017, 12:56 AM.
This revision needs review, but there are no reviewers specified.

Details

Reviewers
None
Summary

Show the server name in gamesetup and remove some unused code out of CNetServer.

Test Plan

Test the server name in multiplayer and lobby with clients. Also try changing username or servername before hosting.

Diff Detail

Repository
rP 0 A.D. Public Repository
Branch
https://svn.wildfiregames.com/public/ps/trunk
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 3102
Build 5361: Vulcan Build (Windows)Jenkins
Build 5360: Vulcan BuildJenkins
Build 5359: arc lint + arc unit

Event Timeline

Imarok created this revision.Sep 7 2017, 12:56 AM
Owners added subscribers: Restricted Owners Package, Restricted Owners Package.Sep 7 2017, 12:56 AM
Vulcan added a subscriber: Vulcan.Sep 7 2017, 2:28 AM

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...

http://jenkins-master:8080/job/phabricator/1987/ for more details.

Vulcan added a comment.Sep 7 2017, 2:29 AM
Executing section Default...
Executing section Source...

source/gui/scripting/ScriptFunctions.cpp
| 717| »   return·*(volatile·int*)0;
|    | [MAJOR] CPPCheckBear (nullPointer):
|    | Null pointer dereference
Executing section JS...
|    | [NORMAL] ESLintBear (no-else-return):
|    | Unnecessary 'else' after 'return'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup_mp/gamesetup_mp.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup_mp/gamesetup_mp.js
| 237| 237| 						g_IsRejoining = true;
| 238| 238| 						return; // we'll process the game setup messages in the next tick
| 239| 239| 					}
| 240|    |-					else
| 241|    |-					{
|    | 240|+					
| 242| 241| 						Engine.SwitchGuiPage("page_gamesetup.xml", {
| 243| 242| 							"type": g_GameType,
| 244| 243| 							"serverName": g_ServerName,
| 246| 245| 							"stunEndpoint": g_StunEndpoint
| 247| 246| 						});
| 248| 247| 						return; // don't process any more messages - leave them for the game GUI loop
| 249|    |-					}
|    | 248|+					
| 250| 249| 
| 251| 250| 				case "disconnected":
| 252| 251| 					cancelSetup();

binaries/data/mods/public/gui/gamesetup_mp/gamesetup_mp.js
| 155| »   while·(true)
|    | [NORMAL] ESLintBear (no-constant-condition):
|    | Unexpected constant condition.

binaries/data/mods/public/gui/gamesetup_mp/gamesetup_mp.js
| 249| »   »   »   »   »   }
|    | [NORMAL] JSHintBear:
|    | Expected a 'break' statement before 'case'.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1696| »   while·(g_IsNetworked)
|    | [NORMAL] ESLintBear (no-unmodified-loop-condition):
|    | 'g_IsNetworked' is not modified in this loop.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1515| »   »   »   »   if·(g_Settings.Biomes.every(bio·=>·bio.Id·!=·biome))
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1516| »   »   »   »   »   warn("Map·'"·+·g_GameAttributes.map·+·"'·contains·unknown·biome·'"·+·biome·+·"'")
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1678| »   if·(g_LoadingState·==·0)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1730| »   »   if·(playerData.some((pData,·j)·=>·i·!=·j·&&·sameColor(playerData[i].Color,·pData.Color)))
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1731| »   »   »   playerData[i].Color·=·g_PlayerColorPickerList.find(color·=>·playerData.every(pData·=>·!sameColor(color,·pData.Color)));
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1922| »   »   »   chosenCiv·=·pickRandom(Object.keys(g_CivData).filter(civ·=>·g_CivData[civ].Culture·==·culture));
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1936| »   »   let·usedName·=·g_GameAttributes.settings.PlayerData.filter(pData·=>·pData.Name·&&·pData.Name.indexOf(chosenName)·!==·-1).length;
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.
Executing section XML GUI...
|    | [INFO] XMLBear:
|    | XML can be formatted better.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup/gamesetup.xml
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup/gamesetup.xml
|   1|   1| <?xml version="1.0" encoding="utf-8"?>
|   2|    |-
|   3|   2| <objects>
|   4|   3| 
|   5|   4| 	<script file="gui/common/color.js"/>
|    | [INFO] XMLBear:
|    | XML can be formatted better.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup/gamesetup.xml
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup/gamesetup.xml
|  61|  61| 						<translatableAttribute id="caption">Civilization</translatableAttribute>
|  62|  62| 					</object>
|  63|  63| 
|  64|    |-					<object name="civInfoButton"
|  65|    |-						type="button"
|  66|    |-						style="IconButton"
|  67|    |-						sprite="iconInfoGold"
|  68|    |-						sprite_over="iconInfoWhite"
|  69|    |-						size="85%-37 0 85%-21 16"
|  70|    |-					>
|    |  64|+					<object name="civInfoButton" type="button" style="IconButton" sprite="iconInfoGold" sprite_over="iconInfoWhite" size="85%-37 0 85%-21 16">
|  71|  65| 						<translatableAttribute id="tooltip">View civilization info</translatableAttribute>
|  72|  66| 						<action on="Press"><![CDATA[
|  73|  67| 							Engine.PushGuiPage("page_civinfo.xml");
|    | [INFO] XMLBear:
|    | XML can be formatted better.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup/gamesetup.xml
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup/gamesetup.xml
|  74|  74| 						]]></action>
|  75|  75| 					</object>
|  76|  76| 
|  77|    |-					<object name="civResetButton"
|  78|    |-						type="button"
|  79|    |-						style="IconButton"
|  80|    |-						sprite="iconResetGold"
|  81|    |-						sprite_over="iconResetWhite"
|  82|    |-						size="85%-16 0 85% 16"
|  83|    |-					>
|    |  77|+					<object name="civResetButton" type="button" style="IconButton" sprite="iconResetGold" sprite_over="iconResetWhite" size="85%-16 0 85% 16">
|  84|  78| 						<translatableAttribute id="tooltip">Reset any civilizations that have been selected to the default (random)</translatableAttribute>
|  85|  79| 						<action on="Press">resetCivilizations();</action>
|  86|  80| 					</object>
|    | [INFO] XMLBear:
|    | XML can be formatted better.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup/gamesetup.xml
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup/gamesetup.xml
|  89|  89| 						<translatableAttribute id="caption">Team</translatableAttribute>
|  90|  90| 					</object>
|  91|  91| 
|  92|    |-					<object name="teamResetButton"
|  93|    |-						type="button"
|  94|    |-						style="IconButton"
|  95|    |-						sprite="iconResetGold"
|  96|    |-						sprite_over="iconResetWhite"
|  97|    |-						size="100%-21 0 100%-5 16"
|  98|    |-					>
|    |  92|+					<object name="teamResetButton" type="button" style="IconButton" sprite="iconResetGold" sprite_over="iconResetWhite" size="100%-21 0 100%-5 16">
|  99|  93| 						<translatableAttribute id="tooltip">Reset all teams to the default.</translatableAttribute>
| 100|  94| 						<action on="Press">resetTeams();</action>
| 101|  95| 					</object>
|    | [INFO] XMLBear:
|    | XML can be formatted better.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup/gamesetup.xml
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup/gamesetup.xml
| 113| 113| 								<translatableAttribute id="tooltip">Select player.</translatableAttribute>
| 114| 114| 							</object>
| 115| 115| 							<object name="playerAssignmentText[n]" type="text" style="ModernLabelText" size="22%+5 0 50%+35 30"/>
| 116|    |-							<object name="playerConfig[n]" type="button" style="StoneButton" size="50%+40 4 50%+64 28"
| 117|    |-								tooltip_style="onscreenToolTip"
| 118|    |-								font="sans-bold-stroke-12"
| 119|    |-								sprite="ModernGear"
| 120|    |-								sprite_over="ModernGearHover"
| 121|    |-								sprite_pressed="ModernGearPressed"
| 122|    |-							>
|    | 116|+							<object name="playerConfig[n]" type="button" style="StoneButton" size="50%+40 4 50%+64 28" tooltip_style="onscreenToolTip" font="sans-bold-stroke-12" sprite="ModernGear" sprite_over="Mode

http://jenkins-master:8080/job/phabricator_lint/502/ for more details.

bb added a subscriber: bb.Sep 7 2017, 3:30 PM

wouldn't it be better to use an input field so the host can change it while in gamesetup?

binaries/data/mods/public/gui/gamesetup/gamesetup.js
992

spaces

Imarok added a comment.EditedSep 7 2017, 3:32 PM
In D882#34487, @bb wrote:

wouldn't it be better to use an input field so the host can change it while in gamesetup?

That would require far more changes.
Though that could be subject of a future patch.

elexis added a subscriber: elexis.Sep 7 2017, 3:59 PM
elexis added inline comments.
binaries/data/mods/public/gui/gamesetup_mp/gamesetup_mp.js
31

make is a bit bold. Construct?
Don't you dare making this the first function of this file

113

Why is there a different code flow depending on being in the lobby?

329–331

ternary and one line per arg to avoid the increasing redundancy?

Imarok marked 4 inline comments as done.Sep 9 2017, 3:29 PM
Imarok added inline comments.
binaries/data/mods/public/gui/gamesetup_mp/gamesetup_mp.js
113

In the lobby you can change the servername.
In gamesetup it just takes the playername at init and creates the servername out of that. If the playername gets changed, it won't be updated. (without this code)

Imarok updated this revision to Diff 3613.EditedSep 9 2017, 3:29 PM
Imarok marked an inline comment as done.

Apply remarks

Vulcan added a comment.Sep 9 2017, 5:45 PM

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...

http://jenkins-master:8080/job/phabricator/2006/ for more details.

Vulcan added a comment.Sep 9 2017, 5:47 PM
Executing section Default...
Executing section Source...

source/gui/scripting/ScriptFunctions.cpp
| 717| »   return·*(volatile·int*)0;
|    | [MAJOR] CPPCheckBear (nullPointer):
|    | Null pointer dereference
Executing section JS...
|    | [NORMAL] ESLintBear (no-else-return):
|    | Unnecessary 'else' after 'return'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup_mp/gamesetup_mp.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup_mp/gamesetup_mp.js
| 237| 237| 						g_IsRejoining = true;
| 238| 238| 						return; // we'll process the game setup messages in the next tick
| 239| 239| 					}
| 240|    |-					else
| 241|    |-					{
|    | 240|+					
| 242| 241| 						Engine.SwitchGuiPage("page_gamesetup.xml", {
| 243| 242| 							"type": g_GameType,
| 244| 243| 							"serverName": g_ServerName,
| 246| 245| 							"stunEndpoint": g_StunEndpoint
| 247| 246| 						});
| 248| 247| 						return; // don't process any more messages - leave them for the game GUI loop
| 249|    |-					}
|    | 248|+					
| 250| 249| 
| 251| 250| 				case "disconnected":
| 252| 251| 					cancelSetup();

binaries/data/mods/public/gui/gamesetup_mp/gamesetup_mp.js
| 155| »   while·(true)
|    | [NORMAL] ESLintBear (no-constant-condition):
|    | Unexpected constant condition.

binaries/data/mods/public/gui/gamesetup_mp/gamesetup_mp.js
| 249| »   »   »   »   »   }
|    | [NORMAL] JSHintBear:
|    | Expected a 'break' statement before 'case'.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1691| »   while·(g_IsNetworked)
|    | [NORMAL] ESLintBear (no-unmodified-loop-condition):
|    | 'g_IsNetworked' is not modified in this loop.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1673| »   if·(g_LoadingState·==·0)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1725| »   »   if·(playerData.some((pData,·j)·=>·i·!=·j·&&·sameColor(playerData[i].Color,·pData.Color)))
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1726| »   »   »   playerData[i].Color·=·g_PlayerColorPickerList.find(color·=>·playerData.every(pData·=>·!sameColor(color,·pData.Color)));
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1917| »   »   »   chosenCiv·=·pickRandom(Object.keys(g_CivData).filter(civ·=>·g_CivData[civ].Culture·==·culture));
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1931| »   »   let·usedName·=·g_GameAttributes.settings.PlayerData.filter(pData·=>·pData.Name·&&·pData.Name.indexOf(chosenName)·!==·-1).length;
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.
Executing section XML GUI...
|    | [INFO] XMLBear:
|    | XML can be formatted better.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup/gamesetup.xml
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup/gamesetup.xml
|   1|   1| <?xml version="1.0" encoding="utf-8"?>
|   2|    |-
|   3|   2| <objects>
|   4|   3| 
|   5|   4| 	<script file="gui/common/color.js"/>
|    | [INFO] XMLBear:
|    | XML can be formatted better.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup/gamesetup.xml
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup/gamesetup.xml
|  61|  61| 						<translatableAttribute id="caption">Civilization</translatableAttribute>
|  62|  62| 					</object>
|  63|  63| 
|  64|    |-					<object name="civInfoButton"
|  65|    |-						type="button"
|  66|    |-						style="IconButton"
|  67|    |-						sprite="iconInfoGold"
|  68|    |-						sprite_over="iconInfoWhite"
|

http://jenkins-master:8080/job/phabricator_lint/508/ for more details.

elexis added inline comments.Dec 23 2017, 1:01 PM
binaries/data/mods/public/gui/gamesetup_mp/gamesetup_mp.js
31

ballsy :P

113

Why wouldn't you be able to chose the servername when hosting a game outside of the lobby if it's displayed outside of the lobby too?

331

-1 space

source/gui/scripting/ScriptFunctions.cpp
1085

JSInterface_Network.cpp

source/network/NetServer.cpp
1028

nucular separatus

elexis added inline comments.Dec 27 2017, 3:32 PM
binaries/data/mods/public/gui/gamesetup_mp/gamesetup_mp.js
113

As in should we make that pregame dialog more simple and always allow typing the gamename and never allow typing the playername (since that is selectable in the options?). Can be done afterwards I guess.