Skip to content

Conversation

@Unde-R
Copy link
Contributor

@Unde-R Unde-R commented Sep 7, 2020

Why we need It?

Well looks like we have onVehicleExplode and many other vehicle events but we don't have this one, also as requested in issue #1426, this PR introduces a new event in MTA since not everyone can use cols etc... and It's better than adding a timer or calling onClientRender or something to check if the element isInWater.
(Also this event is triggered for unoccupied vehicles too)

Functionality

 function teteVehicle(thep)
 local x, y, z = getElementPosition(thep)
 teteCar = createVehicle(411, x+2, y, z)
 addEventHandler("onVehicleDrown", teteCar, function()
 outputChatBox("rip Tete car")
 end)
 end
 addCommandHandler("tetecar", teteVehicle)

This event is triggered when a vehicle drowns.

Screenshots

tete

@TheNormalnij
Copy link
Member

I think that this event need to be complex. I see this like a onElementWaterInteract what will works with peds, vehicles and objects (maybe) and has bWaterInOrOut parameter.

@Unde-R
Copy link
Contributor Author

Unde-R commented Sep 7, 2020

for objects not worth it.
We'll need to get a proper event name

@Unde-R
Copy link
Contributor Author

Unde-R commented Sep 8, 2020

@qaisjp any ideas?

@StrixG StrixG added the enhancement New feature or request label Sep 11, 2020
Comment on lines 423 to 424
CLuaArguments Arguments;
pVehicle->CallEvent("onVehicleDrown", Arguments);
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a convention in the code more or less, afaik.
But I'd use {} there, or CLuaArguments() (but {} should work)
Not even sure why CallEvent doesn't default construct CLuaArguments

Comment on lines 161 to 162
CLuaArguments Arguments;
pVehicle->CallEvent("onVehicleDrown", Arguments);
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here

Co-authored-by: Nikita Obrekht <obrekht@gmail.com>
Copy link
Contributor

@qaisjp qaisjp left a comment

Choose a reason for hiding this comment

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

I think that this event need to be complex. I see this like a onElementWaterInteract what will works with peds, vehicles and objects (maybe) and has bWaterInOrOut parameter.

I agree with @TheNormalnij's comment (copied above, from here: #1657 (comment))

This onVehicleDrown event doesn't look like it's detecting when a vehicle drowns, but when it interacts with water. Changing the event name now would make this a matter of finding a reason to merge code that accidentally doesn't work as intended.

Please create an issue proposing what feature you want to implement. In that issue, once we've discussed the what the feature should look like, we can discuss whether the code here is appropriate for that. In your issue, please try and provide answers to the questions written here. (Yes, it's a link to Windows Calculator's repo. See #887 for more context on what a "proposal" means.)

Unde-R and others added 3 commits October 20, 2020 21:34
Co-authored-by: saml1er <10183157+saml1er@users.noreply.github.com>
Co-authored-by: saml1er <10183157+saml1er@users.noreply.github.com>
Co-authored-by: saml1er <10183157+saml1er@users.noreply.github.com>
@Unde-R
Copy link
Contributor Author

Unde-R commented Oct 28, 2020

I think that this event need to be complex. I see this like a onElementWaterInteract what will works with peds, vehicles and objects (maybe) and has bWaterInOrOut parameter.

I agree with @TheNormalnij's comment (copied above, from here: #1657 (comment))

This onVehicleDrown event doesn't look like it's detecting when a vehicle drowns, but when it interacts with water. Changing the event name now would make this a matter of finding a reason to merge code that accidentally doesn't work as intended.

Please create an issue proposing what feature you want to implement. In that issue, once we've discussed the what the feature should look like, we can discuss whether the code here is appropriate for that. In your issue, please try and provide answers to the questions written here. (Yes, it's a link to Windows Calculator's repo. See #887 for more context on what a "proposal" means.)

sure even if #1426 exists?

@TheNormalnij
Copy link
Member

GTA SA store IN_WATER state in physical flags. You can add onClientElementPhysicalStateChanged ( oldState, newState ) for peds/vehicles/obejct. oldState - physical flags before GTA call ProcessControl, newState - after ProcessControl. You can find ProcessControl hooks in multiplayer_keysync.cpp

@TheNormalnij
Copy link
Member

If you want to add serverside event, you don't need save last isWater state.

if (flags.data.bIsInWater != pVehicle->IsInWater())
{
    ...
}

Correct me if it's wrong

@Unde-R
Copy link
Contributor Author

Unde-R commented Oct 31, 2020

If you want to add serverside event, you don't need save last isWater state.

if (flags.data.bIsInWater != pVehicle->IsInWater())
{
    ...
}

Correct me if it's wrong

cheers gonna try I've seen flag code somewhere I'll try it

@TheNormalnij
Copy link
Member

image

@StrixG StrixG changed the title Add onVehicleDrown event Add onElementWaterInteract event Oct 31, 2020
@patrikjuvonen patrikjuvonen marked this pull request as draft December 29, 2020 01:03
@github-actions
Copy link
Contributor

github-actions bot commented Jan 7, 2022

This draft pull request is stale because it has been open for at least 90 days with no activity. Please continue on your draft pull request or it will be closed in 30 days automatically.

@github-actions github-actions bot added the stale Inactive for over 90 days, to be closed label Jan 7, 2022
@github-actions github-actions bot closed this Feb 7, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Feb 7, 2022

This draft pull request was closed because it has been marked stale for 30 days with no activity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request stale Inactive for over 90 days, to be closed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants