Fix unwanted detaching when damage = 0

[programmerjake]

Add compact, short information about your PR for easier understanding:

• Goal of the PR
  don't detach player on set_hp with new hp equal to old hp
• How does the PR work?
  skips detaching when the hp is positive
• Does it resolve any reported issue?
  not that I could find

To do

This PR is Ready for Review.

How to test

play hamlet's quest as of sep 28, 2019
ride boat in water and observe not detaching after a second or so

backporting to stable

Not sure if the bug is in stable (didn't test), if so, this pull request should be backported to stable as well.

[sfan5]

What is the point of the else clause anyway? Why are players supposed to detach when they receive zero damage?

[SmallJoker]

The idea is to detach GenericCAOs when they die.
For objects other than the current player this happens when they're removed from the environment (handleCommand_ActiveObjectRemoveAdd). For the current players this has to be handled in a different way - where they take damage or where the HP updates.
if (m_hp == 0) would be the correct check there.

[ghost]

Assuming you want the player to detach on die, correct?

…

On Sun, Sep 29, 2019, 9:02 AM SmallJoker ***@***.***> wrote: The idea is to detach GenericCAOs when they die. For objects other than the current player this happens when they're removed from the environment (handleCommand_ActiveObjectRemoveAdd). For the current players this has to be handled in a different way - where they take damage or where the HP updates. if (m_hp == 0) would be the correct check there. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#8999?email_source=notifications&email_token=AFS4UYQU7ZRYECK3EUZFSE3QMCROFA5CNFSM4I3QOZPKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD73UHKY#issuecomment-536298411>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFS4UYUCJGGM72V3YXGL5X3QMCROFANCNFSM4I3QOZPA> .

[programmerjake]

should I change the commit message to match?

m_hp <= 0 also triggers in the case that m_hp goes negative for some reason and is generally not any more expensive, hence why I used it.

[SmallJoker]

@programmerjake No, the commit subject and body can be adjusted on merge.
However it would be better if the detach code (clear parent/child) is run every time when m_hp is zero, not just when there was no damage at all.

[ClobberXD]

m_hp <= 0 also triggers in the case that m_hp goes negative for some reason and is generally not any more expensive, hence why I used it.

m_hp can't go below 0, as it's of type u16 (unsigned short on most systems, IIRC).

[programmerjake]

m_hp <= 0 also triggers in the case that m_hp goes negative for some reason and is generally not any more expensive, hence why I used it.

m_hp can't go below 0, as it's of type u16 (unsigned short on most systems, IIRC).

ah, ok. didn't check the type.

[programmerjake]

Changed the detach condition to whenever m_hp == 0 even if damage is non-zero

[programmerjake]

spurious build failure due to ubuntu package checksums not matching

[ghost]

This broke old mount code, I believe. I.e. mobs_redo.
It detached when minetest.register_on_dieplayer to set some code, as visual_size, but now not.
:-(