We have an issue about this #1701.
Have you read it?

We have an issue about this #1701.
Have you read it?

#1701 is already merged and it works, but only for clientside peds, which in reality won't be of much use. I guess this PR is trying to solve that.

Sorry, its #1835

I dont feel like this is a good solution.
And since it breaks backwards compatibility, we might as well just implement the solution described by me in #1835

I dont feel like this is a good solution.

It's good simply solution

Please, test it with old client and new server

You can't simply change the value from a char to a short, this will cause packet misalignment if the server is running an older version.

This PR must break backwards compatibility, and since it does, we might as well just do it as described in #1835

When doing so, we should allow to set custom vehicle-name when requesting ID for it. #1849

If name not specified, take parent id name.

I see we have some minor checks like:
HasPoliceRadio
DoCheckHasLandingGear
HasTaxiLight
IsTrainModel
HasSirens
etc.
But do we have
-ZR-350 lights
-Packer
-Andromeda back door
(i dont think we have more 'misc' options)
?

@Pirulax check review pls

I don't think requesting a specific id is a good solution either as it will lead to collisions between resources.

Thanks for the PR very much. I'm sorry but I (we?) have to decline it.
We've discussed how it should be implemented, and came to the conclusion that using script entities instead of ID's is a better choice (to avoid people hardcoding stuff).
I really have a bias against using integer's as model ID's as it gets really messy, especially with dynamic IDs.
See #custom-model-ids channel on Dev discord.

Why you want avoid hardcoding stuff? Numeric IDs are easy, backward compatible, good for databases.
script entities instead of ID's sounds like a creepy abstraction layer.

And yeah this stuff is for manual management

From discord:
My implementation allows you to work with custom vehicles as with a standar vehicles, without need to adapt many current scripts for this (only scripts current use engineRequestModel so that there are no id conflicts). And this is important.

Obviously, the problem of collisions should be solved by developer who will add new vehicles. You are now proposing to complicate something that does not require complication. Adding non-static identification don't solve any problem, it only complicates.

The problem I see with using numeric IDs as-is is that they lure people into thinking they are the simplest solution that keeps backwards compatibility, but on practical terms, when actually requesting custom model IDs in the client and the server, complexity scalates quickly, because 1) each server has to reinvent the wheel to manage this problem, 2) depend on a community resource that does this for them or 3) be ignorant of this problem and wonder why things break.

One may think that, if model IDs could only be requested on the server, then these discrepancies would be solved, but requesting a model for every client just because a single client wants to show a vehicle preview in a car shop, for example, wastes a fair amount of bandwidth. Besides, we would break backwards compatibility anyway, because model requests are clientside right now.

It also makes sense to promote element models to their own entity, as with these changes they would have associated logic and a lifetime that's not necessarily the same as the lifetime of the elements that use them, to name some reasons. Insisting on representing entities with primitive data types is known as the primitive obsession antipattern and usually leads to suboptimal solutions in the long (or even short) run. Also, MTA is reasonably expected to keep serverside things synchronized in a transparent and relatively effortless manner, and this approach breaks this expectation.

For reference and clarity purposes, I made this UML class diagram showing an hypothetical OOP Lua API that I think it'd be nice to implement (with some ideas from @sbx320 and @Pirulax):

@startuml "Custom model IDs API"
title Custom model IDs API
class Element {
	+setModel(ElementModel model) : void
	+getModel() : ElementModel
	-- Rest of methods --
	...
}
class ElementModel {
	{static} +fromGameModelId(int id) : Model
	{static} +getAllModels() : Model [1..*]
	{static} +request(ElementModelType type, ElementModel parent) : CustomModel
	-int internalId
}
note left: This API is shared.\n\ninternalId is only known by MTA C++ code.\n\nWhen a client requests a new model,\nits internalId is the next free one for\nuse with GTA.\n\nWhen a server requests a new model,\nits internalId may be a sequential number,\nand clients should request a new model\n(maybe with a different internalId in each\nclient). If a client runs out of internalIds\nduring this operation, it may 1) be kicked\nor 2) the request may return false\n(preferred).\n\nClients keep a server internal ID -> actual\ninternal ID mapping, so they know how to\ntranslate server IDs in packets and so on\nappropriately to keep sync.\n\nServer databases may store models as a\n(type, ID) pair, where "type" distinguishes\nbetween GTA and custom models, and ID\ncan be an index for a Lua table entry that\ncontains data about that model, like a\nfactory method that calls\nElementModel.request to initialize the\nmodel for the first time.\n\nWhen a ElementModel element gets\ndestroyed, the elements that use it are\nreset to the parent element model or\nanother suitable default. This can happen\nwhen the resource that requested them\nstops or destroyElement() is used.
ElementModel "1" --> "0..*" ElementModel : children
Element <|.. ElementModel
enum ElementModelType {
	PED
	VEHICLE
	OBJECT
}
ElementModel ..> ElementModelType
@enduml

If we agree that this is a good API, then we can implement these features with less risk of them being rejected or problematic in the long term. Of course it'd depend on things like #1591, but I think it's a good way to go.