BUGFIX: Coding contracts may have duplicate names (#2399)

markdown/bitburner.codingcontract.createdummycontract.md

@@ -9,7 +9,7 @@ Generate a dummy contract.
 **Signature:**
 
 ```typescript
-createDummyContract(type: CodingContractName): string;
+createDummyContract(type: CodingContractName): string | null;
 ```
 
 ## Parameters
@@ -50,7 +50,7 @@ Type of contract to generate
 
 **Returns:**
 
-string
+string \| null
 
 Filename of the contract.
 
@@ -60,3 +60,5 @@ RAM cost: 2 GB
 
 Generate a dummy contract on the home computer with no reward. Used to test various algorithms.
 
+This function will return null and not generate a contract if the randomized contract name is the same as another contract's name.
+

src/CodingContract/ContractGenerator.ts

@@ -86,6 +86,9 @@ export function generateRandomContract(): void {
   const problemType = getRandomProblemType(maxDif);
 
   const contractFn = getRandomFilename(randServer, reward);
+  if (contractFn == null) {
+    return;
+  }
   const contract = new CodingContract(contractFn, problemType, reward);
 
   randServer.addContract(contract);
@@ -102,16 +105,22 @@ export function generateRandomContractOnHome(): void {
   const serv = Player.getHomeComputer();
 
   const contractFn = getRandomFilename(serv, reward);
+  if (contractFn == null) {
+    return;
+  }
   const contract = new CodingContract(contractFn, problemType, reward);
 
   serv.addContract(contract);
 }
 
-export const generateDummyContract = (problemType: CodingContractName): string => {
+export const generateDummyContract = (problemType: CodingContractName): string | null => {
   if (!CodingContractTypes[problemType]) throw new Error(`Invalid problem type: '${problemType}'`);
   const serv = Player.getHomeComputer();
 
   const contractFn = getRandomFilename(serv);
+  if (contractFn == null) {
+    return null;
+  }
   const contract = new CodingContract(contractFn, problemType, null);
   serv.addContract(contract);
 
@@ -152,7 +161,9 @@ export function generateContract(params: IGenerateContractParams): void {
   }
 
   const filename = params.fn ? params.fn : getRandomFilename(server, reward);
-
+  if (filename == null) {
+    return;
+  }
   const contract = new CodingContract(filename, problemType, reward);
   server.addContract(contract);
 }
@@ -252,28 +263,34 @@ function getRandomServer(): BaseServer | null {
   return randServer;
 }
 
-function getRandomFilename(
+function getRandomAlphanumericString(length: number) {
+  const alphanumericChars = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789";
+  let result = "";
+  for (let i = 0; i < length; ++i) {
+    result += alphanumericChars.charAt(Math.random() * alphanumericChars.length);
+  }
+  return result;
+}
+
+/**
+ * This function will return null if the randomized name collides with another contract's name on the specified server.
+ * Callers of this function must return early and not generate a contract when it happens. It likely happens when there
+ * are ~240k contracts on the specified server.
+ */
+export function getRandomFilename(
   server: BaseServer,
   reward: ICodingContractReward = { type: CodingContractRewardType.Money },
-): ContractFilePath {
+): ContractFilePath | null {
-  let contractFn = `contract-${getRandomIntInclusive(0, 1e6)}`;
+  let contractFn = `contract-${getRandomAlphanumericString(6)}`;
-
-  for (let i = 0; i < 1000; ++i) {
-    if (
-      server.contracts.filter((c: CodingContract) => {
-        return c.fn === contractFn;
-      }).length <= 0
-    ) {
-      break;
-    }
-    contractFn = `contract-${getRandomIntInclusive(0, 1e6)}`;
-  }
-
   if ("name" in reward) {
     // Only alphanumeric characters in the reward name.
     contractFn += `-${reward.name.replace(/[^a-zA-Z0-9]/g, "")}`;
   }
   contractFn += ".cct";
+  // Return null if there is a contract with the same name.
+  if (server.contracts.filter((c: CodingContract) => c.fn === contractFn).length) {
+    return null;
+  }
   const validatedPath = resolveContractFilePath(contractFn);
   if (!validatedPath) throw new Error(`Generated contract path could not be validated: ${contractFn}`);
   return validatedPath;

src/ScriptEditor/NetscriptDefinitions.d.ts

@@ -4059,10 +4059,12 @@ export interface CodingContract {
    *
    * Generate a dummy contract on the home computer with no reward. Used to test various algorithms.
    *
+   * This function will return null and not generate a contract if the randomized contract name is the same as another contract's name.
+   *
    * @param type - Type of contract to generate
    * @returns Filename of the contract.
    */
-  createDummyContract(type: CodingContractName): string;
+  createDummyContract(type: CodingContractName): string | null;
 
   /**
    * List all contract types.

src/utils/APIBreaks/3.0.0.ts

@@ -498,5 +498,13 @@ export const breakingChanges300: VersionBreakingChange = {
         'It has been automatically replaced with "ns.getBitNodeMultipliers().CloudServerMaxRam".',
       showWarning: false,
     },
+    {
+      brokenAPIs: [{ name: "createDummyContract" }],
+      info:
+        "ns.codingcontract.createDummyContract might generate a contract with the same name of another contract.\n" +
+        "This bug was fixed. Now this function will return null and not generate a contract if the randomized contract " +
+        "name is the same as another contract's name.",
+      showWarning: false,
+    },
   ],
 };

test/jest/CodingContract/ContractGenerator.test.ts

@@ -0,0 +1,76 @@
+import { Player } from "@player";
+import {
+  generateContract,
+  generateDummyContract,
+  generateRandomContract,
+  generateRandomContractOnHome,
+  getRandomFilename,
+} from "../../../src/CodingContract/ContractGenerator";
+import { CodingContractName } from "../../../src/Enums";
+import { GetAllServers } from "../../../src/Server/AllServers";
+import type { BaseServer } from "../../../src/Server/BaseServer";
+import { SpecialServers } from "../../../src/Server/data/SpecialServers";
+import { initGameEnvironment, setupBasicTestingEnvironment } from "../Utilities";
+
+beforeAll(() => {
+  initGameEnvironment();
+});
+
+beforeEach(() => {
+  setupBasicTestingEnvironment();
+  assertNumberOfContracts(0, GetAllServers());
+});
+
+function assertNumberOfContracts(expected: number, servers: BaseServer[]): void {
+  expect(servers.reduce((sum, server) => sum + server.contracts.length, 0)).toStrictEqual(expected);
+}
+
+describe("Generator", () => {
+  test("generateRandomContract", () => {
+    generateRandomContract();
+    assertNumberOfContracts(1, GetAllServers());
+  });
+  test("generateRandomContractOnHome", () => {
+    generateRandomContractOnHome();
+    assertNumberOfContracts(1, [Player.getHomeComputer()]);
+  });
+  test("generateDummyContract", () => {
+    generateDummyContract(CodingContractName.FindLargestPrimeFactor);
+    assertNumberOfContracts(1, GetAllServers());
+  });
+  // generateContract is flexible. All properties in IGenerateContractParams are optional. This test checks the usage in
+  // CodingContractsDev.tsx.
+  test("generateContract - home", () => {
+    generateContract({ problemType: CodingContractName.FindLargestPrimeFactor, server: SpecialServers.Home });
+    assertNumberOfContracts(1, [Player.getHomeComputer()]);
+  });
+  // This test checks the case in which we randomize everything (e.g., problemType, server).
+  test("generateContract - random server", () => {
+    generateContract({});
+    assertNumberOfContracts(1, GetAllServers());
+  });
+});
+
+describe("getRandomFilename", () => {
+  test("Check format and collision", () => {
+    const server = Player.getHomeComputer();
+    const set = new Set();
+    // Contract names contain only alphanumeric chars.
+    const regex = /[^a-zA-Z0-9]/g;
+    const maxIter = 1000;
+    for (let i = 0; i < maxIter; ++i) {
+      const filename = getRandomFilename(server);
+      if (filename == null) {
+        throw new Error("Cannot generate random filename");
+      }
+      expect(filename).toMatch(regex);
+      set.add(filename);
+    }
+    // getRandomFilename had a bug that made the filenames collide much easier than what we expected. Please check
+    // https://github.com/bitburner-official/bitburner-src/pull/2399 for more information.
+    // This test is designed to catch that kind of bug. The probability of collision is greater than zero, so this test
+    // may give a false positive. However, the probability is very low (0.000008802741646607437), and the error log
+    // should point that out immediately.
+    expect(set.size).toStrictEqual(maxIter);
+  });
+});