-
Notifications
You must be signed in to change notification settings - Fork 295
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
BLADE: Adjust IV action to current-city affect rather than global #1586
Draft
Faenre
wants to merge
9
commits into
bitburner-official:dev
Choose a base branch
from
Faenre:blade-iv-action-balance
base: dev
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
b16c207
Adjust IV action to current-city changes rather than global
Faenre 9fc0de8
Add clarity to action description
Faenre b892eb0
undo log change
Faenre 793e544
use clamping to fix NaN bug
Faenre e3b2ee4
percent
Faenre fd4bf1b
finish adding clamping to all instances of chaos +=
Faenre f7c4469
keep same formula
Faenre 48d3088
prettier
Faenre 8e4c874
use same language as Diplomacy
Faenre File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This part is the logical change. The scaling stays the same in this PR, but the penalty now only affects the player's current city.
Also, the change to
changeChaosByCount
handles integer safety with a clamp that defaults toNumber.MAX_VALUE
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To continue on the discord discussion: I vote for combining the two logics. This is not a perfect, final solution, but merely something that I'm confident will move this PR forward and allow us to make adjustments that have a positive effect on playability.
This way the cost of generating contracts remains global, but the escalating effect only stays on the one you are currently looking at (ie. you won't accidentally end up with your non-current city having 1e50 chaos in two days).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I request separating this commit out to a new PR and reverting it in this one so we can merge the surrounding refactors.