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

Rework Bionic weapon selection and make BIONIC_GUNS work for NPCs. Fix NPC Aiming. #1705

Merged
merged 38 commits into from
May 1, 2023

Conversation

KheirFerrum
Copy link
Collaborator

@KheirFerrum KheirFerrum commented Jul 7, 2022

Summary

SUMMARY: Bugfixes "Rework Bionic weapon selection and make BIONIC_GUNS work for NPCs. Fix NPC Aiming."

Purpose of change

Deals with an issue found in #1227 where bionic guns were not firing and instead being used as melee weapons.

Secondarily deal with bugs in NPC AI that prevented them from aiming guns.

Describe the solution

Goal here is in several parts.

  • First, need the check_or_use_weapon_cbm() code to be reorganized. such that it compares all possible bionics, and picks the "best" of the lot.
  • Tear out the unfinished implementation that would attempt to make it work like toggle CBMs. It was not designed to work as a toggle and making the player and NPCs use two different systems for the same kinds of bionics can only cause us suffering.
  • New implementation: cbm_toggle and cbm_active store toggled and activated cbm weapons respectively. cbm_fake_active stores a copy of the fake weapon from the activated CBM if one is chosen, to prevent segfaults from the item disappearing into the void after being initialized in a function.
  • wield_better_weapon() now allows swapping between bionic weapons and wielded weapons where applicable. On the first turn, before wield_better_weapon() is called a bionic toggled weapon will be selected first (and most likely pass unless you specifically tell them to wield something. Then, just after it will start to compare available weapons against the chosen bionic weapon. If a better one is found it will deactivate the bionic weapon and wield the selected one.
  • Update "peacetime" reloading code so NPCs will reload their toggled bionics where applicable.

Separate from all of this is a bugfix. invalidate_ranged_cache() is supposed to update the confident_range_cache using confident_shoot_range(), but DDA PR #35295 broke it by making confident_shoot_range() use the confident_range_cache if available, so it would never properly update after the first time.

Describe alternatives you've considered

I can't think of any good alternatives. Probably due to myopia, the only other solution I can think of is to ignore this entirely because it's driving me insane.

Testing

Active Bionic Weapon test

  • Install Finger Laser into target NPC, grant NPC bionic power. spawn menacing debug monster. Check that NPC will move toward monster, then stop, aim, and shoot their Finger Laser at the menacing debug monster until their power drops below threshold.
  • Check that wielded weapons do not disappear when they activate their active bionic weapons. Delete monster and NPC.

Toggled Bionic Weapon test

  • Install Shotgun Arm into target NPC, grant NPC bionic power, then give them 2 round of 00shot. Check that on pause, NPC will attempt to reload shotgun arm. Spawn menacing debug monster, check that NPC will aim then shoot monster twice. Delete monster.
  • Grant target NPC 10 00shot shells. Check that NPC will only load 4. Spawn menacing debug monster, check that NPC will shoot 4 times, then reload and fire 1 shell at a time until empty. Delete target NPC and monster.

Bionic Weapon selection test

  • Install claws and monomolecular blade. Check that NPC will activate only the Monomolecular Blade when entering combat. Similarly, check that NPC will only activate the shotgun when given a choice between all current bionic weapons (assuming they have energy/ammo for all of them)
  • Note, due to issues described in Additional Context, NPCs should not be expected to swap from Shotgun Arm to other weapons once empty. Or indeed, any gun.

Reload/NPC AI fixes

  • Check that NPCs given M4A1 will only perform combat reloads with magazines, while shotguns will result in NPCs reloading a single shell and immediately shooting the enemy instead of trying to reload the full thing.
  • Check that NPCs with loaded guns will aim once within their confident range instead of running to their enemy and shooting them point blank.

Additional context

Bug noted while testing. npc_ai::weapon_value() calls and writes to a cache, but the cached weapon value is only a snapshot for weapons with ammo requirements. To further complicate matters, npc_ai::weapon_value() may be called to produce "ideal" values instead of current values, leading to wrong values being read from the cache during combat. Fixing this has been laborious and annoying, I'm relegating it to a separate PR.

  • Future goal: Make BIONIC_GUNS key off the fake_item it spawns. And make toggled bionic weapons that use charges draw from bionic power using USES_BIONIC_POWER flag.

@github-actions github-actions bot added data PRs related to datas. Won't crash game (probably) src PR changes related to source code. labels Mar 20, 2023
@github-actions github-actions bot removed src PR changes related to source code. data PRs related to datas. Won't crash game (probably) labels Mar 20, 2023
@KheirFerrum KheirFerrum reopened this Mar 20, 2023
@github-actions github-actions bot added data PRs related to datas. Won't crash game (probably) src PR changes related to source code. labels Mar 20, 2023
@KheirFerrum KheirFerrum force-pushed the Fix-bionic-guns branch 3 times, most recently from 5e2711e to ea49340 Compare March 21, 2023 23:13
@KheirFerrum KheirFerrum marked this pull request as ready for review March 22, 2023 09:00
@KheirFerrum
Copy link
Collaborator Author

KheirFerrum commented Mar 22, 2023

Toggled CBM reload not yet implemented, not sure how to implement honestly, since again the item doesn't exist unless the NPC activates it. I could have an extra int like magazine size for bionics so it doesn't have to instantiate the item to check how much ammo can be held in it.

Done

@KheirFerrum KheirFerrum changed the title Make Bionic Guns work Make Bionic Guns work. Fix NPC Aiming. Mar 22, 2023
Complete first implementation

Update bionics.json

NO_UNWIELD has no purpose on bionics, only fake_weapons.

Lightning, wrist mounted rockets excluded from BIONIC_NPC_USABLE until we code in more sophisticated considerations for friendly fire.

EMP gun excluded on basis that NPCs cannot distinguish vulnerabilities and do not care if they're actually doing damage.
DDA PR 35295 broke NPC confident_range_cache's. This fixes it, so now NPCs will actually aim their guns instead of attempting to use them in melee
Reload enough for one shot if your best weapon requires ammo to be loaded one by one, you're out of ammo, and you're in danger.
@KheirFerrum KheirFerrum force-pushed the Fix-bionic-guns branch 3 times, most recently from eaf23d3 to ddb8043 Compare March 22, 2023 21:20
Bionic Shotguns are now reloaded both in and out of combat depending on situation

Added a "Menacing Debug Monster".
@KheirFerrum KheirFerrum changed the title Make Bionic Guns work. Fix NPC Aiming. Rework Bionic weapon selection and make BIONIC_GUNS work for NPCs. Fix NPC Aiming. Mar 23, 2023
Removed modes.clear(), seems to work for now.
No need for std::max anymore
@KheirFerrum KheirFerrum force-pushed the Fix-bionic-guns branch 2 times, most recently from 3d1351b to 1ac631a Compare April 25, 2023 21:03
What's the point of having a definition for infinite charges if it's not used?

Reorganizes ammo_count_for function and comments on something that might be obtuse.

Accurately consider ammo drain
Some stuff isn't used, clean it up.

Fix up the message since now the cache checks for non-negative.
Copy link
Member

@scarf005 scarf005 left a comment

Choose a reason for hiding this comment

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

Cataclysm: Bright Nights - cbn-experimental-2023-04-19-1517-36-g851a5f15e960_01
image

segmentation fault when reading cbm_weapon_index from pre-PR saves. could you check on them?

tested with: EmptyWorld.zip

cbm_weapon_index defaults to -1 in legacy saves when empty
Copy link
Member

@scarf005 scarf005 left a comment

Choose a reason for hiding this comment

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

i'm struggling to make npc with bionic shotgun CBM installed actually fire shotshells. it'd be appreciated if you could provide an example steps of testing the PR.

@chaosvolt
Copy link
Member

Are you able to get it working with finger lasers or other standard bionic guns? If so it might be something to do with differences in behavior between bionic ranged weapons that draw solely from bionic power vs the ones that need to be loaded.

@scarf005
Copy link
Member

scarf005 commented Apr 27, 2023

yes. i was able to make NPC fire their finger laser, but was unable to give them shotshell (they refuse because npcs think given ammo as 'weapons'), thus failed to test them.

@chaosvolt
Copy link
Member

chaosvolt commented Apr 27, 2023

Ah, the "I want you to use this item" option? Yeah, picking that always makes them assume you want them to wield or wear the item being given. For giving ammo you have to use "let's trade" instead.

EDIT: Huh, turns out they can also be made to use consumables from that prompt. Damn, I was tempted to change the line to "I want you to wield/wear this item" to make it more clear but wield/wear/consume would be a lot more clunky.

@KheirFerrum
Copy link
Collaborator Author

KheirFerrum commented Apr 27, 2023

yes. i was able to make NPC fire their finger laser, but was unable to give them shotshell (they refuse because npcs think given ammo as 'weapons'), thus failed to test them.

Don't use "I want you to use this" but instead "I want you to hold on to this" further down.

Copy link
Member

@scarf005 scarf005 left a comment

Choose a reason for hiding this comment

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

Active Bionic Weapon test

0.finger-laser-test.mp4

Toggled Bionic Weapon test

Shotgun arm test (2 rounds)

1.bionic-shotgun-test-1.mp4

Shotgun arm test (10 rounds)

1.bionic-shotgun-test-2.mp4

Bionic Weapon selection test

NPC will activate only the Monomolecular Blade

2.blade-selection-test.mp4

NPC will only activate the shotgun

2.choose-bionic-shotgun-hand-bug.mp4

also bug confirmed

Reload/NPC AI fixes

M4A1 fires then reload magazine

3.m4a1-reload.mp4

Infinite loop found:

image

if npc has both weapon and bionic weapon, and wielded weapon gets empty, they will forever switch between weapon and bionic weapon. in the above video, i had to uninstall all the bionics to break the loop (and use m4a1). it may be related to the known noted error.

Shotguns fire once then reload once

https://user-images.githubuserconten
t.com/54838975/235451168-8f382c4e-9ac9-4159-920f-5bb8edd220f5.mp4


approving despite having bug, as

  1. it looks like a complex bug to solve
  2. it may be related to aforementioned known bug
  3. other than that, all tests work

@scarf005 scarf005 merged commit 81cfc0f into cataclysmbnteam:upload May 1, 2023
18 checks passed
@KheirFerrum KheirFerrum deleted the Fix-bionic-guns branch May 1, 2023 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
data PRs related to datas. Won't crash game (probably) src PR changes related to source code.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants