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

CORPORATION: Add a new API to sell a division #1210

Merged

Conversation

catloversg
Copy link
Contributor

This PR adds 2 APIs:

  • restartCorporation: selfFund does NOT have a default value.
  • sellDivision.

Other changes:

  • Change the import of Player from import { Player as player } to import { Player } in many places.
  • Change the relative path of Player's import to @player in many places.
  • Remove SellCorporationModal.tsx. Its implementation is merged with CreateCorporationModal.tsx.

Copy link
Collaborator

@d0sboots d0sboots left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The refactoring here looks good.

However, I'm slightly hesitant about sellCorporation. In the past, there were cheese stats for BN3 that revolved around repeatedly selling your corp; those might still exist, or might come back in the future. I'm also not sure what the method is good for except cheese strats.

sellDivision is similar, but probably OK.

src/Corporation/ui/modals/CreateCorporationModal.tsx Outdated Show resolved Hide resolved
src/NetscriptFunctions/Corporation.ts Outdated Show resolved Hide resolved
@catloversg
Copy link
Contributor Author

After discussing with maintainers and other contributors, I decide to remove restartCorporation API.
The behavior of createCorporation in Actions.ts is not changed to avoid a breaking change in createCorporation API despite the different behaviors between throwing errors and returning true/false.

@catloversg catloversg changed the title CORPORATION: Add new APIs to restart a corporation and sell a division CORPORATION: Add a new API to sell a division Apr 10, 2024
@d0sboots d0sboots merged commit 216500e into bitburner-official:dev Apr 16, 2024
6 checks passed
@catloversg catloversg deleted the pull-request/corporation/new-api branch April 16, 2024 04:23
antoinedube pushed a commit to antoinedube/bitburner-source that referenced this pull request May 5, 2024
Also refactoring around use of "player" variable (whether it is capitalized or not).
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.

2 participants