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

Client-sided setVehicleHandling for local entities (aka client-sided vehicles) #192

Merged
merged 2 commits into from
Jun 8, 2018

Conversation

Renkon
Copy link
Contributor

@Renkon Renkon commented May 7, 2018

The following includes some basic tests of the features implemented.

Basically, implemented setVehicleHandling client-side allowing it only to work with local entities so as to avoid having to sync the changes (if you need to sync them, you can do it serverside, since it means you your vehicle is serverside).

When resetting vehicleHandling, it can only be resetted to original value. (there's no setModelHandling client-side, which means that you should use the following syntax).

setVehicleHandling(veh, nil) -- resets all to gta default
setVehicleHandling(veh, property, nil) -- resets property to nil

Tested programatically as it will be shown below (can't get inside client-sided vehicles to test-drive)

9610f082-6cf5-49d5-a99b-b9dbd8d8a7ec

Please code review and tell me if there's something I can do to improve.

Thank you,

-ffs-Renkon

@qaisjp
Copy link
Contributor

qaisjp commented May 7, 2018

It looks like some of this code was copied from the server. Have you explored ways to share code?

@Renkon
Copy link
Contributor Author

Renkon commented May 7, 2018

Sincerely no.

I have tried to have a HandlingDefs function like server-side (so that it would serversided guidelines), and I got to a dead end (probably due to my lack of knowledge).

Also, the signature of the handling methods use a CClientVehicle instead of a CVehicle checking if it's a localEntity. I'm not sure if they implement any interface, in which case we could think about sharing the code.

Finally, resetting got the boolean parameter removed, although the code is still very similar.

What do you suggest I could do to improve it?

Should I try to implement it shared?

@qaisjp
Copy link
Contributor

qaisjp commented May 12, 2018

Should I try to implement it shared?

Yes please! Not everything has to implemented shared but the more, the better.

Also, the signature of the handling methods use a CClientVehicle instead of a CVehicle checking if it's a localEntity. I'm not sure if they implement any interface, in which case we could think about sharing the code.

I think the trick here is to wrap the initial declaration in an #ifdef, something like this (note, this is kinda pseudocode):

#ifdef MTA_CLIENT
CClientVehicle* pVeh = getVehicle();
#else
CVehicle* pVeh = getVehicle();
#endif

// Now you can use common methods
pVeh->something();

It's probable that the only suitable thing to implement shared is CStaticFunctionDefinitions::SetEntryHandling, and maybe CLuaVehicleDefs::SetVehicleHandling(lua_State* luaVM). The static bool ResetVehicleHandling(CClientVehicle* pVehicle); methods might be a bit messy, in which case you can just omit this.

Finally, resetting got the boolean parameter removed, although the code is still very similar.

I think a good way forward would to actually keep the boolean feature of setVehicleHandling, except just disable false (model handling).

That way both functions (server and client side) function exactly the same, except on the client the model handling would throw an error. (This would make it easier to document and easier to remember.)

Thank you for your contribution! 👍 💖

@Renkon
Copy link
Contributor Author

Renkon commented May 23, 2018

Due to exams I won't be able to work on it till late June/early July.

Anybody available to work on it is welcome. Feel free to work on my fork (if you need to get collaborator status send me a message).

@qaisjp
Copy link
Contributor

qaisjp commented May 23, 2018

@Renkon I might have a dabble if you can add me as a contributor

@qaisjp qaisjp self-requested a review June 8, 2018 11:49
@ccw808 ccw808 merged commit 599d85e into multitheftauto:master Jun 8, 2018
@patrikjuvonen patrikjuvonen added the enhancement New feature or request label Aug 7, 2018
@patrikjuvonen patrikjuvonen added this to the 1.5.6 milestone Aug 7, 2018
@patrikjuvonen patrikjuvonen added this to In progress in release/v1.5.6 via automation Aug 7, 2018
@patrikjuvonen patrikjuvonen moved this from In progress to Done in release/v1.5.6 Aug 7, 2018
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
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants