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

Vendors equip strongest weapon #59

Closed
AmProsius opened this issue Jan 7, 2021 · 8 comments · Fixed by #62
Closed

Vendors equip strongest weapon #59

AmProsius opened this issue Jan 7, 2021 · 8 comments · Fixed by #62
Labels
impl: hook engine func This issue requires hooking engine functions. type: session fix The fix for this issues is persistent across a session.
Milestone

Comments

@AmProsius
Copy link
Owner

When the player sells a weapon to a vendor that's stronger than their current one, the vendor will equip the new weapon. That prevents the player from buying that weapon back.

@AmProsius AmProsius added this to To Do in v1.0.0 via automation Jan 7, 2021
@szapp
Copy link
Collaborator

szapp commented Jan 9, 2021

A few words about the fix. It will prevent NPCs from equipping a weapon when they already have one of the type equipped. That means in particular:

  1. If an NPC is created with a melee weapon, they will stick to this weapon for the entire game, unless explicitly exchanged through the scripts.
  2. If an NPC is created without a ranged weapon, they will equip a ranged weapon should they ever acquire one (e.g. through trading with the player).

In short: An NPC willl auto-equip a ranged and a melee weapon if available - but only the first time.

So, you could argue the fix only works to a certain extend. In most cases, the first sold ranged weapon will be "lost" for most traders.

@szapp szapp added the type: session fix The fix for this issues is persistent across a session. label Jan 9, 2021
AmProsius added a commit that referenced this issue Jan 9, 2021
v1.0.0 automation moved this from To Do to Done Jan 9, 2021
szapp added a commit that referenced this issue Jan 15, 2021
szapp added a commit that referenced this issue Jan 15, 2021
@AmProsius AmProsius linked a pull request Jan 17, 2021 that will close this issue
szapp added a commit that referenced this issue Jan 31, 2021
@AmProsius AmProsius added this to the v1.0.0 milestone Feb 9, 2021
@szapp
Copy link
Collaborator

szapp commented Mar 14, 2021

I had an idea to improve this slightly.

Trading with mages (e.g. Xardas or Cronos) will always result in them equipping weapons, because they typically do not wear any to begin with. As soon as the player sells them a weapon (of any type) they will have it equipped when the player returns to them even with this fix. There is the option of disabling that for NPCs that exclusively use magic to fight:

func void G1CP_059_NpcEquipBestWeapons_Hook() {
G1CP_ReportFuncToSpy();
// Define possibly missing symbols locally
const int ITEM_KAT_NF = 2;
const int ITEM_KAT_FF = 4;
var C_Npc npc; npc = _^(ESI);
if (!Npc_HasEquippedMeleeWeapon(npc))
&& (!Npc_HasReadiedMeleeWeapon(npc)) {
G1CP_NpcEquipBestWeaponByType(npc, ITEM_KAT_NF);
};
if (!Npc_HasEquippedRangedWeapon(npc))
&& (!Npc_HasReadiedRangedWeapon(npc)) {
G1CP_NpcEquipBestWeaponByType(npc, ITEM_KAT_FF);
};
};

changed to

func void G1CP_059_NpcEquipBestWeapons_Hook() {
    G1CP_ReportFuncToSpy();

    // Define possibly missing symbols locally
    const int ITEM_KAT_NF = 2;
    const int ITEM_KAT_FF = 4;
    const int FAI_HUMAN_MAGE = -2;
    if (FAI_HUMAN_MAGE == -2) {
        FAI_HUMAN_MAGE = G1CP_GetIntVar("FAI_HUMAN_MAGE", 0, -1);
    };

    var C_Npc npc; npc = _^(ESI);

    if (!Npc_HasEquippedMeleeWeapon(npc))
    && (!Npc_HasReadiedMeleeWeapon(npc))
    && (npc.fight_tactic != FAI_HUMAN_MAGE) {
        G1CP_NpcEquipBestWeaponByType(npc, ITEM_KAT_NF);
    };

    if (!Npc_HasEquippedRangedWeapon(npc))
    && (!Npc_HasReadiedRangedWeapon(npc))
    && (npc.fight_tactic != FAI_HUMAN_MAGE) {
        G1CP_NpcEquipBestWeaponByType(npc, ITEM_KAT_FF);
    };
};

This should work great in the main game. In mods, however, it could cause issues. If a mod, for example, adds wands or staffs for mages but does not equip them (only adds them to the inventory), they would not show up in the game. Given that those NPCs would only fight with magic anyway it might only cause cosmetic issues - given that the fight AI was not adjusted such the mage-fighters switch to melee in close combat for example.

We could discuss this. In my opinion it would be nice not seeing Xardas with weird orc weapons on his back.

@szapp szapp reopened this Mar 14, 2021
v1.0.0 automation moved this from Done to In Progress Mar 14, 2021
@szapp
Copy link
Collaborator

szapp commented Mar 14, 2021

If we choose to do so, that might be something for v1.1.0 or even a hotfix for v1.0.0 should we have to create one anyway.

@AmProsius
Copy link
Owner Author

Trading with mages (e.g. Xardas or Cronos) will always result in them equipping weapons, because they typically do not wear any to begin with. As soon as the player sells them a weapon (of any type) they will have it equipped when the player returns to them even with this fix. There is the option of disabling that for NPCs that exclusively use magic to fight[.]

I like this idea, but I'd like to treat it as a separate fix, so mods would be able to disable the mage fight (if they introduce staffs) without disabling the vendor fix.

@szapp
Copy link
Collaborator

szapp commented Mar 14, 2021

I'd like to treat it as a separate fix, so mods would be able to disable the mage fight (if they introduce staffs) without disabling the vendor fix.

I think that should be possible. Of course, the separate fix will only take effect if this fix here (#59) is applied, too.

@szapp szapp closed this as completed Mar 14, 2021
v1.0.0 automation moved this from In Progress to Done Mar 14, 2021
@szapp szapp added this to Hook engine function in Fix templates Mar 15, 2021
@szapp szapp added the impl: hook engine func This issue requires hooking engine functions. label Mar 16, 2021
@szapp szapp moved this from Hook engine function to Unsorted in Fix templates Mar 16, 2021
@szapp szapp moved this from Unsorted to NPC function in Fix templates Mar 17, 2021
szapp added a commit that referenced this issue Apr 20, 2021
@szapp szapp moved this from NPC function to NPC behavior in engine in Fix templates Apr 26, 2021
@Quintus24
Copy link

Hello, everyone!

I am sorry for spamming a closed report, I wasn't sure if I should ask here or open a new report.

I noticed that NPC "Scorpio" is equipping some of his merchandise.
In Chapter 4 he has the Heavy Crossbow and Guards' Sword equipped.
In Chapter 5 he equips the Vengeful Steel.

Is the fix supposed to take effect on him as well?

  1. Scorpio in Chapter 4.
    Scorpio @ Chapter 4

  2. Scorpio in Chapter 5.
    Scorpio @ Chapter 5

Regards,
Quintus24

@szapp
Copy link
Collaborator

szapp commented Jan 22, 2022

@Quintus24 Thanks for the report. Judging from the script this should not happen. I will try to reproduce this.

In the meantime some notes:

  1. This fix only takes effect if the NPC did not already equip the weapon, i.e. it will not retrospectively unequip weapons. That means, it could have happened if the G1CP was not installed for a period of time.
  2. After you verified the integrity of your game files, the G1CP may have been removed. Can you confirm, that the version string in the menu, i.e. G1CP 1.2 (or similar) is still there (and was never gone in between)?
  3. Each fix of the G1CP is only applied if it does not interfere with the underlying mod or third party tools. It may be that this fix here is simply not applied. A way to check this is via the in-game console by entering fix status 59.

Nevertheless, we will check on our side if we can reproduce the error.

@Quintus24
Copy link

Hi @szapp,

  1. I was still in Chapter 1 when I first installed the G1CP, never removed it, just a reinstall yesterday to actually double check this issue.
  2. Yes, the G1CP 1.2 version string was always displayed in the main menu even after verifying the integrity of the files.
  3. Just the usual, Steam version with the SystemPack incorporated in it, nothing else, however, I checked the console and it said "Fix # 59 is active".

I checked a few more merchants and noticed that only Sharky seems to have equipped a crossbow, which I sold him.
However, he never had a ranged weapon equipped in the first place, additionally, he equipped it when I was not around him.

Best Regards,
Quintus24

@szapp szapp moved this from Modify NPC behavior in engine to Hook engine function in Fix templates Feb 6, 2022
@szapp szapp moved this from Hook engine function to Some NPC Test? (Todo adjust name) in Fix templates Feb 6, 2022
@szapp szapp moved this from NPC manual test (TODO rename) to NPC auto test (TODO rename) in Fix templates Feb 6, 2022
@szapp szapp moved this from NPC auto test (TODO rename) to NPC manual test (TODO rename) in Fix templates Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impl: hook engine func This issue requires hooking engine functions. type: session fix The fix for this issues is persistent across a session.
Projects
Fix templates
NPC manual test (TODO rename)
v1.0.0
  
Done
Development

Successfully merging a pull request may close this issue.

3 participants