Skip to content

Commit df29ca4

Browse files
SadPencilSadPencil
authored
Avoid the abuse of random generator initialization (CnCNet#728)
* Share the use of Random class * Throw on too many attempts in randomize channel name
1 parent a77dc0f commit df29ca4

File tree

11 files changed

+79
-41
lines changed

11 files changed

+79
-41
lines changed

‎DXMainClient/DXGUI/GameClass.cs‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,15 @@ protected override void Initialize()
244244
(wm.RenderResolutionY - ls.Height) / 2, ls.Width, ls.Height);
245245
}
246246

247+
private static Random GetRandom()
248+
{
249+
var rng = System.Security.Cryptography.RandomNumberGenerator.Create();
250+
byte[] intBytes = new byte[sizeof(int)];
251+
rng.GetBytes(intBytes);
252+
int seed = BitConverter.ToInt32(intBytes, 0);
253+
return new Random(seed);
254+
}
255+
247256
private IServiceProvider BuildServiceProvider(WindowManager windowManager)
248257
{
249258
// Create host - this allows for things like DependencyInjection
@@ -261,7 +270,8 @@ private IServiceProvider BuildServiceProvider(WindowManager windowManager)
261270
.AddSingleton<TunnelHandler>()
262271
.AddSingleton<DiscordHandler>()
263272
.AddSingleton<PrivateMessageHandler>()
264-
.AddSingleton<MapLoader>();
273+
.AddSingleton<MapLoader>()
274+
.AddSingleton<Random>(GetRandom());
265275

266276
// singleton xna controls - same instance on each request
267277
services

‎DXMainClient/DXGUI/Generic/LoadingScreen.cs‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,18 +24,22 @@ public LoadingScreen(
2424
CnCNetManager cncnetManager,
2525
WindowManager windowManager,
2626
IServiceProvider serviceProvider,
27-
MapLoader mapLoader
27+
MapLoader mapLoader,
28+
Random random
2829
) : base(windowManager)
2930
{
3031
this.cncnetManager = cncnetManager;
3132
this.serviceProvider = serviceProvider;
3233
this.mapLoader = mapLoader;
34+
this.random = random;
3335
}
3436

3537
private static readonly object locker = new object();
3638

3739
private MapLoader mapLoader;
3840

41+
private Random random;
42+
3943
private PrivateMessagingPanel privateMessagingPanel;
4044

4145
private bool visibleSpriteCursor;
@@ -87,7 +91,7 @@ protected override void GetINIAttributes(IniFile iniFile)
8791
if (randomTextures.Count == 0)
8892
return;
8993

90-
BackgroundTexture = AssetLoader.LoadTexture(randomTextures[new Random().Next(randomTextures.Count)]);
94+
BackgroundTexture = AssetLoader.LoadTexture(randomTextures[random.Next(randomTextures.Count)]);
9195
}
9296

9397
private void InitUpdater()

