Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DOCUMENTATION: Clarify ns.scan #1965

Conversation

catloversg
Copy link
Contributor

With ns.scan, there is some common knowledge among experienced players:

  • The parent node is always the first item of the returned array.
  • Use BFS and DFS to discover all servers.

This kind of knowledge is essential for effectively using this API, but it's not obvious for new players, especially the first one. This PR adds that information to TSDoc of ns.scan.

@LJNeon
Copy link
Contributor

LJNeon commented Feb 15, 2025

That seems like a lot of text to add to a function in the definitions file, I would've thought a shorter version would fit better there and the extended version can be in the full documentation?

@catloversg
Copy link
Contributor Author

The added information is not really long. It looks long because of the example. The part about the parent node definitely should be in TSDoc. It's the undocumented behavior of this API. The hint about BFS and DFS is not long enough to move to a different page.

@LJNeon
Copy link
Contributor

LJNeon commented Feb 15, 2025

There are no other comments in the definitions file with even as many as 150 characters... this one approaches 1,500 characters. That is definitely what I would quantity as "really long".

Edit: I was wrong about that 150 figure, but I still think this one is too long.

@d0sboots
Copy link
Collaborator

I'm not concerned about the length.

@d0sboots d0sboots merged commit 3bb56ef into bitburner-official:dev Feb 17, 2025
5 checks passed
@catloversg catloversg deleted the pull-request/documentation/clarify-ns.scan branch February 17, 2025 04:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants