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

[RDY] Bionics installing overhaul #22877

Merged
merged 22 commits into from
Jun 13, 2018
Merged

[RDY] Bionics installing overhaul #22877

merged 22 commits into from
Jun 13, 2018

Conversation

Night-Pryanik
Copy link
Contributor

@Night-Pryanik Night-Pryanik commented Feb 4, 2018

Added an alternative for manual and instant installing of CBM. Now you can use autodoc for this task.

  • Player has to be on the couch to perform operation;
  • Autodoc need to be configured prior to use;
  • Autodoc uses player skills and INT to calculate chance of success;
  • Installation now takes time, duration is based on bionic difficulty. For example, the easiest CBM (Power Storage with difficulty 1) will take twenty minutes to install;
  • Player falls asleep because of autodoc's anesthesia (abstracted for now). Player should be sure no monster will disturb the operation;
  • Autodoc spawns at the hospital.

Minor:

  • Player will now have 0 sight range when sleeping;
  • Added message when player is falling asleep.

[CR] because I want to know if this approach is good enough.

Questions:

  • Is autodoc using player skills and INT good? Or should it has some high skill, independent from the player one, removing the need to configure it?
  • Right now autodoc isn't spawned anywhere. Where should it spawn, and how frequent?
  • Should we add explicit anesthesia item, spawn it somewhere, and require to bring it to autodoc? Or we can abstract it like autodoc has it (unlimited?) somewhere in inner storage?
  • Is dividing this overhaul to several parts (installing, uninstalling, removing manual (un)installing) good?

@John-Candlebury
Copy link
Member

John-Candlebury commented Feb 4, 2018

Ideas for spawning:

-A custom room in labs, similar to the armories and holding cells. Frequently
-Custom room in hospitals. Frequently
-New bionic clinics. Rarely
-Black market clinics in house basements. Rarely

@kevingranade
Copy link
Member

I can see two major options for installation chance (you essentially have added the first option):

  1. Player programs operation into autodoc.
    Outcomes would be based on player int + medical knowledge + programming vs CBM difficulty.
    The time to create this program would scale with CBM install difficulty, and might take a very long time.
    1. Variation, buy a program from a NPC with better skills.
  2. Player acquires a program for installing a specific CBM (i.e. on a USB stick).
    This would have an almost 0 chance of bad outcomes.
    This also gives low-int characters a chance of installing, the requirements to just load the software should be very easy.
    Players would find these in lab databases, loaded into autodocs, or perhaps rarely elsewhere on USB sticks.
    1. This might have qualifications for when it will work, such as not working if the targeted body part is mutated. It will still be safe, but it will refuse to work.

@John-Candlebury covered spawn locations pretty well 👍

I'd like to eventually have anesthesia, surgical tools, bandages, suturing material, etc be consumables for this, but we can get it working first and add those things later.

Just getting installation working first is a great approach, it's by far the most common operation. In particular we don't want to remove the manual operations until this works pretty much perfectly.

inb4 "what about innawoods characters". Your options will eventually be NPC surgeon or an autodoc (or a mod, sure do whatever in a mod), so innawoods characters with NPCs disabled will be unable to access CBMs, and that's fine.

Also filled the room with lockers and racks
@Night-Pryanik
Copy link
Contributor Author

Added autodoc spawn at the previously empty hospital room, also filled it with lockers and racks.

@Night-Pryanik
Copy link
Contributor Author

For now autodoc spawns at the hospital and is working almost fully right (installing bionics still requires painkillers despite autodoc injects anesthesia) for the first step of the overhaul, so I think the PR is ready for the review.

@Night-Pryanik Night-Pryanik changed the title [WIP] [CR] Bionics installing overhaul [CR] Bionics installing overhaul Feb 5, 2018
src/iexamine.cpp Outdated

uimenu amenu;
amenu.selected = 0;
amenu.text = string_format( _( "Autodoc Mk. XI. Status: %s. Please choose operation." ), status );
Copy link
Contributor

Choose a reason for hiding this comment

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

. Please
Double whitespace required here.

src/iexamine.cpp Outdated
amenu.selected = 0;
amenu.text = string_format( _( "Autodoc Mk. XI. Status: %s. Please choose operation." ), status );
amenu.addentry( CONFIGURE, true, 'c', str_to_illiterate_str( _( "Configure Autodoc." ) ) );
amenu.addentry( INSTALL_CBM, uistate.iexamine_autodoc_config ? true : false, 'i',
Copy link
Contributor

Choose a reason for hiding this comment

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

uistate.iexamine_autodoc_config ? true : false
can be effectively shorten to just uistate.iexamine_autodoc_config since it's already a boolean variable.

src/iexamine.cpp Outdated

case INSTALL_CBM: {
if( !adjacent_couch ) {
popup( _( "No connected couches found. Operation impossible. Exiting." ) );
Copy link
Contributor

Choose a reason for hiding this comment

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

Double whitespaces after periods required.

src/iexamine.cpp Outdated
return;
}
if( !in_position ) {
popup( _( "No patient found located on the connected couches. Operation impossible. Exiting." ) );
Copy link
Contributor

Choose a reason for hiding this comment

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

here as well.

src/iexamine.cpp Outdated
bool adjacent_couch = false;
bool in_position = false;
for( const auto &couch_loc : g->m.points_in_radius( examp, 1, 0 ) ) {
if( g->m.furn( couch_loc ).obj().id == "f_autodoc_couch" ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

You don't need to get the furn_t object to get its id. map::furn returns an id that you can compare directly. I suggest you store "f_autodoc_couch" as local constant of type furn_str_id and compare it to the result of map::furn.

src/iexamine.cpp Outdated
}

const int bionic = g->inv_for_flag( "CBM", _( "Choose CBM to install" ) );
const item &it = p.i_at( bionic );
Copy link
Contributor

Choose a reason for hiding this comment

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

The two references it and itemtype are only needed below the if block, create them there (right before they are needed).

src/iexamine.cpp Outdated

switch( static_cast<options>( amenu.ret ) ) {
case CONFIGURE:
uistate.iexamine_autodoc_config = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a global variable, setting it will affect all couches all over the world. It's also not saved anywhere, so it's reset whenever the player loads a game.

Copy link
Contributor Author

@Night-Pryanik Night-Pryanik Feb 27, 2018

Choose a reason for hiding this comment

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

This is a global variable, setting it will affect all couches all over the world.

Can you recommend something to achieve the desired goal, but only for the couch player interacts with?

Copy link
Contributor

Choose a reason for hiding this comment

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

Prob you need two furnitures: f_autodoc_unconfigured, f_autodoc_configured.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd rather not add another furniture. Anybody know, if there is a way to add flags to furniture during runtime?

@kevingranade
Copy link
Member

kevingranade commented Feb 28, 2018 via email

@Night-Pryanik
Copy link
Contributor Author

After player configures the autodoc, the game places pseudo item autodoc_config onto the tile with autodoc, which is now behaves like a sealed container with no means of opening. The game then checks for this item, if it exists, then autodoc is considered configured.

@BevapDin
Copy link
Contributor

BevapDin commented Mar 6, 2018

I'd rather not add another furniture. Anybody know, if there is a way to add flags to furniture during runtime?

We don't currently have the infrastructure to store data for a specific place on the map. It's currently solved by having various terrain types (see for example open/closed doors, filled/unfilled kiln).

However, why does the autodoc need to be configure? According to the code, it does nothing, just consumes 3 turns time. This could be done automatically as part of the operation itself. In other words: why are configuration and operation separate steps?

@Night-Pryanik
Copy link
Contributor Author

Well, there's no certain reason. The configuration step is for flavor mostly. I just thought that this will help players feel that they're involved in the process. Should I remove the separate configuration step?

@kevingranade
Copy link
Member

Yes remove it. This sounds more complicated than its worth.

@Night-Pryanik
Copy link
Contributor Author

default

Not too much text at once?

@Leland
Copy link
Contributor

Leland commented Mar 6, 2018

I think the quantity of text is fine, but I'd probably expect the "Using your knowledge..." text first in that list

@Night-Pryanik
Copy link
Contributor Author

That is because the game first checks if player confirms the installation, and then prints text about knowledge and so on.

@Night-Pryanik
Copy link
Contributor Author

Moved message about configuring to popup from message in the side log - looks better imo.
Added message when player falls asleep.
Made zero sight range when player is asleep.

@Night-Pryanik
Copy link
Contributor Author

Night-Pryanik commented Mar 26, 2018

The Using your knowledge... text will appear every time one will install CBM. I think that's not good if this big chunk of text will be repeated time and time again. Maybe bring back the separate configuration step, so the text will be shown only once, and, as I said earlier, it will create base for using the medical software in lieu of manual configuring? I really like the way it looked: configure-then-install.

@ZhilkinSerg ZhilkinSerg added Bionics CBM (Compact Bionic Modules) Game: Balance Balancing of (existing) in-game features. [JSON] Changes (can be) made in JSON [C++] Changes (can be) made in C++. Previously named `Code` (P3 - Medium) Medium (normal) priority labels Apr 2, 2018
@Leland
Copy link
Contributor

Leland commented Apr 16, 2018

If the quantity of text is an issue, can be shortened:

You type into the console, configuring Autodoc to install a CBM

@Night-Pryanik
Copy link
Contributor Author

The "knowledge" part is important, I think it can help players understand that their skills are playing a role in success chance.

@Theundyingcode
Copy link
Contributor

Was reading through the open PRs out of curiosity but I had a thought on this if it's still getting worked on:

Have the Autodoc "run" the installation program from an SD card just like a CNC machine or 3d printer would. (part of iexamine method) The player could find cards with the "official installation program" from the factory or use a laptop or console to encode cards themselves. The official factory made onse would result in near zero failure chance while the outcome of player made ones would be subject to the usual int and skill levels. Requiring a certain level of computers to write the program and/or having it affect the outcomes would be probably be called for in this case.

I'm not familiar with how items are tracked but, given things decay, it should be possible to encode int and relevant skill levels at time of creation. If not, a rather lengthy copy pasted json file would do the trick.

@Night-Pryanik
Copy link
Contributor Author

Night-Pryanik commented May 21, 2018

Using pre-written programs for bionics installation saved on SD cards I consider as next step in this overhaul. At first I just want to make installation work with no errors.

@cainiaowu
Copy link
Contributor

This could be changed to [RDY], right?
After 4 month, I guess everyone already said everything about it and it is already good enough.
I really want to see this get merged.

@Night-Pryanik Night-Pryanik changed the title [CR] Bionics installing overhaul [RDY] Bionics installing overhaul Jun 6, 2018
@ZhilkinSerg ZhilkinSerg self-assigned this Jun 13, 2018
@ZhilkinSerg
Copy link
Contributor

Two suggestions for subsequent PRs:

  • Make possible to select bionics to install from adjacent tiles instead of from player inventory only;
  • Removed need to take painkillers prior to operation as autodoc injects player with anesthetics too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bionics CBM (Compact Bionic Modules) [C++] Changes (can be) made in C++. Previously named `Code` Game: Balance Balancing of (existing) in-game features. [JSON] Changes (can be) made in JSON (P3 - Medium) Medium (normal) priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants