MafiaHub / MafiaMP

Multiplayer experience for Mafia: Definitive Edition
https://mafiahub.dev
Other
39 stars 5 forks source link

Move PlayerVehicleEnter/Leave events to Player #98

Open Deewarz opened 6 months ago

Deewarz commented 6 months ago

Makes more sense to attach the events to the Player: he is the one who causes the events.

To be more precise we should attach it to Human and transfer it to Player but we don't yet have this level of layers.


This is also the case in other implementations (who created a usage).

Some examples:

Segfaultd commented 6 months ago

Please put a description to the PR.

Overall comment, why renaming the events?

Deewarz commented 6 months ago

@Segfaultd PR description updated.

Segfaultd commented 6 months ago

Thanks. Looks way better now. Open to discussion tho, what about naming something like playerEnterVehicle and playerExitVehicle. It makes more sense when it comes to litteral reading of the event

sdk.on('playerEnterVehicle', cb);

Deewarz commented 6 months ago

@Segfaultd I don't really know, for me it makes more sense to put "vehicle" as a prefix to the action.

It creates a sort of subcontext.

This is the same approach I used for the setColorPrimary and setColorSecondary methods on Vehicle.

As a developer, our model of thinking is to seek context then action. You think about putting the color, and more precisely the primary. You think about the vehicle, and more precisely enter.

Also, it allows you to group them at a single glance when you use or document them.

I do not know if I'm clear, I don't really know how to explain it