removeEventHandler

[Cronoss]

I made these functions because I want to avoid low fps and lag in my server, in my mind this was a great idea because "if I delete the handler from a "onClientRender" function I'm deleting unnecesary stuff" but I was thinking about it recently... and I have the question, is this system efficient or not?

function hideSpeedometer()
    removeEventHandler("onClientRender", root, speedoMeterGUI) ----------------removes the eventhandler 
end

function showSpeedometer()
    if not isEventHandlerAdded("onClientRender", root, speedoMeterGUI) then ----------if event handler it is not added then the function adds it
        addEventHandler("onClientRender", root, speedoMeterGUI) 
    end
end

----------------------------------------------

function A1(player)
   vehicle = getPedOccupiedVehicle(player)
   if getPedOccupiedVehicleSeat(player)==0 or getPedOccupiedVehicleSeat(player)==1 then
       showSpeedometer() -------call to create the handler
   end
end
addEventHandler("onClientVehicleEnter", root, A1)

function A2()
    hideSpeedometer() -----call to remove the handler
end
addEventHandler("onClientVehicleStartExit", root, A2)

I don't know if it is a good idea or I'm messing up the server with this, I need answers because I'm bad at optimizing 

Edited March 7, 2022 by Cronoss

[βurak]

actually you can solve this with a single variable instead of using "isEventHandlerAdded" 

Since there are more operations in the isEventHandlerAdded function, you can do it economically with a single variable.

local isSpeedoMeterVisible = false --speedometer visibility status

function hideSpeedometer()
    if(isSpeedoMeterVisible) then -- Is the speedometer visible?
       removeEventHandler("onClientRender", root, speedoMeterGUI)
    end
end

function showSpeedometer()
    if(isSpeedoMeterVisible == false) then -- Is the speedometer invisible?
       addEventHandler("onClientRender", root, speedoMeterGUI)
    end
end

----------------------------------------------

function A1(player)
   vehicle = getPedOccupiedVehicle(player)
   if getPedOccupiedVehicleSeat(player)==0 or getPedOccupiedVehicleSeat(player)==1 then
       showSpeedometer() -------call to create the handler
   end
end
addEventHandler("onClientVehicleEnter", root, A1)

function A2()
    hideSpeedometer() -----call to remove the handler
end
addEventHandler("onClientVehicleStartExit", root, A2)

 

Edited March 7, 2022 by Burak5312

[Cronoss]

How do I change the local isSpeedoMeterVisible status? 

[Citizen]

@Burak5312There is no issue to solve, the original code is working and has no perf problems. Also your code won't work, you forgot to flip the boolean inside show and hide functions.

@CronossYour code is fine. And you are right, technically it's better because "onClientRender" won't call your lua function for nothing (if the speedometer is hidden).

local isSpeedoVisible = false

function drawSpeedometer()
    if not isSpeedoVisible then return end -- cancel immediatly

    -- drawing
end
addEventHandler("onClientRender", root, drawSpeedometer)

------

function A1(player)
    if player == localPlayer then return end -- you forgot this btw or else other players will trigger it too
    vehicle = getPedOccupiedVehicle(player)
    if getPedOccupiedVehicleSeat(player) == 0 or getPedOccupiedVehicleSeat(player) == 1 then
        isSpeedoVisible = true
    end
 end
 addEventHandler("onClientVehicleEnter", root, A1)
 
 function A2(player)
    if player == localPlayer then return end -- you forgot this btw or else other players will trigger it too
    isSpeedoVisible = false
 end
 addEventHandler("onClientVehicleStartExit", root, A2)

