Mtasa-blue: engineFreeModel doesn't "free" itself after disconnecting a server

[MrDadosz]

Describe the bug
When server 1 doesn't free models and you connect server 2, client will have "reserved" slots for skins. When you save models to a table on server 1 for using it later without engineFreeModel function when resource is stopping, the function will always return false on server 2 at some point. You can't check even if ped model is valid by getValidPedModels() function - it will return default list of models in GTA (313).

To reproduce

1. connect to server with script:

    local possibleCustomSkins = {}
    for i = 1, 10 do -- you can change 10 to 10000 to get false after using engineRequestModel
        local id = engineRequestModel("ped")
        possibleCustomSkins[id] = true
    end
    outputChatBox(inspect(possibleCustomSkins))

return:

{
  [3] = true,
  [4] = true,
  [5] = true,
  [6] = true,
  [8] = true,
  [42] = true,
  [65] = true,
  [74] = true,
  [86] = true,
  [119] = true
}

1. restart that script first time:
   return:

{
  [3] = true,
  [4] = true,
  [5] = true,
  [6] = true,
  [8] = true,
  [42] = true,
  [65] = true,
  [74] = true,
  [86] = true,
  [119] = true
}

1. restart that script second time:

{
  [208] = true,
  [273] = true,
  [289] = true,
  [329] = true,
  [332] = true,
  [340] = true,
  [382] = true,
  [383] = true,
  [398] = true,
  [399] = true
}

1. disconnect with server 1 and connect to server 2

2. use this script on server 2:

    for i = 1, 10 do
        local id = engineRequestModel("ped")
        possibleCustomSkins[id] = true
    end
    outputChatBox(inspect(possibleCustomSkins))

return:

{
  [208] = true,
  [273] = true,
  [289] = true,
  [329] = true,
  [332] = true,
  [340] = true,
  [382] = true,
  [383] = true,
  [398] = true,
  [399] = true
}

it should return 3,4,5 etc

Expected behaviour
It should work like you use this function first time because you can't force other servers to fix their scripts.

Version
client: Multi Theft Auto v1.5.7-release-20348

[qaisjp]

I can reproduce this for reconnections, but for resource _restarts_ it sometimes works properly. Odd...

@MrDadosz, please note that the numbers returned can be anything, and does not need to be in any specific order. You need to check the total number of models that can be returned instead, like this:

addCommandHandler("s", function()
    id = -1
    t = {}
    while id do
        table.insert(t,id)
        id = engineRequestModel("ped")
    end
    local f = File("p"..getTickCount()..".txt")
    f:write(toJSON(t))
    f:flush()
    f:close()
    outputConsole("Length: " .. #t)
end)

[Pirulax]

other than you inserting a -1 id into the table, its fine.
@qaisjp

[Neproify]

@qaisjp It is probably due to that deallocation is done only in CResource destructor. If you don't destroy resource from memory then it won't deallocate. I used same place as other cleanups, but it probably don't run on restarts. However it looks like resources aren't destroyed when disconnecting(not sure about this, just a hypothesis). I will try to investigate it further when I got more time.

[Neproify]

@qaisjp It is because of resources that are destroyed only when CResourceManager::Refresh(or whatever it is) is called AND when resource is stopped. Stop and run when refresh wasn't called in between, and you got a problem. About disconnecting thing - I think MTA isn't destroying resources when you disconnect from server. That probably is thing for another issue.

[qaisjp]

I can't reproduce this issue when disconnecting, but I can during reconnects.

About disconnecting thing - I think MTA isn't destroying resources when you disconnect from server. That probably is thing for another issue.

CResource::~CResource() is not being called for reconnects, but it is during disconnects. Probably because the deathmatch module is being unloaded on disconnect.

[qaisjp]

CResource::~CResource() is not being called for reconnects.

I can't reproduce this anymore.

---

Quick and short way to reproduce:

-- in client resource
addCommandHandler("do", function()
    outputChatBox("Result: " .. tostring(engineRequestModel("ped")))
end)

addCommandHandler("undo", function(_, model)
    outputChatBox("Result: " .. tostring(engineFreeModel(model)))
end)

Then run the following commands:

do            # Observe 3
undo 3        # Observe true
do            # Observe 3
undo 3        # Observe true
do            # Observe 3
restart test  # Observe 4 (ACHTUNG: should be 3)
undo 3        # Observe true (ACHTUNG: should have been false)
undo 4        # Observe true

(Note that this returning the same number is an implementation detail, and not actually guaranteed by our Lua API.)

---

I've got a fix but it's not a proper fix.

https://github.com/multitheftauto/mtasa-blue/blob/475544f1753ce1af24c4cdff7f0d0be48ede709b/Client/mods/deathmatch/logic/CClientModelManager.cpp#L75-L87

Replace line 83 here with delete m_Models[i];, and it is fixed.

It's not a proper fix because the engineRestoreModel Lua function uses CClientModel::Deallocate, and it works fine. So why doesn't it work fine when CClientModelManager::DeallocateModelsAllocatedByResource is called?

---

I think the bug is inside engineRequestModel itself. Here in engineRequestModel we create a CClientModel:

https://github.com/multitheftauto/mtasa-blue/blob/475544f1753ce1af24c4cdff7f0d0be48ede709b/Client/mods/deathmatch/logic/luadefs/CLuaEngineDefs.cpp#L531-L538

Then pModel->Allocate (L534 above) then calls to CClientModelManager::Add (L21 below):

https://github.com/multitheftauto/mtasa-blue/blob/475544f1753ce1af24c4cdff7f0d0be48ede709b/Client/mods/deathmatch/logic/CClientModel.cpp#L21

And CClientModelManager::Add won't add because the model already exists:

https://github.com/multitheftauto/mtasa-blue/blob/475544f1753ce1af24c4cdff7f0d0be48ede709b/Client/mods/deathmatch/logic/CClientModelManager.cpp#L35-L41