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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Function: isVehicleWheelCollided #146

Merged
merged 9 commits into from Feb 16, 2018

Conversation

Projects
8 participants
@ZReC
Contributor

ZReC commented May 29, 2017

This function detect if car wheel is on air or in collision.

Example:
isVehicleWheelCollided ( theVehicle, "rear_left" )
or
isVehicleWheelCollided ( theVehicle, 1 )
or
theVehicle.isWheelCollided ( "front_right" )

The wheels are:

"front_left" or 0
"rear_left" or 1
"front_right" or 2
"rear_right" or 3

Bikes only have front / rear left wheel .

If someone has a better name for the function please comment 馃憤

ZReC added some commits Mar 26, 2017

Function isVehicleWheelCollided
This function detect if car wheel is on air or in collision.
Example:
isVehicleWheelCollided ( theVehicle, "rear_left" )
or
isVehicleWheelCollided ( theVehicle, 1 )

The wheels are:
"front_left" or 0
"rear_left" or 1
"front_right" or 2
"rear_right" or 3
Bikes only have front / rear left wheel .

If someone has a better name for the function please comment 馃憤

@ZReC ZReC changed the title from Feature/is vehicle wheel collided to Function: isVehicleWheelCollided May 29, 2017

@CrosRoad95

This comment has been minimized.

Show comment
Hide comment
@CrosRoad95

CrosRoad95 May 29, 2017

Collaborator

Next can you add function getVehicleWheelContactMaterial(vehicle,wheel) it can be used for detect if player drive over grass

Collaborator

CrosRoad95 commented May 29, 2017

Next can you add function getVehicleWheelContactMaterial(vehicle,wheel) it can be used for detect if player drive over grass

Little change.
By Necktrox
@qaisjp

This comment has been minimized.

Show comment
Hide comment
@qaisjp

qaisjp May 30, 2017

Member

I suggest the function is called doesVehicleWheelCollide or getVehicleWheelCollide.

Also, perhaps we should instead only support text arguments or number arguments. I lean towards only supporting text arguments. The simpler and easier to understand, the better. This is up for discussion.

Member

qaisjp commented May 30, 2017

I suggest the function is called doesVehicleWheelCollide or getVehicleWheelCollide.

Also, perhaps we should instead only support text arguments or number arguments. I lean towards only supporting text arguments. The simpler and easier to understand, the better. This is up for discussion.

ZReC added some commits May 30, 2017

Rename enum
eWheels to eWheelPosition
@Jusonex

This comment has been minimized.

Show comment
Hide comment
@Jusonex

Jusonex Jul 29, 2017

Member

Could you please refactor CVehicleSA::IsWheelCollided to use the appropriate member attributes of CVehicleSAInterface instead of manual offset calculations.

E.g.:

bool CVehicleSA::IsWheelCollided(std::uint8_t wheelPosition) // eWheelPosition might also be better here
{
  CVehicleSAInterface* vehicle = GetInterface();
  switch (vehicle->wheelType) // or whatever this is exactly
  {
  case 0:
    if (wheelPosition < 4)
      return vehicle->wheelCollisionState[wheelPosition] == 4.0f;
  // ...
}
Member

Jusonex commented Jul 29, 2017

Could you please refactor CVehicleSA::IsWheelCollided to use the appropriate member attributes of CVehicleSAInterface instead of manual offset calculations.

E.g.:

bool CVehicleSA::IsWheelCollided(std::uint8_t wheelPosition) // eWheelPosition might also be better here
{
  CVehicleSAInterface* vehicle = GetInterface();
  switch (vehicle->wheelType) // or whatever this is exactly
  {
  case 0:
    if (wheelPosition < 4)
      return vehicle->wheelCollisionState[wheelPosition] == 4.0f;
  // ...
}
@ZReC

This comment has been minimized.

Show comment
Hide comment
@ZReC

ZReC Jul 29, 2017

Contributor

Not a bad idea, I'll do my best.

Contributor

ZReC commented Jul 29, 2017

Not a bad idea, I'll do my best.

@CrosRoad95

This comment has been minimized.

Show comment
Hide comment
@CrosRoad95

CrosRoad95 Jul 30, 2017

Collaborator

what if vehicle dont have 4 wheel? ex 3 ( dodo ) or motorbike ?

Collaborator

CrosRoad95 commented Jul 30, 2017

what if vehicle dont have 4 wheel? ex 3 ( dodo ) or motorbike ?

@ZReC

This comment has been minimized.

Show comment
Hide comment
@ZReC

ZReC Jul 30, 2017

Contributor

In vehicles of 3 wheels are combined 2 in 1, in motorbikes only the left

Contributor

ZReC commented Jul 30, 2017

In vehicles of 3 wheels are combined 2 in 1, in motorbikes only the left

@ZReC

This comment has been minimized.

Show comment
Hide comment
@ZReC

ZReC Jul 31, 2017

Contributor

Jusonex, I have problems implementing the code to CVehicleSAInterface, it turns out that from offset 0x5A0 (1440 decimal) CVehicleSAInterface should give rise to a class more specific of the type of vehicle in particular (CAutomobile, CBike, CHeli, CTrain, CPlane) But instead the class extends with variables of different types of vehicles as of trains:

//1440
unsigned char m_ucTrackNodeID;  // Current node on train tracks

In my opinion would have to be rewritten this sector of the code, to make it more structured and manageable, meanwhile could graft the code of the collision but would not be the most appropriate.
What do you think?

Note: the offset 0x590 if it is the vehicle type, type uint8_t and its values correspond to:
0 = car - plane | 5 = boat | 6 = train | 9 = bike

Contributor

ZReC commented Jul 31, 2017

Jusonex, I have problems implementing the code to CVehicleSAInterface, it turns out that from offset 0x5A0 (1440 decimal) CVehicleSAInterface should give rise to a class more specific of the type of vehicle in particular (CAutomobile, CBike, CHeli, CTrain, CPlane) But instead the class extends with variables of different types of vehicles as of trains:

//1440
unsigned char m_ucTrackNodeID;  // Current node on train tracks

In my opinion would have to be rewritten this sector of the code, to make it more structured and manageable, meanwhile could graft the code of the collision but would not be the most appropriate.
What do you think?

Note: the offset 0x590 if it is the vehicle type, type uint8_t and its values correspond to:
0 = car - plane | 5 = boat | 6 = train | 9 = bike

@Jusonex

This comment has been minimized.

Show comment
Hide comment
@Jusonex

Jusonex Jul 31, 2017

Member

In my opinion would have to be rewritten this sector of the code, to make it more structured and manageable

Yes, that's the way to go - not as a part of this PR though.

I'd say update as much as possible (e.g. 0x590) and leave the rest as it is.

Member

Jusonex commented Jul 31, 2017

In my opinion would have to be rewritten this sector of the code, to make it more structured and manageable

Yes, that's the way to go - not as a part of this PR though.

I'd say update as much as possible (e.g. 0x590) and leave the rest as it is.

Refactored code section
CVehicleSAInterface size it was not according to offset of
m_fBurningTime
@ZReC

This comment has been minimized.

Show comment
Hide comment
@ZReC

ZReC Oct 6, 2017

Contributor

Updated. Excuse me for the lack of activity, studies are killing me 馃槅

Contributor

ZReC commented Oct 6, 2017

Updated. Excuse me for the lack of activity, studies are killing me 馃槅

@Necktrox Necktrox merged commit 192445b into multitheftauto:master Feb 16, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@patrikjuvonen patrikjuvonen added this to In progress in release/v1.5.6 via automation Aug 7, 2018

@patrikjuvonen patrikjuvonen added this to the 1.5.6 milestone 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