bitburner-official / bitburner-src

Bitburner source code.
Other
688 stars 232 forks source link

MISC: BitNode options #1411

Open catloversg opened 2 weeks ago

catloversg commented 2 weeks ago

This PR adds a new feature: BitNode options. When the player enters a BN, they can set custom BitNode options:

Intentional changes:

Test case

Normal gameplay: Check SF level and active SF level after entering a new BN:

Change active SF level:

Change boolean options:

Screenshot

1 2 3

d0sboots commented 1 week ago

High-level comments before I dive into the code:

d0sboots commented 1 week ago

I don't know if you've been following allong in #development, here's some other comments that came from discussion there:

catloversg commented 4 days ago

New UI: Capture

Advanced options are cleared when the player switches between targeted BitNodes. They are also not persisted in Settings. This is intentional. IMO, a minor inconvenience is better than "Oops! I forgot resetting those options!".

d0sboots commented 4 days ago

UI is starting to look pretty good, or at the minimum, good compared to many of our UIs XD

I agree with your reasoning on not saving the settings. Especially with the new-style of adding individual SFs.

Some other UI thoughts: We should add tooltips for the toggles. What, exactly, "Restrict home PC upgrade" entails isn't clear without extra words. Extra words can also make it clear that "Disable hacknet servers" means "Reenable hacknet nodes".

For the buttons, IMO we should do more graphically and less with text. I'm assuming "level" means "current owned level of this SF" - that should be expressed by number of buttons. (There's no point having a button you can't click on.) If you don't have the SF, you shouldn't be able to add it, and if you only have 1 level you only get one button.

Similarly, the currently active level should use a bright highlight or something - standard "button selected" stuff. That eliminates the need for the extra text and IMO will be easier to visually understand.

d0sboots commented 4 days ago

One other suggestion that I needed to read the code first to make: IIUC, the "Set all SF" button isn't a setting of its own, but rather it immediately creates/sets the SF setting for all possible values. It's not what I was initially expecting, but I think it's fine. However, in that case it should get an additional "X" or trashcan button to remove all SF settings so that the state is back to default.

jjclark1982 commented 3 days ago

Should there be a toggle to enter a BitNode with no Intelligence (without disabling SF5 features)? And how should the results merge with previous Intelligence?

A reasonable scenario for comparing speed-runs would be "How long does it take to complete BitNode N with all features unlocked and no starting Intelligence?" so disabling SF5 is not a great option as it removes Formulas.exe at start and Intelligence gain within the node, which are changes to strategy.

catloversg commented 3 days ago

New UI: Capture

UI changes:

catloversg commented 3 days ago

Should there be a toggle to enter a BitNode with no Intelligence (without disabling SF5 features)? And how should the results merge with previous Intelligence?

A reasonable scenario for comparing speed-runs would be "How long does it take to complete BitNode N with all features unlocked and no starting Intelligence?" so disabling SF5 is not a great option as it removes Formulas.exe at start and Intelligence gain within the node, which are changes to strategy.

I can add "Disable Intelligence bonus". That option will only disable the bonus of Intelligence (calculateIntelligenceBonus). I'll add it if d0sboots and Snarling agree.

d0sboots commented 3 days ago

I can add "Disable Intelligence bonus". That option will only disable the bonus of Intelligence (calculateIntelligenceBonus). I'll add it if d0sboots and Snarling agree.

That seems like a good way to handle it: you still gain int (if applicable), it just has no effect. It might need a UI bit though, otherwise it'll look like you have normal int and won't be obvious that it's disabled. Maybe strikethrough where int is displayed?

I think @Snarling has yet to sign off on the overall design, but I would encourage him to (I'm not going to merge this one without some kind of thumbs up).

catloversg commented 3 days ago

Snarling mentioned that they would release v2.6.2 soon. This PR is a bit large, so it's best to merge it after that. I want it to be tested in the dev branch for a bit longer.