‎DXMainClient/DXGUI/Multiplayer/CnCNet/CnCNetLobby.cs‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ public CnCNetLobby(WindowManager windowManager, CnCNetManager connectionManager,
3939
CnCNetGameLobby gameLobby, CnCNetGameLoadingLobby gameLoadingLobby,
4040
TopBar topBar, PrivateMessagingWindow pmWindow, TunnelHandler tunnelHandler,
4141
GameCollection gameCollection, CnCNetUserData cncnetUserData,
42-
OptionsWindow optionsWindow, MapLoader mapLoader)
42+
OptionsWindow optionsWindow, MapLoader mapLoader, Random random)
4343
: base(windowManager)
4444
{
4545
this.connectionManager = connectionManager;
@@ -52,6 +52,7 @@ public CnCNetLobby(WindowManager windowManager, CnCNetManager connectionManager,
5252
this.cncnetUserData = cncnetUserData;
5353
this.optionsWindow = optionsWindow;
5454
this.mapLoader = mapLoader;
55+
this.random = random;
5556

5657
ctcpCommandHandlers = new CommandHandlerBase[]
5758
{
@@ -141,6 +142,8 @@ public CnCNetLobby(WindowManager windowManager, CnCNetManager connectionManager,
141142

142143
private GameFiltersPanel panelGameFilters;
143144

145+
private Random random;
146+
144147
private void GameList_ClientRectangleUpdated(object sender, EventArgs e)
145148
{
146149
panelGameFilters.ClientRectangle = lbGameList.ClientRectangle;
@@ -1118,13 +1121,16 @@ private void GameLoadingChannel_UserAdded(object sender, ChannelUserEventArgs e)
11181121
/// <returns>A random channel name based on the currently played game.</returns>
11191122
private string RandomizeChannelName()
11201123
{
1121-
while (true)
1124+
int maxTries = 10000;
1125+
for (int i = 0; i < maxTries; i++)
11221126
{
1123-
string channelName = gameCollection.GetGameChatChannelNameFromIdentifier(localGameID) + "-game" + new Random().Next(1000000, 9999999);
1127+
string channelName = gameCollection.GetGameChatChannelNameFromIdentifier(localGameID) + "-game" + random.Next(1000000, 9999999);
11241128
int index = lbGameList.HostedGames.FindIndex(c => ((HostedCnCNetGame)c).ChannelName == channelName);
11251129
if (index == -1)
11261130
return channelName;
11271131
}
1132+
1133+
throw new Exception(string.Format("Could not find a random channel name after {0} retries", maxTries));
11281134
}
11291135

11301136
private void Gcw_Cancelled(object sender, EventArgs e) => gameCreationPanel.Hide();

‎DXMainClient/DXGUI/Multiplayer/GameLobby/CnCNetGameLobby.cs‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,15 +50,17 @@ public CnCNetGameLobby(
5050
CnCNetUserData cncnetUserData,
5151
MapLoader mapLoader,
5252
DiscordHandler discordHandler,
53-
PrivateMessagingWindow pmWindow
54-
) : base(windowManager, "MultiplayerGameLobby", topBar, mapLoader, discordHandler, pmWindow)
53+
PrivateMessagingWindow pmWindow,
54+
Random random
55+
) : base(windowManager, "MultiplayerGameLobby", topBar, mapLoader, discordHandler, pmWindow, random)
5556
{
5657
this.connectionManager = connectionManager;
5758
localGame = ClientConfiguration.Instance.LocalGame;
5859
this.tunnelHandler = tunnelHandler;
5960
this.gameCollection = gameCollection;
6061
this.cncnetUserData = cncnetUserData;
6162
this.pmWindow = pmWindow;
63+
this.random = random;
6264

6365
ctcpCommandHandlers = new CommandHandlerBase[]
6466
{
@@ -144,6 +146,8 @@ PrivateMessagingWindow pmWindow
144146

145147
private MapSharingConfirmationPanel mapSharingConfirmationPanel;
146148

149+
private Random random;
150+
147151
/// <summary>
148152
/// The SHA1 of the latest selected map.
149153
/// Used for map sharing.
@@ -238,7 +242,7 @@ public void SetUp(Channel channel, bool isHost, int playerLimit,
238242

239243
if (isHost)
240244
{
241-
RandomSeed = new Random().Next();
245+
RandomSeed = random.Next();
242246
RefreshMapSelectionUI();
243247
btnChangeTunnel.Enable();
244248
}
@@ -1249,7 +1253,7 @@ protected void ResetGameState()
12491253

12501254
if (IsHost)
12511255
{
1252-
RandomSeed = new Random().Next();
1256+
RandomSeed = random.Next();
12531257
OnGameOptionChanged();
12541258
ClearReadyStatuses();
12551259
CopyPlayerDataToUI();

‎DXMainClient/DXGUI/Multiplayer/GameLobby/GameLobbyBase.cs‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -55,17 +55,21 @@ public GameLobbyBase(
5555
string iniName,
5656
MapLoader mapLoader,
5757
bool isMultiplayer,
58-
DiscordHandler discordHandler
58+
DiscordHandler discordHandler,
59+
Random random
5960
) : base(windowManager)
6061
{
6162
_iniSectionName = iniName;
6263
MapLoader = mapLoader;
6364
this.isMultiplayer = isMultiplayer;
6465
this.discordHandler = discordHandler;
66+
this.random = random;
6567
}
6668

6769
private string _iniSectionName;
6870

71+
private Random random;
72+
6973
protected XNAPanel PlayerOptionsPanel;
7074

7175
protected List<MultiplayerColor> MPColors;
@@ -747,10 +751,10 @@ private void PickRandomMap()
747751
if (maps.Count < 1)
748752
return;
749753

750-
int random = new Random().Next(0, maps.Count);
754+
int randomValue = random.Next(0, maps.Count);
751755
bool isFavoriteMapsSelected = IsFavoriteMapsSelected();
752-
GameModeMap = GameModeMaps.Find(gmm => (gmm.GameMode == GameMode || gmm.IsFavorite && isFavoriteMapsSelected) && gmm.Map == maps[random]);
753-
Logger.Log("PickRandomMap: Rolled " + random + " out of " + maps.Count + ". Picked map: " + Map.Name);
756+
GameModeMap = GameModeMaps.Find(gmm => (gmm.GameMode == GameMode || gmm.IsFavorite && isFavoriteMapsSelected) && gmm.Map == maps[randomValue]);
757+
Logger.Log("PickRandomMap: Rolled " + randomValue + " out of " + maps.Count + ". Picked map: " + Map.Name);
754758

755759
ChangeMap(GameModeMap);
756760
tbMapSearch.Text = string.Empty;
@@ -1333,7 +1337,7 @@ protected virtual PlayerHouseInfo[] Randomize(List<TeamStartMapping> teamStartMa
13331337

13341338
// Randomize options
13351339

1336-
Random random = new Random(RandomSeed);
1340+
Random pseudoRandom = new Random(RandomSeed);
13371341

13381342
for (int i = 0; i < totalPlayerCount; i++)
13391343
{
@@ -1352,10 +1356,10 @@ protected virtual PlayerHouseInfo[] Randomize(List<TeamStartMapping> teamStartMa
13521356
disallowedSides = GetDisallowedSidesForGroup(forHumanPlayers:false);
13531357
}
13541358

1355-
pHouseInfo.RandomizeSide(pInfo, SideCount, random, disallowedSides, RandomSelectors, RandomSelectorCount);
1359+
pHouseInfo.RandomizeSide(pInfo, SideCount, pseudoRandom, disallowedSides, RandomSelectors, RandomSelectorCount);
13561360

1357-
pHouseInfo.RandomizeColor(pInfo, freeColors, MPColors, random);
1358-
pHouseInfo.RandomizeStart(pInfo, random, freeStartingLocations, takenStartingLocations, teamStartMappings.Any());
1361+
pHouseInfo.RandomizeColor(pInfo, freeColors, MPColors, pseudoRandom);
1362+
pHouseInfo.RandomizeStart(pInfo, pseudoRandom, freeStartingLocations, takenStartingLocations, teamStartMappings.Any());
13591363
}
13601364

13611365
return houseInfos;

‎DXMainClient/DXGUI/Multiplayer/GameLobby/LANGameLobby.cs‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,8 @@ public class LANGameLobby : MultiplayerGameLobby
4444
public const string PING = "PING";
4545

4646
public LANGameLobby(WindowManager windowManager, string iniName,
47-
TopBar topBar, LANColor[] chatColors, MapLoader mapLoader, DiscordHandler discordHandler, PrivateMessagingWindow pmWindow) :
48-
base(windowManager, iniName, topBar, mapLoader, discordHandler, pmWindow)
47+
TopBar topBar, LANColor[] chatColors, MapLoader mapLoader, DiscordHandler discordHandler, PrivateMessagingWindow pmWindow, Random random) :
48+
base(windowManager, iniName, topBar, mapLoader, discordHandler, pmWindow, random)
4949
{
5050
this.chatColors = chatColors;
5151
encoding = Encoding.UTF8;
@@ -77,6 +77,8 @@ public LANGameLobby(WindowManager windowManager, string iniName,
7777
localGame = ClientConfiguration.Instance.LocalGame;
7878

7979
WindowManager.GameClosing += WindowManager_GameClosing;
80+
81+
this.random = random;
8082
}
8183

8284
private void WindowManager_GameClosing(object sender, EventArgs e)
@@ -123,6 +125,8 @@ private void HandleFileHashCommand(string sender, string fileHash)
123125

124126
private string localFileHash;
125127

128+
private Random random;
129+
126130
public override void Initialize()
127131
{
128132
IniNameOverride = nameof(LANGameLobby);
@@ -139,7 +143,7 @@ public void SetUp(bool isHost,
139143

140144
if (isHost)
141145
{
142-
RandomSeed = new Random().Next();
146+
RandomSeed = random.Next();
143147
Thread thread = new Thread(ListenForClients);
144148
thread.Start();
145149

@@ -669,7 +673,7 @@ protected override void GameProcessExited()
669673

670674
if (IsHost)
671675
{
672-
RandomSeed = new Random().Next();
676+
RandomSeed = random.Next();
673677
OnGameOptionChanged();
674678
ClearReadyStatuses();
675679
CopyPlayerDataToUI();

‎DXMainClient/DXGUI/Multiplayer/GameLobby/MultiplayerGameLobby.cs‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,11 @@ public abstract class MultiplayerGameLobby : GameLobbyBase, ISwitchable
2828
private const int MAX_DIE_SIDES = 100;
2929

3030
public MultiplayerGameLobby(WindowManager windowManager, string iniName,
31-
TopBar topBar, MapLoader mapLoader, DiscordHandler discordHandler, PrivateMessagingWindow pmWindow)
32-
: base(windowManager, iniName, mapLoader, true, discordHandler)
31+
TopBar topBar, MapLoader mapLoader, DiscordHandler discordHandler, PrivateMessagingWindow pmWindow, Random random)
32+
: base(windowManager, iniName, mapLoader, true, discordHandler, random)
3333
{
3434
TopBar = topBar;
35+
this.random = random;
3536

3637
chatBoxCommands = new List<ChatBoxCommand>
3738
{
@@ -61,6 +62,8 @@ public MultiplayerGameLobby(WindowManager windowManager, string iniName,
6162
protected XNAClientButton btnLockGame;
6263
protected XNAClientCheckBox chkAutoReady;
6364

65+
private Random random;
66+
6467
protected bool IsHost = false;
6568

6669
private bool locked = false;
@@ -522,7 +525,6 @@ private void RollDiceCommand(string dieType)
522525
}
523526

524527
int[] results = new int[dieCount];
525-
Random random = new Random();
526528
for (int i = 0; i < dieCount; i++)
527529
{
528530
results[i] = random.Next(1, dieSides + 1);

‎DXMainClient/DXGUI/Multiplayer/GameLobby/SkirmishLobby.cs‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,21 +19,24 @@ public class SkirmishLobby : GameLobbyBase, ISwitchable
1919
{
2020
private const string SETTINGS_PATH = "Client/SkirmishSettings.ini";
2121

22-
public SkirmishLobby(WindowManager windowManager, TopBar topBar, MapLoader mapLoader, DiscordHandler discordHandler)
23-
: base(windowManager, "SkirmishLobby", mapLoader, false, discordHandler)
22+
public SkirmishLobby(WindowManager windowManager, TopBar topBar, MapLoader mapLoader, DiscordHandler discordHandler, Random random)
23+
: base(windowManager, "SkirmishLobby", mapLoader, false, discordHandler, random)
2424
{
2525
this.topBar = topBar;
26+
this.random = random;
2627
}
2728

2829
public event EventHandler Exited;
2930

31+
private Random random;
32+
3033
TopBar topBar;
3134

3235
public override void Initialize()
3336
{
3437
base.Initialize();
3538

36-
RandomSeed = new Random().Next();
39+
RandomSeed = random.Next();
3740

3841
//InitPlayerOptionDropdowns(128, 98, 90, 48, 55, new Point(6, 24));
3942
InitPlayerOptionDropdowns();
@@ -229,7 +232,7 @@ protected override void GameProcessExited()
229232

230233
DdGameModeMapFilter_SelectedIndexChanged(null, EventArgs.Empty); // Refresh ranks
231234

232-
RandomSeed = new Random().Next();
235+
RandomSeed = random.Next();
233236
}
234237

235238
public void Open()

‎DXMainClient/DXGUI/Multiplayer/LANLobby.cs‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,16 +39,20 @@ public LANLobby(
3939
WindowManager windowManager,
4040
GameCollection gameCollection,
4141
MapLoader mapLoader,
42-
DiscordHandler discordHandler
42+
DiscordHandler discordHandler,
43+
Random random
4344
) : base(windowManager)
4445
{
4546
this.gameCollection = gameCollection;
4647
this.mapLoader = mapLoader;
4748
this.discordHandler = discordHandler;
49+
this.random = random;
4850
}
4951

5052
public event EventHandler Exited;
5153

54+
private Random random;
55+
5256
XNAListBox lbPlayerList;
5357
ChatListBox lbChatMessages;
5458
GameListBox lbGameList;
@@ -239,7 +243,7 @@ public override void Initialize()
239243
gameCreationPanel.SetPositionAndSize();
240244

241245
lanGameLobby = new LANGameLobby(WindowManager, "MultiplayerGameLobby",
242-
null, chatColors, mapLoader, discordHandler, pmWindow);
246+
null, chatColors, mapLoader, discordHandler, pmWindow, random);
243247
DarkeningPanel.AddAndInitializeWithControl(WindowManager, lanGameLobby);
244248
lanGameLobby.Disable();
245249

‎DXMainClient/Online/CnCNetManager.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,11 +46,11 @@ public class CnCNetManager : IConnectionManager
4646
public event EventHandler<UserNameIndexEventArgs> UserRemoved;
4747
public event EventHandler MultipleUsersAdded;
4848

49-
public CnCNetManager(WindowManager wm, GameCollection gc, CnCNetUserData cncNetUserData)
49+
public CnCNetManager(WindowManager wm, GameCollection gc, CnCNetUserData cncNetUserData, Random random)
5050
{
5151
gameCollection = gc;
5252
this.cncNetUserData = cncNetUserData;
53-
connection = new Connection(this);
53+
connection = new Connection(this, random);
5454

5555
this.wm = wm;
5656

0 commit comments

Comments
 (0)