bitburner-official / bitburner-src

Bitburner source code.
Other
837 stars 273 forks source link

CODEBASE: Recheck all usages of typecasting with JSON.parse #1775

Closed catloversg closed 3 days ago

catloversg commented 1 week ago

While we search and fix lint errors, we (kind of) agree that we should not typecast the result of JSON.parse (e.g., JSON.parse(saveString, Reviver) as Something. This PR searches all references of JSON.parse and checks if they are typecasting in a questionable way. My guideline:

catloversg commented 1 week ago

In the latest commit, I refactored loadStaneksGift. There is a behavior change:

Rationale:

From now on, I'll use this code for other "loader" functions if I need to change them (I'm testing another PR that changes loadAllGangs):

let somethingData: unknown;
try {
  somethingData = JSON.parse(saveString, Reviver);
} catch (error) {
  console.error(error);
}
if (!validate(somethingData)) {
  console.error("Invalid somethingData:", saveString);
  somethingData = DefaultData;
  setTimeout(() => {
    dialogBoxCreate("Cannot load data of something. Something is reset.");
  }, 1000);
  return;
}
something = somethingData;

After #1774 is merged, with proper "loader" functions, we can gradually deal with legacy code in loadGame in a safe way.

catloversg commented 1 week ago

Unrelated question: We have src\utils\helpers\typeAssertion.ts and src\utils\TypeAssertion.ts. Can I merge them later in another PR (src\utils\helpers\typeAssertion.ts -> src\utils\TypeAssertion.ts)?

d0sboots commented 6 days ago

Unrelated question: We have src\utils\helpers\typeAssertion.ts and src\utils\TypeAssertion.ts. Can I merge them later in another PR (src\utils\helpers\typeAssertion.ts -> src\utils\TypeAssertion.ts)?

Please do, that's just silly. Unless there are circular dependency issues, but those should be solved by breaking individual functions out, not having two of the same file.

d0sboots commented 3 days ago

In testing this change with the old saves I have, I ran into this issue. I don't think this is a problem with this change directly, but rather an earlier undetected regression:

Error: WorldDaemon is not a normal server. This is a bug. Please contact developers. (at "Loading Screen")

test_save_0.47.2.json

d0sboots commented 3 days ago

I confirmed that it has the same issue on dev. So, an uncaught regression. This PR seems fine across all the test saves I have.