If you had something like this, it is fine too and the perf are so close that the difference can be ignored (but technically it's better to add and remove the event handler like in your original post).

If you are having perf issues with your speedometer, make sure you don't call get/set ElementData inside the drawing function. Those methods are expensive.

[Cronoss]

58 minutes ago, Citizen said:

If you are having perf issues with your speedometer, make sure you don't call get/set ElementData inside the drawing function. Those methods are expensive.

I modified the code a little bit and this how it looks now;

function showSpeedoMeterFunction(player)
    if player == localPlayer then return end
    vehicle = getPedOccupiedVehicle(player)
    if getPedOccupiedVehicleSeat(player) == 0 or getPedOccupiedVehicleSeat(player) == 1 then
        showSpeedometer() ------add the render
        isSpeedoVisible=true
        if getPedOccupiedVehicleSeat(player) == 0 then
            timer = setTimer(function() ------this part it's because the fuel-system for the vehicle
                if getVehicleEngineState(vehicle)==true then -----if the engine it's on
                triggerServerEvent("chekG", localPlayer, localPlayer, vehicle)
                end
            end, 20000, 0) ---------the timer update the fuel every 20 seconds
        end
    end
 end
 addEventHandler("onClientVehicleEnter", root, showSpeedoMeterFunction)
 
 function hideSpeedoMeterFunction(player)
    if player == localPlayer then return end
    hideSpeedometer() ---- removes the render
    isSpeedoVisible=false 
    if getPedOccupiedVehicleSeat(player)==0 then -----check if the player who left the vehicle was the driver
        killTimer(timer) --------kills the fuel-check timer 
    end
 end
 addEventHandler("onClientVehicleStartExit", root, hideSpeedoMeterFunction)

I want to know if this may be a problem. I'm using "onElementDataChange" for the fuel system detection in the drawImage;

function checkDataFuel(Fuel, oldValue, newValue)
    if vehicle then
        if Fuel=="Fuel" then
            if tonumber(newValue) ~= tonumber(oldValue) then
                percentG = tonumber(newValue) ------- I convert the new value from "Fuel" data to a number
                iprint("changed to: "..percentG)
            end
        end
    end
end
addEventHandler("onClientElementDataChange", root, checkDataFuel)

function getCarStateColor() --------this function it's called in the ("onclientrender") function
    local fuelVeh = percentG or 100 -------I use the "percentG" value in number to change the image on client render
    local engineON = getVehicleEngineState(vehicle)
    if (engineON==true) then
        local g = (255/100) * fuelVeh
        local r = 255 - g
        local b = 0
     
        return r, g, b
    elseif not (fuelVeh) and (engineON==false) then
        return 0, 255, 0
    else
        return 106, 106, 106
    end
end

Why I'm using it like this way? I want to change the color of the image with the fuel data number, when the fuel start to decrease, the image starts to turn in red

 

It is a good way to make what I want or I should change this? I didn't used getElementData because Burak told me it would be a bad idea, and I know it is. That's why I changed to "onElementDataChange", but I don't know if this is right 

[Citizen]

if player == localPlayer then return end

UPS ! My bad, I meant ~= there

if player ~= localPlayer then return end

(replace in both)

Okay yeah, here is your problem:

You are creating 1 new timer per frame (x30 per sec at 30fps) so after 20 seconds of calmness you are calling triggerServerEvent every frame. Does that make sense ?

What I would do is:

-- on server side:

• listen to onVehicleEnter and when someone enters it, create the 20sec timer to update the fuel. Remember the timer (a table or a "not synced" element data and using the vehicle element as index/key to retreive it later)
• To update the fuel: make the necessary checks (engine state = ON etc) and use element data (element data get so much hate and I don't agree with all that hate, its perfect to sync an element data between server and clients when used correctly)

  local currentFuel = getElementData(vehicle, "fuel") or 0
  setElementData(vehicle, "fuel", currentFuel - 5) -- updating the fuel on server

• Listen to onVehicleExit and get back the timer you created (and that you stored) and kill that timer

-- on client side:

• Listen to onVehicleEnter and onVehicleExit to show or hide the speedometer (= add or remove the onClientRender)
• Yes you can use "onClientElementDataChange" to listen and update the fuel value updates for drawing

  local currentVeh = nil
  local currentFuel = 0
  
  function checkDataFuel(dataName, oldValue, newValue)
      if dataName ~= "fuel" or source ~= currentVeh then return end -- cancel if it's not "fuel" update or if it's not for our vehicle
      currentFuel = newValue
  end
  addEventHandler("onClientElementDataChange", root, checkDataFuel)
  
  function enterVehicle(player, seat)
      if player ~= localPlayer then return end
      if seat == 0 or seat == 1 then
          currentVeh = source
          currentFuel = getElementData(source, "fuel") -- making sure we show the right value at 1st frame
          addEventHandler("onClientRender", root, drawSpeedo) -- show speedo
          isSpeedoVisible = true
      end
  end
  addEventHandler("onClientVehicleEnter", root, enterVehicle)
  
  function exitVehicle(player)
      if player ~= localPlayer then return end
      currentVeh = nil
      if isSpeedoVisible then
          removeEventHandler("onClientRender", root, drawSpeedo) -- hide speedo
          isSpeedoVisible = false
      end
  end
  addEventHandler("onClientVehicleStartExit", root, exitVehicle)
  
  function drawSpeedo()
      if not currentVeh then return end -- should never happen, just in case
  
      --[[ 
          draw vehicle speed and fuel here. You have access to these variables to draw:
          currentVeh
          currentFuel
      ]]
  end

I gave you a lot of client code to show you how it should look like with the new logic (your latest version was going a bit wild)

For the server part, I let you try with the instructions I gave you. 

Note: After all that, the last optimization would be to not call getVehicleEngineState at every frame (in your getCarStateColor function) but it's a minor optimization, the biggest gain is to not spam the triggerServerEvent by implementing the new logic I'm suggesting.

[Cronoss]

3 hours ago, Citizen said:

After all that, the last optimization would be to not call getVehicleEngineState at every frame (in your getCarStateColor function) but it's a minor optimization

I made the code with the new logic and it works pretty good but the only problem it's that I only want to show the color when the vehicle engine it's ON, so deleting that part could be "weird" to see. (fuel "light" it's ON while the vehicle it's OFF?) anyway, I just want to know if "minor optimization" means that it doesn't matter too much or you are trying to say that it's nothing compared to the "triggerServerEvent" change. 

Also, I have a question about how to recognize if the player turns on the engine in this function...

function setFuelCar(thePlayer)
    local source = getPedOccupiedVehicle(thePlayer)
    if getPedOccupiedVehicleSeat(thePlayer)==0 then
        if getVehicleEngineState(source)==true then ---------if the engine it's on I start the timer
            local x,y,z = getElementPosition(source)
            local fx,fy,fz = x+0.05, y+0.05,z+0.05
            local fuelConsumption = 0.900
            local fuelVeh = getElementData(source, "Fuel") or 0
            if (fuelVeh <= 0) then
                setVehicleEngineState(source, false)
            else
                timerF = setTimer( function ()
                local enginePlus = 0.200
                local distance = getDistanceBetweenPoints3D(x,y,z,fx,fy,fz)
                local newFuel = tonumber((fuelVeh - (fuelConsumption*(distance+enginePlus))))
                setElementData(source, "Fuel", newFuel)
                end, 20000, 0)
            end
        end
    end
end
addEventHandler("onVehicleEnter", root, setFuelCar)

I modified the original script following your instructions and it's very different now, that's why I lost the possibility to check if the vehicle's engines it's on or off (AFTER the player enters) to start the timer, I don't know if there exist a command or handler like "onVehicleEngineChange" to check when the player turn on or turn off their car but i'm needing an alternative to this, no need code, just ideas of how to do it (if it is possible to make inside that function)  also, thank you for your help, it's very appreciated

Edited March 8, 2022 by Cronoss

[Citizen]

On 08/03/2022 at 06:30, Cronoss said:

I just want to know if "minor optimization" means that it doesn't matter too much or you are trying to say that it's nothing compared to the "triggerServerEvent" change.

Both.

function setFuelCar(thePlayer)
    local source = getPedOccupiedVehicle(thePlayer) -- (1)
    if getPedOccupiedVehicleSeat(thePlayer)==0 then -- (2)
        if getVehicleEngineState(source)==true then ---------if the engine it's on I start the timer  -- (3)
            local x,y,z = getElementPosition(source)
            local fx,fy,fz = x+0.05, y+0.05,z+0.05 -- (4)
            local fuelConsumption = 0.900
            local fuelVeh = getElementData(source, "Fuel") or 0
            if (fuelVeh <= 0) then
                setVehicleEngineState(source, false)
            else
                timerF = setTimer( function () -- (5)
                local enginePlus = 0.200
                local distance = getDistanceBetweenPoints3D(x,y,z,fx,fy,fz) -- (6)
                local newFuel = tonumber((fuelVeh - (fuelConsumption*(distance+enginePlus))))
                setElementData(source, "Fuel", newFuel)
                end, 20000, 0)
            end
        end
    end
end
addEventHandler("onVehicleEnter", root, setFuelCar)

1. You don't need to overwrite the source variable. It already exists thanks to the onVehicleEnter event. Have a look at: https://forum.multitheftauto.com/topic/33407-list-of-predefined-variables/#comment-337231
2. onVehicleEnter already gives you the seat number, you have to "grab" it too :

   function setFuelCar(thePlayer, seat)
       if seat == 0 then

3. Here in your code, this check is only done once, when entering the vehicle. You have to change it so that you create the fuel timer anyway when entering the vehicle at seat 0 but inside the timer function, the 1st thing you do is to check the engine state. If it's ON, do the newFuel calculation thing. And if fuel is <= 0 then set it to OFF.
4. (Why the +0.05 to each components ? seems totaly useless)
5. You can't use a global variable on the server side or else if another player enters another vehicle, it will start his vehicle fuel timer and override the value in timerF so you have no way to kill the timer of your vehicle later. Must be local and "link it" with your vehicle element: setElementData(source, "fuelTimer", timerF, false) (false because there is no need to sync this value with all connected clients, only the server needs to know and manipulate it).
6. You have to update the value of fx,fy,fz with the current x, y, z at the end to calculate the new distance traveled between point B and point C (20 secs later). Again, use element data to store that position for the next fuel consumption calculations (you can't use global variables).

[Cronoss]

3 hours ago, Citizen said:

 

1. You don't need to overwrite the source variable. It already exists thanks to the onVehicleEnter event. Have a look at: https://forum.multitheftauto.com/topic/33407-list-of-predefined-variables/#comment-337231
2. onVehicleEnter already gives you the seat number, you have to "grab" it too :

   function setFuelCar(thePlayer, seat)
       if seat == 0 then

3. Here in your code, this check is only done once, when entering the vehicle. You have to change it so that you create the fuel timer anyway when entering the vehicle at seat 0 but inside the timer function, the 1st thing you do is to check the engine state. If it's ON, do the newFuel calculation thing. And if fuel is <= 0 then set it to OFF.
4. (Why the +0.05 to each components ? seems totaly useless)
5. You can't use a global variable on the server side or else if another player enters another vehicle, it will start his vehicle fuel timer and override the value in timerF so you have no way to kill the timer of your vehicle later. Must be local and "link it" with your vehicle element: setElementData(source, "fuelTimer", timerF, false) (false because there is no need to sync this value with all connected clients, only the server needs to know and manipulate it).
6. You have to update the value of fx,fy,fz with the current x, y, z at the end to calculate the new distance traveled between point B and point C (20 secs later). Again, use element data to store that position for the next fuel consumption calculations (you can't use global variables).

 

1.- and 2.- Solved and fixed that part, i didn't realize that vehicleEnter gives the seat number

3.- I changed that part before your answer, I thought it would be a bad idea because the server basically starts the timer and ends when the engine it's off, like, "start, cancel, start, cancel, start, cancel", but with your reply I'm confident about the logic of the script

4.-  Got it

5.- I get it, but if i want to optimize this part I could use "removeElementData" inside  "onVehicleStartExit" function ? I mean; I'm overwritting the timer everytime the player enters in a  vehicle, and if he leaves the car... it is not neccesary to have that info saved in the server, right?

I'm thinking about something like this:

function removeFuelData(thePlayer, seat)
	if seat~=0 then return end 
    if getVehicleEngineState(source)==false then
        if isTimer(timerF) then killTimer(timerF) end -------Don't know if this is neccesary if I set the timer as you said ('setElementData(source,"fuelTimer", timerF')
    	removeElementData(source, "fuelTimer")
    end
end
addEventHandler("onVehicleStartExit", root, removeFuelData)

I still don't know how to use properly the setTimer and killTimer functions but I'm trying to understand

 

6.- Got it

 

Edited March 9, 2022 by Cronoss

[Citizen]

2.

Quote

i didn't realize that vehicleEnter gives the seat number

And even 1 more info if you check the wiki https://wiki.multitheftauto.com/wiki/OnVehicleEnter. It also can give you the player that you are kicking out of the vehicle to take its place (if there was a player already in ofc) as 3rd argument of your event handler.

3. and 4. okay perfect

5. Yes you can totally do that, even though the place this data takes on the server is very tiny. Even if you multiply that by 5,000+ potential vehicles. But yes, it's better and you are right.

if isTimer(timerF) then killTimer(timerF) end -------Don't know if this is neccesary if I set the timer as you said ('setElementData(source,"fuelTimer", timerF')

Yes you still have to kill it. removeElementData will only remove the timer from element (kind of unlinking) but the timer will still run in background.
Note: My setElementData for the fuelTimer had a 4th argument which was false (it's true by default if you omit it) and it's important to prevent it to be automatically sent to all clients connected to the server (if you have 500 players, this only call will make 500 calls to triggerClientEvent (well, not exactly but almost like if) which is a waste of CPU and bandwidth as we only need it on the server side).

Also, make sure you get your timerF using getElementData(source, "fuelTimer")

6. perfect

You are doing good by the way (understanding and implementing the suggestions), keep it up 

[Cronoss]

@Citizen

I know this may be in another section but I see that you have experience and I don't want to create a whole topic for a single question; why setElementData and getElementData functions are so hated and not recommended in MTA Community? I understand that if I want to save "custom" data I should use SQL, but I would prefer this:

function mySavedData(player, command, number, ...)
   if getElementData(player, "doIHaveCellphone")~=false then
    	---blablabla
   end
end
addCommandHandler("callsomeone", mySavedData)

Instead this:

function mySavedData(player, command, number, ...)
  	local name = getPlayerName(player)
  	local check = exports.mysql:_Query("SELECT cellphone FROM characters WHERE name=?", name)
  	if (#check>0) then
    	---blablabla
    end
end
addCommandHandler("callsomeone", mySavedData)

And... how do I know if I'm overusing the getelement and setelement functions? In my new fuel-system a lot of set element and get element has been created in the script, in first place I thought it shouldn't be like that but I think it's the only way a fuel system work properly

[Citizen]

No, SQL is not meant to replace element data, it is way slower than element data. What should replace element data is custom lua table using element as indices.

SQL is meant to persist data so that (for example) one player can load back his data. Like on an RP server, when someone leaves the server, his position, health, money weapons etc are saved into SQL tables, and when he joins back the next day (or even a month later) you load back his data so he can start from where he left.

Element data are only in RAM and are destroyed when the element is destroyed (for player elements, when they leave the server).

Element data are hated because by default, they are synced with all clients and the server so if a data is modified, even if only 1 player should care about that modification which is a waste of resources and bandwidth. One should replace them with lua tables and syncing them when needed using triggerServer/ClientEvent so you have control over what is synced and what isn't and its better for performance.

Imo if they are used correctly, they are perfect to store data regarding an element on said element using elementData .

For example, if I had to implement a trunk system, I'll defintely use elementData to store what is inside the trunk.

EDIT: Want to point out again that SQL queries are expensive, for the cell phone example, I would use SQL when the player logs in so I can load all his account data (and so the doIHaveCellphone value) and will call setElementData(player, "doIHaveCellphone", sqlrow['doIHaveCellphone'] or false) and then will only use setElementData to modify it (if needed) during his entire game session. Once the player leaves (or crash, or anything that makes the player disconnect => onPlayerQuit) I'll make the needed SQL queries to save all his data into SQL tables from the elementData.
You can't affort to make SQL queries for every data you need to get. Per player it will adds up and you will end making 100+ SQL queries per second with only 25 players and your server will cry. SQL queries are way more expensive than element data.