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

No murder bounty when a player follower commits murder (Bug #2852) #921

Closed
wants to merge 7 commits into from
Closed

Conversation

NeveHanter
Copy link
Contributor

This commit fixes https://bugs.openmw.org/issues/2852. I've tested that change and now it properly raises bounty when summon (Skeleton) or the follower (High-Pockets) kills an NPC.

// Doesn't handle possible edge case where no one reported the assault, but in such a case,
// for bystanders it is not possible to tell who attacked first, anyway.
if (victimStats.getCrimeId() != -1)
commitCrime(attacker, victim, MWBase::MechanicsManager::OT_Murder);
commitCrime(attackerIsPlayerFollowerOrSummon ? player : attacker, victim, MWBase::MechanicsManager::OT_Murder);
Copy link

Choose a reason for hiding this comment

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

This doesn't seem to be in line with the original game. Try this:

npc1->aifollow, player, 0,0,0,0
npc1->startcombat, npc2

No bounty is added.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, You're right, my bad, only summons increases bounty (maybe this should be fixed by some compatibility flag, isn't it a some vanilla bug?).

Copy link

Choose a reason for hiding this comment

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

I'm not talking about a difference between summons/normal followers, but the fact that any follower attacking first doesn't produce a bounty for the player.

I suppose that this is a) intended to work this way (kinda makes sense that the player isn't responsible for the follower's actions, if they acted on their own accord) or b) no one really considered this case since followers usually don't attack a peaceful NPC on their own, except maybe for scripted events.

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've tested startcombat, npc_id on the summoned skeleton and high pockets reference and after it killed the NPC it got me 1000 bounty with vanilla. Also I've checked if after I attacked an NPC, and then summon or high pockets killed it, i also got 1000 bounty. The same after I've summoned summon after attacking the NPC, it also got me 1000 bounty.

@ghost
Copy link

ghost commented Apr 4, 2016

Just checked again, you are right there is a difference in the handling of summons and normal followers. Weird.

But, the current state of this PR no longer fixes the original bug report.

The expected behaviour with aifollow-ing actors is:

  • Player attacks first, follower kills: murder bounty should be added (this is what the bug report was about).
  • Follower attacks first and kills without the player getting involved: no murder bounty should be added.

@NeveHanter
Copy link
Contributor Author

NeveHanter commented Dec 11, 2016

Hello again @scrawl,

I want to complete this PR at last.

I've checked vanilla behaviour again with following actors:

  • summon,
  • aifollow'ed NPC,
  • aiescort'ed NPC,
  • recruited High-Pockets

in the following scenarios:

  • by issuing startcombat command on different NPC in crowded room,
  • by me attacking NPC

In every of the eight scenarios, I've got 1000 bounty for "siding with me" actor killing other NPC, but in startcombat scenario no-one of the other NPCs in the room attacked me afterwards getting the bounty (of course guards wanted to throw me in jail anyway)

And this conflicts with your last comment:

Follower attacks first and kills without the player getting involved: no murder bounty should be added.

Additionally I've checked what happens if aifollow'ing/aiescort'ing actor summon kills an NPC, and when I've attacked that NPC and "follower" summon killed him, I've got the bounty, but when I've used startcombat on my follower and his summon killed the NPC, I've got no bounty at all.
This looks like a vanilla bug for me, because when following NPC kills another NPC, I got the bounty, but when his summon kills an NPC, I didn't get it.

@ghost
Copy link

ghost commented Dec 13, 2016

Looks like you're right. I don't remember what testing I did before, maybe I was impatient and used 'sethealth 0' after the initial attack to speed things up, but in doing so made the game not attribute the death to the attacking actor.

Feel free to go ahead with implementation.

…ills NPC (Bug #2852)

Made damage spell effects casted by actors different than the player, also notify about target death (allows spellcasting summons/siding actors commit crime).
Made player summoned creatures, actors siding with the player (AiEscort & AiFollow) and their summoned creatures commit crime in name of the player.
Prevent follower summoned creature attack the player when player or his follower/summoned creature commited the crime.
@NeveHanter
Copy link
Contributor Author

@scrawl I've probably made every needed corrections and prepared code for the review.

@ghost
Copy link

ghost commented Dec 17, 2016

The hasSummonedCreature checks seem redundant, the summoned creature would also have a follow package.

@NeveHanter
Copy link
Contributor Author

NeveHanter commented Dec 17, 2016

Right, my bad as I've had problem in my implementation before and thought it was because summoned creature package wasn't there, everything seems ok after removal 👍

@@ -374,9 +374,13 @@ namespace MWMechanics
if (LOS)
{
MWBase::Environment::get().getMechanicsManager()->startCombat(actor1, actor2);
if (!againstPlayer) // start combat between each other
if (!againstPlayer) // start combat between each other if engaging actor is not one of the player followers
Copy link

Choose a reason for hiding this comment

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

Can you explain why this change is necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is done to prevent not-committing the crime when startcombat is issued i.e. when I issue "startcombat elone" on arrille, he summons the skeleton and elone starts fighting it without skeleton commiting the crime

Copy link
Contributor Author

Choose a reason for hiding this comment

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

More specifically this is done because actorAttacked() checks for !seq.isInCombat(attacker) before calling commitCrime().

Which will of course fail when startcombat is issued in here also for actor2.

Copy link

Choose a reason for hiding this comment

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

start combat between each other if engaging actor is not one of the player followers

That's not how vanilla works, AFAIK. Can you think of a different solution to the problem?

Copy link

Choose a reason for hiding this comment

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

Please check for any conflicts/overlaps with the just merged PR #1174.

@@ -12,6 +12,8 @@
#include "aisequence.hpp"
#include "drawstate.hpp"

#include "../mwworld/class.hpp"
Copy link

Choose a reason for hiding this comment

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

Forgot to remove include?

@@ -1094,6 +1082,7 @@ namespace MWMechanics
}

// Make surrounding actors within alarm distance respond to the crime
// TODO: Vanilla doesn't make neighbors attack player if one of his follower/summon attacks first (probably vanilla bug)
Copy link
Contributor

@Allofich Allofich Dec 19, 2016

Choose a reason for hiding this comment

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

This seems to be because in vanilla followers only give you a crime for a kill, not for attacking. The same thing will happen if you kill an NPC instantly. You will only get the murder bounty and nearby NPCs won't attack you. I guess the code to have them respond is tied to the "on attacked" crime code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So now, there's question, should we follow vanilla or not regarding this?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think it matters that much, because normally your follower won't start combat with a non-aggressive actor anyway. They just respond to ones that are aggressive to you or them. So this would only matter if there is a script using "startcombat" on the follower, or if the player is using console commands. I suppose the best thing would be to follow vanilla, just in case there is a mod that uses startcombat to make the player follower attack and expects the vanilla behavior, but it seems like a very rare case to me.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah that sounds like the right vanilla behavior. When getting boots of blinding speed, the woman I was helping would just zip off to kill enemies which was annoying to say the least...

@jefhai
Copy link
Contributor

jefhai commented Dec 22, 2016

@NeveHanter are you working on this issue? https://bugs.openmw.org/issues/2852

If so please mark yourself as the assignee. We wouldn't want others doubling up on this task.

Edit: Resolved.

@NeveHanter
Copy link
Contributor Author

NeveHanter commented Dec 23, 2016

@scrawl Any more comments on this PR or above comment thread about engageCombat changes?

@Allofich
Copy link
Contributor

I think maybe he's waiting to merge things until the new release is out. Is that right, scrawl? Or he's just busy with other things.

@Allofich
Copy link
Contributor

Allofich commented Dec 24, 2016

By the way, I just noticed that in vanilla, the player is given crime bounties for murders that happen during fighting even from the opponents hitting each other with area-effect spells. You can test with Arrille because he will use some spells that can hit other NPCs. The player even gets a murder bounty if you select a non-aggressive NPC A, give it "startcombat" on NPC B who is fighting the player due to a crime, and NPC B is killed. I guess NPCs responding to a crime are labeled to be possible murder victims, and the player gets a murder bounty no matter how such labeled NPCs die. Right now I don't know if we should follow that implementation.

@Allofich
Copy link
Contributor

I guess NPCs responding to a crime are labeled to be possible murder victims, and the player gets a murder bounty no matter how such labeled NPCs die.

Any opinions? I suppose I support doing it the same as vanilla, which I think is a good policy for laying a base that could potentially have alternate implementations added on top of it but will get us closer to 1.0. In this case, you could imagine that the law of Morrowind blames the initiator of an assault for all deaths that result from it, even if they didn't directly do it.

@ghost
Copy link

ghost commented Jan 1, 2017

Any opinions? I suppose I support doing it the same as vanilla, which I think is a good policy for laying a base that could potentially have alternate implementations added on top of it but will get us closer to 1.0. In this case, you could imagine that the law of Morrowind blames the initiator of an assault for all deaths that result from it, even if they didn't directly do it.

Makes sense to me. In some cases (i.e. multiple damage effects in the same frame) you couldn't really tell who made the kill anyway.

@@ -420,6 +420,15 @@ namespace MWMechanics

if (LOS)
Copy link
Collaborator

Choose a reason for hiding this comment

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

You should add "{" after this line or remove "}" from 431 line to compile OpenMW properly.

@ghost
Copy link

ghost commented Jul 30, 2017

Current blocker is #921 (review) , I think.

@Capostrophic
Copy link
Collaborator

This needs some massive cleanup, the actual changes here that are necessary to resolve the issue are somewhat small.

@akortunov
Copy link
Collaborator

This PR can be closed now.

@psi29a
Copy link
Member

psi29a commented Jun 19, 2018

Closing on advice from @akortunov

@psi29a psi29a closed this Jun 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants