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

Fix new vehicle upgrades #3261

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

Serius41
Copy link

@Serius41 Serius41 commented Dec 5, 2023

This PR resolves issue #2198 by fixing vehicle upgrades on the client for added vehicles (engineRequestModel).

Video:
https://streamable.com/zngjok

Comment on lines 66 to 67
unsigned short usModel = m_pVehicle->GetModel();

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
unsigned short usModel = m_pVehicle->GetModel();
unsigned short usModel = m_pVehicle->GetModel();
usModel = g_pGame->GetModelInfo(usModel)->GetParentID() || usModel;

This do same thing, but less code.

Copy link
Member

@tederis tederis Dec 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This do same thing, but less code.

Your expression will produce 1 all the time.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (auto* pModelInfo = g_pGame->GetModelInfo(usModel); pModelInfo && pModelInfo->GetParentID() != 0)
        usModel = static_cast<uint16_t>(pModelInfo->GetParentID());

Would be better I guess.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lol. I forgot about this in c++. Ty @tederis

@@ -1435,7 +1435,7 @@ void CVehicleModelInterface_SetClump()
{
// Loop through all vehicles and find the vehicle id that this interface belongs to
CModelInfo* pModelInfo = NULL;
for (int i = 400; i < 612; i++)
for (int i = 400; i < 80000000; i++)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you need that?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It adds more than one parts. Here you specified only the GTA vehicle ids for (int i = 400; i < 612; i++), but it was not checking when there were more ids, so I increased the number. This is a temporary solution. I could not connect the deatmach file to the multiplayer:sa file to capture the parent id. I did something like this for him and it worked.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is extremely inefficient and can lead to freezes when spawning vehicles.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How else can I prevent it? I already thought of this I put it as a temporary solution and it worked

Copy link
Member

@tederis tederis Dec 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I can see, there is two possible solutions. The first one is to use one of the associative containers to link model ptr to a model ID at the game's start. And the second one is to store model ID in CBaseModelInfo class(but it will require reverse engineering skills). Maybe there is something else, it requires additional study.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tederis
I updated it again, can you check it? I tested it and it worksworks

// We add both left and right for some reason seems to be SA doing it
bReturn &= supportedUpgrades.m_bBonnet_Left;
bReturn &= supportedUpgrades.m_bBonnet_Left_dam;
int modelRequestIDParent = g_pGame->GetModelInfo(usModel)->GetParentID();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused variable


// No nitro or other upgrades for planes
if (vehicleType == CLIENTVEHICLE_PLANE)
return false;

if (us == VEHICLEUPGRADE_NITRO_5X || us == VEHICLEUPGRADE_NITRO_2X || us == VEHICLEUPGRADE_NITRO_10X || us == VEHICLEUPGRADE_HYDRAULICS)
return true;

bool bReturn = false;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused variable

Comment on lines +300 to +301
if (auto* pModelInfo = g_pGame->GetModelInfo(usModel); pModelInfo && pModelInfo->GetParentID() != 0)
usModel = static_cast<uint16_t>(pModelInfo->GetParentID());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indentation problem

size_t vehicleIndex = (usModel - 400);
size_t upgradeIndex = (usUpgrade - 1000);

const VehicleUpgradeSlots& slots = vehicleUpgradeSlots[vehicleIndex];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is unsafe since potentially usModel(and usUpgrade) can be extented in future and you will get out of the range access problem. Maybe you should use at method instead of [] to properly handle with the help of exceptions? Or at least add assert for the early diagnosis.


const VehicleUpgradeSlots& slots = vehicleUpgradeSlots[vehicleIndex];
size_t slotIndex = upgradeIndex / 32;
uint32_t slot = slots[slotIndex];
Copy link
Member

@tederis tederis Dec 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same (line 308)

@StrixG StrixG added the bugfix Solution to a bug of any kind label Dec 21, 2023
@TheNormalnij TheNormalnij added the feedback Further information is requested label Oct 1, 2024
Copy link
Contributor

@Fernando-A-Rocha Fernando-A-Rocha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tederis 's reviews make sense

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix Solution to a bug of any kind feedback Further information is requested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants