otland / forgottenserver

A free and open-source MMORPG server emulator written in C++
https://otland.net
GNU General Public License v2.0
1.59k stars 1.06k forks source link

onDeath(..., mostDamageKiller, ... , mostDamageUnjustified) bug #4204

Open krafttomten opened 2 years ago

krafttomten commented 2 years ago

### Steps to reproduce The core of the problem is that the killer does not get unjustified kills where he should and that the mostDamageUnjustified never has any other value than false, which makes it useless. There are several ways of showing the different nuances of the problem:

Setup:

  1. Modify player_death.lua and/or drop_loot.lua by adding print("drop_loot:", lastHitUnjustified, mostDamageUnjustified) somewhere where it always gets executed (e.g. line 2 for drop_loot.lua and line 5 for player_death.lua)

Procedure 1

  1. Get two players, one killer and one victim
  2. Use the killer to deal damage > 50% of the victims hp
  3. Summon creatures to finish off the victim (e.g. Fire Elemental)
  4. Kill the victim using fire elementals
  5. Look at what gets printed in the terminal: it says false false , which means that neither the last hit nor the most damage was unjustified

Procedure 2

  1. Get three players, two killers and one victim
  2. Use the first killer to deal damage > 50% of the victims hp
  3. Use the second killer to finish off the victim
  4. Look at what gets printed in the terminal: it says true false , which means that the last hit was unjustified, but not the most damage one

Procedure 3

  1. Get two players, one killer and one victim, and also one monster without master (i.e. a regular monster)
  2. Use the killer to deal damage > 50% of the victims hp
  3. Use the victim to "flee" from the killer and get finished off by the monster
  4. Look at what gets printed in the terminal: it says false false , which means that neither the last hit nor the most damage was unjustified

### Behaviour Procedure 1: ### Expected behaviour

The terminal should print true, true The killer should get an unjustified kill, which he does - as expected.

### Actual behaviour The terminal prints false, false The killer was a monster, but it had a master which commanded its behaviour, so for the SQL entry to make sense, it should be registred as an unjustified kill because the last hit was caused by a player. Most damage was dealt by a player, so this should also register as unjustified. The killer gets an unjustified kill, as expected.

Procedure 2: ### Expected behaviour The terminal should print true true Both killers should get an unjustified kill

### Actual behaviour The terminal prints true false Only the lasthit killer gets an unjustified kill

Procedure 3: ### Expected behaviour The terminal should print false true The mostDamageKiller should get an unjustified kill

### Actual behaviour The terminal prints false false The killer does not get an unjustified kill

Environment

TFS 1.4, Windows, but it should not be environment specific

Reproduction environment

Not necessary, the only change that is recommended is a print, to visualize the problem.

marmichalski commented 2 years ago

As per lastHitUnjustified, this looks like an easy pick.

If you see here: https://github.com/otland/forgottenserver/blob/eb5f5d68a85eb5f8e5ade23153d699e93d4287a3/src/creature.cpp#L657 this is always false for non-players (monsters, including summons etc).

What you could try, is changing this method: https://github.com/otland/forgottenserver/blob/eb5f5d68a85eb5f8e5ade23153d699e93d4287a3/src/creature.cpp#L1130-L1142

into:

bool Creature::onKilledCreature(Creature* target, bool)
{
+      bool unjustified = false;
    if (master) {
-               master->onKilledCreature(target);
+       unjustified = master->onKilledCreature(target);
    }

    // scripting event - onKill
    const CreatureEventList& killEvents = getCreatureEvents(CREATURE_EVENT_KILL);
    for (CreatureEvent* killEvent : killEvents) {
        killEvent->executeOnKill(this, target);
    }
-       return false;
+   return unjustified;
}