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

Weapon Value Caches properly update #2742

Merged
merged 7 commits into from
May 4, 2023

Conversation

KheirFerrum
Copy link
Collaborator

Summary

SUMMARY: Bugfixes "Weapon Value Caches properly update"

Purpose of change

#1705 revealed issues with weapon value cache for NPCs, where they try to store and read ideal and current values, leading to mismatch and incorrect evaluations.

Describe the solution

Adds new wielded_value() function dedicated to updating value of currently wielded weapon. Accepts Character and bool as arguments, where the bool decides whether the value to be retrieved is the ideal value if the NPC had access to "infinite" ammo (set to magazine size otherwise values can get absurd).

Refactors wield_better_weapon() to solve secondary issue @scarf005 noticed in #1705 where NPC would swap to bionic weapon only to swap back due to how weapon swapping occurs in both check_or_use_weapon_cbms() and wield_better_weapon(). Now weapon swapping can only occur in wield_better_weapon(), check_or_use_weapon_cbms() instead queues the cbm weapon for checking.

Describe alternatives you've considered

Can't think of any.

Testing

Spawn NPC, give them a katana, an M4A1, a filled STANAG 20 round magazine, 50 5.56 NATO rounds, 6 00 shotshells, and then install the bionic shotgun into them.

  • Spawn menacing debug monster within range. Check that the NPC wields their M4A1.
  • Check that NPC empties first magazine, then swaps magazines and continues to fire
    • Check that NPC does not try to reload empty magazines during combat.
  • Check that NPC then swaps to bionic shotgun once ammo sufficiently depleted and shoots until they run out of ammo
    • Check that NPC, having emptied the bionic shotgun's magazine, loads ammo one at a time and shoots the monster after each load.
  • Check that once bionic shotgun is emptied (and NPC swaps back to M4A1 and fully empties that.) NPC swaps to katana.

Additional context

Note, NPC will not empty the M4A1 on their last mag but leave it on 1 bullet remaining, fixing this is somewhat out of scope, as it requires adding consideration for reload cost, weapon swap cost, and firing cost.

@github-actions github-actions bot added the src PR changes related to source code. label May 1, 2023
@KheirFerrum KheirFerrum force-pushed the weapon-value-cache-fix branch 2 times, most recently from 31cfb9a to 2e440fa Compare May 1, 2023 20:13
Best cannot be cbm_fake_toggled if the fake item is being wielded.
@KheirFerrum KheirFerrum changed the title wielded_value(), refactor wield_better_weapon() Weapon Value Caches properly update May 1, 2023
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.

NPC's inventory

image

output.mp4

I'm not sure why but the weapon switch of death (WSOD) still happens. could you check again?

this is my save with NPC setup for test.
EmptyWorld.zip

Properly considers the item itself, reworks some old code.

Ideal weapon value creates fake item with max ammo
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.

Spawn menacing debug monster within range. Check that the NPC wields their M4A1.

2023-05-04.09-28-26.mp4

using the same save. this time, npc only wields katana, despite having m4a1.

@KheirFerrum KheirFerrum requested a review from scarf005 May 4, 2023 01:21
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.

cache.mp4

confirmed that the npc

  1. wields and fires m4a1 rifle first, and reloaded only once.
npc only reloaded empty magazine after combat.

image
image

  1. wields and fires their bionic shotgun until emptying magazine, then loads and fires ammo one at a time.
  2. uses their katana.

npc logic is still very wacky and sometimes they whack debug monster with m4a1 instead of their katana, but let's fix them later.

@@ -2015,7 +2015,7 @@ double npc_ai::gun_value( const Character &who, const item &weap, int ammo )
}

// Gives an approximation of DPS disregarding modes.
float move_cost_factor = move_cost / 100;
float move_cost_factor = move_cost / 100.0f;
Copy link
Member

Choose a reason for hiding this comment

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

interesting. (godbolt)

#include <array>
#include <iostream>

auto main() -> int {
    const auto a = int{123};
    const auto b = int{100};
    const auto c = float{100};

    std::cout << "123 / 100 :: int = " << float{a / b} << "\n";
    std::cout << "123 / 100 :: float = " << float{a / c} << "\n";
}
/*
123 / 100 :: int = 1
123 / 100 :: float = 1.23
*/

@scarf005 scarf005 merged commit e039107 into cataclysmbnteam:upload May 4, 2023
16 checks passed
@KheirFerrum KheirFerrum deleted the weapon-value-cache-fix branch May 4, 2023 02:31
@KheirFerrum KheirFerrum mentioned this pull request May 6, 2023
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
src PR changes related to source code.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants