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

Add onElementHealthChange & onClientElementHealthChange events #3278

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

1Lorenzo-1
Copy link

New health events onElementHealthChange & onClientElementHealthChange

Features:

  • Server-Side Event: onElementHealthChange

    • Triggered when the health of an element changes on the server side.
    • Parameters:
      • source: The affected element.
      • arg1: Old health value.
      • arg2: New health value.
      • client (optional): The client responsible for the change (if initiated from the client side).
  • Client-Side Event: onClientElementHealthChange

    • Triggered when the health of an element changes on the client side.
    • Parameters:
      • source: The affected element.
      • arg1: Old health value.
      • arg2: New health value.

Example Usage:

-- Server-Side
addEventHandler("onElementHealthChange", root,
    function (oldHealth, newHealth)
        if (client and client~=source and newHealth > 100) then 
            kickPlayer(client, "AC", "CHEATER")
        end 
    end
)

-- Client-Side
addEventHandler("onClientElementHealthChange", root,
    function (oldHealth, newHealth)
        if source==localPlayer then 
            iprint("your new/health: " .. newHealth)
        end 
    end
)

@TracerDS
Copy link
Contributor

#3152 duplicate

@1Lorenzo-1
Copy link
Author

#3152 duplicate

I should have looked up before starting it..

@znvjder
Copy link
Contributor

znvjder commented Dec 29, 2023

Sure, it might be a duplicate, but I can see that the health status is updated through the packet - that's actually a good idea. My PR has been abandoned... I don't think I even have local access to it (it's actually on the cloud and rotting).

@znvjder
Copy link
Contributor

znvjder commented Dec 30, 2023

I have run some test cases. The event is only triggered through function setElementHealth. Generally, we would like the event to be triggered in any case of health change.

@1Lorenzo-1
Copy link
Author

I have run some test cases. The event is only triggered through function setElementHealth. Generally, we would like the event to be triggered in any case of health change.

Tbh we got already (on[client]PlayerDamge) on[client]VehicleDamge,

So I dont think adding it would be nice
Then we should remove both others events?

@znvjder
Copy link
Contributor

znvjder commented Dec 30, 2023

No, no... These events must stay. I'll allow myself to quote Lpsd comment from my PR.

As Pirulax mentioned, I think it would be nice for onElementHealthChange to trigger whenever element health changes, not just via setElementHealth, but also when changed in-game (from fall damage, explosion, etc). There should be a parameter for the event to indicate whether it was changed via script.

@1Lorenzo-1
Copy link
Author

No, no... These events must stay. I'll allow myself to quote Lpsd comment from my PR.

As Pirulax mentioned, I think it would be nice for onElementHealthChange to trigger whenever element health changes, not just via setElementHealth, but also when changed in-game (from fall damage, explosion, etc). There should be a parameter for the event to indicate whether it was changed via script.

It's such a nice thing.. but why?
i do still think we don't need it.. but i can do it

@srslyyyy
Copy link
Contributor

srslyyyy commented Dec 30, 2023

The events you've mentioned are meant only for streamed players/vehicles. Why exactly it should be as znvjder explained above? To avoid shenanigans of using extra logic, from event you'd expect to cover any health change, in any case. Otherwise it will be very inconvenient for scripter. Removing them doesn't make sense, as they have different purpose - either tweak how it works, disable damage or completely overhaul it.

@1Lorenzo-1
Copy link
Author

The events you've mentioned are meant only for streamed players/vehicles. Why exactly it should be as znvjder explained above? To avoid shenanigans of using extra logic, from event you'd expect to cover any health change, in any case. Otherwise it will be very inconvenient for scripter. Removing them doesn't make sense, as they have different purpose - either tweak how it works, disable damage or completely overhaul it.

well i'll start rn with it doing what you want

@1Lorenzo-1
Copy link
Author

The events you've mentioned are meant only for streamed players/vehicles. Why exactly it should be as znvjder explained above? To avoid shenanigans of using extra logic, from event you'd expect to cover any health change, in any case. Otherwise it will be very inconvenient for scripter. Removing them doesn't make sense, as they have different purpose - either tweak how it works, disable damage or completely overhaul it.

I believe it will only be server-side events. Otherwise, client-side damage is challenging to work with. I have tried everything in Client/game_sa/CPedSA.cpp, but nothing worked. Btw I'll try my best to make it. If someone has any idea how it could be done, that's going to help me much.

@Pirulax
Copy link
Contributor

Pirulax commented Jan 5, 2024

You could do a check each frame perhaps.
It's impossible to track every instance of health change down, so that's your best bet.
Yes, it's going to be a frame late but oh well....

@tederis tederis added the enhancement New feature or request label Feb 18, 2024
@Nico8340
Copy link
Contributor

Any updates on this?

@1Lorenzo-1
Copy link
Author

Any updates on this?

No, somehow I can't do it... I've tried it in various ways, but there are so many FPS drops when a PLAYER got health changed. Maybe I'll close it, hoping that next time someone with more experience will make better one. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants