API: Added external api which returns gainExperience #1955

AdamAndreatta · 2025-02-09T07:11:27Z

Created a function to provide expGain to the getmemberinfo api

Adds the total experience gained based on cycles presented, then returns all of those values in a list.

… each stat.

catloversg

You also need to build and test it. I'm pretty sure it does not work in the current state due to the lack of RAM cost information in src\Netscript\RamCostGenerator.ts.

catloversg · 2025-02-09T14:51:01Z

src/Gang/formulas/formulas.ts

@@ -79,3 +80,62 @@ export function calculateAscensionPointsGain(exp: number): number {
 export function calculateAscensionMult(points: number): number {
  return Math.max(Math.pow(points / 2000, 0.5), 1);
 }
+
+//FUNCTION RETURNS TOTAL EXP GAINED FOR EACH STAT BASED ON THE PARAMS PROVIDED CYCLES, MEMBER, AND TASK


Don't write all-caps comments. It's very hard to read.

catloversg · 2025-02-09T14:57:03Z

src/Gang/formulas/formulas.ts

+//RETURNS A LIST OF THOSE TOTAL VALUES IN THIS ORDER [hackExp, strExp, defExp, dexExp, agiExp, chaExp]
+//IF THERE ARE NO TASKS ASSIGNED EMPTY ARRAY IS RETURNED
+export function getGainExperience(numCycles: number, member: GangMember): number[] | string {
+  const task = member.getTask();


Most parts of this function duplicate code of gainExperience in src\Gang\GangMember.ts. You need to refactor gainExperience in a way that we can reuse it.

catloversg · 2025-02-09T14:59:11Z

src/ScriptEditor/NetscriptDefinitions.d.ts

@@ -1,5 +1,7 @@
 /** All netscript definitions */

+import { GangMember } from "src/Gang/GangMember";


Don't import anything to NetscriptDefinitions.d.ts. I get that you do this due to how you design getGainExperience, but that API needs to be designed in a different way. More about this later.

catloversg · 2025-02-09T15:30:55Z

src/ScriptEditor/NetscriptDefinitions.d.ts

+   *List is returned in this exact order  [hackExp, strExp, defExp, dexExp, agiExp, chaExp]
+   *if a gang member is passed with no current tasks assigned will return a string that says that their is no task currently assigned
+   */
+  getGainExperience(numCycles: number, member: GangMember): number[] | string


There are many problems with this API:

It requires GangMember, but GangMember is our internal data structure. The player does not have access to this type. We expose GangMemberInfo for member's info.

number[] | string is a very bad type for the result. You are returning a string to tell the player that the input/current condition is invalid. Never do this. You should return null/undefined or throw an error.

It may be better to return an object containing relevant information instead of an array.

RAM cost for this kind of API should not be 0.

You also need to:

Validate user input.

Add RAM cost information in src\Netscript\RamCostGenerator.ts.

You can check how we implement other APIs for Gang for an example.

I have a suggestion for you: Don't add a new API, just reuse getMemberInformation. Your API returns data for a member, but we already have an API for it (getMemberInformation). That API already returns "gain" for many things: respectGain, wantedLevelGain, moneyGain. It's totally okay to add expGain. By doing this, you only need to refactor gainExperience and skip many things required for a new API.

Sounds good that should be more than doable I will make those adjustments and run the tests again

AdamAndreatta · 2025-02-10T04:22:37Z

Finished to adjust of the function used your guy's already existing API to send the data to the user. Made a get function though since refactoring felt it would be more volatile since that function is also how members receive their experience in the function. I ended up returning an object of the exp gained from a cycle for each stat.

catloversg · 2025-02-10T10:13:13Z

src/Gang/GangMember.ts

@@ -146,46 +147,65 @@ export class GangMember {
      cha: (this.cha_mult - 1) / 4 + 1,
    };
  }
-
-  gainExperience(numCycles = 1): void {
+  gainExperience(numCycles = 1): number | undefined {


Why is the return type number | undefined?

catloversg · 2025-02-10T10:13:28Z

src/Gang/GangMember.ts

+  getMemberExperience(member: GangMember): object | undefined {
+    const numCycles = 1;
+    const task = member.getTask();
+    if (member === undefined) {


member cannot be undefined.

catloversg · 2025-02-10T10:13:39Z

src/Gang/formulas/formulas.ts

@@ -72,6 +72,62 @@ export function calculateMoneyGain(gang: FormulaGang, member: GangMember, task:
  return Math.pow(5 * task.baseMoney * statWeight * territoryMult * respectMult, territoryPenalty);
 }

+//Calculates memberEXP based on member passed and task passed that the member currently has assigned.
+//@returns object containing all exp values that would be gain on task compeletion.
+export function calculateMemberExperience(numCycles: number, member: GangMember, task: GangMemberTask): object {


Don't use the generic object. You have to define what you return.

catloversg · 2025-02-10T10:13:43Z

src/Gang/formulas/formulas.ts

+    chaEXP: 0,
+  };
+
+  const difficultyMult = Math.pow(task.difficulty, 0.9);


You still duplicate code in gainExperience.

Okay, so I refactored the function in a way that we no longer duplicate functions, it now just provides that object based on whether the place in the code provides a necessary member or not. When a character is actually gaining exp I merely made it take null since we already have the player object we need to access. Where in the data retrieval API I instead passed the member object to it to grab the exp data and return it. Hopefully that is a much better alternative instead.

catloversg · 2025-02-10T10:13:48Z

src/NetscriptFunctions/Gang.ts

@@ -1,6 +1,6 @@
 import type { Gang as IGang, EquipmentStats, GangOtherInfoObject } from "@nsdefs";
 import type { Gang } from "../Gang/Gang";
-import type { GangMember } from "../Gang/GangMember";
+import { GangMember } from "../Gang/GangMember";


Is this change necessary? I don't see anything that requires this change.

You really need to check this change.

catloversg · 2025-02-10T10:13:54Z

src/ScriptEditor/NetscriptDefinitions.d.ts

@@ -1004,6 +1003,8 @@ interface GangMemberInfo {
  wantedLevelGain: number;
  /** Per Cycle Income for this gang member */
  moneyGain: number;
+  /**PerCycle Exp object for each stat for this gang member */
+  expGain: object | undefined;


Don't use the generic object. You have to define what you return. undefined vs null is opinion-based, so it may be fine, but I prefer null in this case.

Complete the fixes you guys mentioned, hopefully this version is much more appealing. My apologies again for the previous iterations.

Jumping in here to say: Iterating is how you learn! (Also, it is common in general, and for catlover and I specifically, to be rather terse in code review suggestions. Don't take that as us being upset; if we were upset, you'd know XD)

No worries at all, I totally understand! I appreciate you guys taking the time to look over as well as give advice on improving.

…ns, Instead refactoredexperience gain function to pass back an object under specified parameters.

catloversg · 2025-02-11T04:45:35Z

src/Gang/GangMember.ts

@@ -147,63 +147,99 @@ export class GangMember {
      cha: (this.cha_mult - 1) / 4 + 1,
    };
  }
-  gainExperience(numCycles = 1): number | undefined {
+  gainExperience(numCycles = 1, member: GangMember | null): GangMemberExpGain | null {


You need to design and implement this function in a different way.

It does not make sense for gainExperience, which is a method of GangMember, receives a member instance.

gainExperience, as its name implies, has only one job: increase the exp. You are trying to make it do 2 different jobs at the same time.

Again, you still duplicate code here. The moment you copy/paste code, you know that it's wrong.

You can use 2 functions.

Function A: calculate the gain.

Function B: call A, then increase the calculated gain.

catloversg · 2025-02-11T04:45:44Z

src/ScriptEditor/NetscriptDefinitions.d.ts

-  /**PerCycle Exp object for each stat for this gang member */
-  expGain: object | undefined;
+  /** Per Cycle Exp object for each stat for this gang member */
+  expGain: GangMemberExpGain | null;


You have to explain the case in which it's null in the documentation.

Okay, just to make sure that this will work out fine. I separated the functions. One function independently handles the calculation of the exp that should be gained by the member. These calculations were already made in the original gainExperience function, I merely moved to another function within the formulas. I created a second function which receives those calculations in the form of an object from functionA the values from this object are added to the member's values own values. However, I was struggling very much so to find a way to ensure that the API could receive this object without duplicating exp gain. I settled on creating a getter function which is a GangMember function which receives a NumCycles value that is 1 since we are trying to calculate the exp gain of a task per cycle. There is no duplicate code and the calculations only happen once now with each function having one independent job. Please let me know if this works out if not I can attempt to adjust it more.

…lue of a member returns those values as an object to be handled accordingly

d0sboots · 2025-02-12T06:02:16Z

Thank you catlover for jumping on this while I was being lazy. I'm going to be mostly hands-off on this one (aside from pushing the "run tests" button) until it's close to ready to go.

AdamAndreatta · 2025-02-12T06:21:22Z

Ran the formatter, apologies for forgetting that. Everything should be tested and good. Just let me know if I need to make further adjustments.

catloversg · 2025-02-12T15:15:02Z

src/Gang/formulas/formulas.ts

@@ -72,6 +73,60 @@ export function calculateMoneyGain(gang: FormulaGang, member: GangMember, task:
  return Math.pow(5 * task.baseMoney * statWeight * territoryMult * respectMult, territoryPenalty);
 }

+export function calculateExpGain(numCycles = 1, member: GangMember, task: GangMemberTask): GangMemberExpGain {


Task is a property of GangMember. There is no need to pass it to this function.

catloversg · 2025-02-12T15:16:04Z

src/NetscriptFunctions/Gang.ts

@@ -1,6 +1,6 @@
 import type { Gang as IGang, EquipmentStats, GangOtherInfoObject } from "@nsdefs";
 import type { Gang } from "../Gang/Gang";
-import type { GangMember } from "../Gang/GangMember";
+import { GangMember } from "../Gang/GangMember";


You really need to check this change.

catloversg · 2025-02-12T15:22:15Z

src/Gang/GangMember.ts

+  // if nothing is provided to the member it returns null
+  getGangMemberExpGain(numCycles = 1): GangMemberExpGain | null {
+    const task = this.getTask();
+    if (task == GangMemberTasks.Unassigned) {


Be careful with == and ===. In most cases, using === is the safest choice.

catloversg · 2025-02-12T15:22:34Z

src/Gang/GangMember.ts

@@ -147,43 +149,28 @@ export class GangMember {
    };
  }

+  // Grabs the GangMemberExpGain object based on the passed paramters to be exported to the getMemberInfo API
+  // if nothing is provided to the member it returns null
+  getGangMemberExpGain(numCycles = 1): GangMemberExpGain | null {


This function is unnecessary. The check of GangMemberTasks.Unassigned is duplicated here and in gainExperience. It should be in calculateExpGain. If the task is Unassigned, calculateExpGain returns null. In gainExperience, if calculateExpGain returns null, gainExperience returns immediately.

Okay got rid of the unnecessary function made the calculation a member function which made the most sense, there was no reason to pass all the parameters when it's merely going to be called from member anyway where all the necessary data is present to return the per-cycle exp.

… that it no longer needs to pass parameters it just takes from whatever member is called from and returns their per-cycle exp

catloversg · 2025-02-13T02:59:54Z

src/Gang/GangMember.ts

+    return expValues;
+  }
+
+  gainExperience(): void {


This function needs to receive numCycles, then pass it to calculateExpGain. You have to build and test your change.

Made the change, ran the linter, formatter, and testing suite. Testing suite showed all tests were passed.

catloversg · 2025-02-13T04:36:17Z

@d0sboots The basic design looks good. Please review the rest. Some things are opinion-based:

GangMemberExpGain | null: I prefer returning null when the action is invalid (the current task is Unassigned), so I suggested it. If you think returning an object with exp = 0 is better, please feel free to request that change.
Wording of some comments needs to be improved. It's best to let you recommend it.

@AdamAndreatta Our test suite is not really good. For this kind of change, you should (at least) write in-game scripts to test it, copy/paste those scripts and the result here. It's even better if you add more Jest tests, but it may be too hard for a new contributor, so you can skip it.

AdamAndreatta · 2025-02-13T05:28:53Z

Presented no input to the function which provided the expected error

export async function main(ns){
    let names = ns.gang.getMemberNames();
    let value = ns.gang.getMemberInformation("");
    ns.tprint(value);
}

export async function main(ns){
    let names = ns.gang.getMemberNames();
    let value = ns.gang.getMemberInformation("guy2");
    ns.tprint(value);
}

Provided input when not task was currently assigned, expected result was null and null was presented.

Provided input while also ensuring the member was assigned a task, the expected object was presented with the per cycle values.

d0sboots

Looking good, it looks like catlover already put you through the wringer XD

I'm primarily concerned with stuff that affects the public (ns) interface, but these should be quick to fix.

d0sboots · 2025-02-16T09:31:55Z

src/ScriptEditor/NetscriptDefinitions.d.ts

+  /** Per Cycle Exp object for each stat for this gang member returned as a object containing each stats value
+   * returns null in the even that the member does not have a given task.


Suggested change

/** Per Cycle Exp object for each stat for this gang member returned as a object containing each stats value

* returns null in the even that the member does not have a given task.

/**

* Per Cycle Exp object for each stat for this gang member.

*

* null in the event that the member does not have a given task.

The second part of the sentence was redundant and confusing IMO

In docs like this, you need a extra newline, otherwise it will all run together on one line when formatted. (Or, maybe that's what you want, but you will still want a period at the end of the prior sentence.)

d0sboots · 2025-02-16T09:33:29Z

src/Gang/GangMember.ts

+      return;
+    }
+
+    this.hack_exp += gains.hackEXP;


Here and everywhere: Name the fields consistently (should be hack_exp etc.)

Also in the external interface. We're not super consistent, but for stats underscores and lowercase is pretty consistently the style.

@d0sboots made those fixes for ya if there's anything else ya need me to do lmk, more than happy to do so. Thanks very much to you both for being so helpful.

This might seem nitpicky, but it's hack_exp not hack_EXP.

@d0sboots fixed it 👍

Don't forget to rerun npm doc

@d0sboots sorry about that! completely slipped my mind its updated now

AdamAndreatta added 3 commits February 8, 2025 19:10

added new api call for experience gain returns list of experience for…

ca5c33d

… each stat.

finished api functionality

f51b075

edited documentation and ran tests/linting

82a8fde

catloversg suggested changes Feb 9, 2025

View reviewed changes

adjust api function works as intended

db08b60

catloversg suggested changes Feb 10, 2025

View reviewed changes

Got rid of unnecessary functions that merely duplicated other functio…

db8e5df

…ns, Instead refactoredexperience gain function to pass back an object under specified parameters.

catloversg suggested changes Feb 11, 2025

View reviewed changes

AdamAndreatta added 2 commits February 11, 2025 20:16

created a function which only handles calculations of exp for each va…

5fba661

…lue of a member returns those values as an object to be handled accordingly

changed the function to have a default value

37dfd62

fixed formatting issue

5ba867c

catloversg suggested changes Feb 12, 2025

View reviewed changes

Go rid of uneccessary function. Made calculation a member function so…

9daaf6d

… that it no longer needs to pass parameters it just takes from whatever member is called from and returns their per-cycle exp

catloversg suggested changes Feb 13, 2025

View reviewed changes

Made gain exp recieve numCycles and pass it to the calculation function

0f5b2a2

d0sboots reviewed Feb 16, 2025

View reviewed changes

AdamAndreatta and others added 5 commits February 17, 2025 20:04

Fixed inconsitencies in naming for variables

b3bfce2

Adjusted comment in Netscript definitions.d.ts

4b5171c

adjust value definition for exp object GangMember.ts

362edf2

Update interface defintions NetscriptDefinitions.d.ts

5412b49

updated doc file

f7b737d

d0sboots merged commit a25ab3d into bitburner-official:dev Feb 20, 2025
5 checks passed

		@@ -1,5 +1,7 @@
		/** All netscript definitions */

		import { GangMember } from "src/Gang/GangMember";

		/** Per Cycle Exp object for each stat for this gang member returned as a object containing each stats value
		* returns null in the even that the member does not have a given task.

API: Added external api which returns gainExperience #1955

API: Added external api which returns gainExperience #1955

Conversation

AdamAndreatta commented Feb 9, 2025 • edited Loading

Created a function to provide expGain to the getmemberinfo api

catloversg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AdamAndreatta commented Feb 10, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

catloversg Feb 10, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

catloversg Feb 11, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AdamAndreatta Feb 12, 2025 • edited Loading

Choose a reason for hiding this comment

d0sboots commented Feb 12, 2025

AdamAndreatta commented Feb 12, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

catloversg commented Feb 13, 2025

AdamAndreatta commented Feb 13, 2025 • edited Loading

d0sboots left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AdamAndreatta Feb 18, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AdamAndreatta commented Feb 9, 2025 •

edited

Loading

catloversg Feb 10, 2025 •

edited

Loading

catloversg Feb 11, 2025 •

edited

Loading

AdamAndreatta Feb 12, 2025 •

edited

Loading

AdamAndreatta commented Feb 12, 2025 •

edited

Loading

AdamAndreatta commented Feb 13, 2025 •

edited

Loading

AdamAndreatta Feb 18, 2025 •

edited

Loading