Skip to content

Conversation

@Nico8340
Copy link
Member

Fixes vehicle upgrades

@tederis
Copy link
Member

tederis commented Mar 27, 2025

Could you elaborate on what exactly it fixes?

@Nico8340
Copy link
Member Author

Could you elaborate on what exactly it fixes?

Everything, because the previous pull request broke it all

bool CVehicleUpgrades::IsUpgradeCompatible(const std::uint16_t upgrade) noexcept
{
eClientVehicleType type = m_pVehicle->GetVehicleType();
const auto type = m_pVehicle->GetVehicleType();
Copy link
Member

Choose a reason for hiding this comment

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

It's better to avoid replacing small types with auto.

return false;
}

std::uint16_t model = 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.

It's better to avoid replacing small types with auto.


const auto& set = it->second;

return set.find(upgrade) != set.end();
Copy link
Member

Choose a reason for hiding this comment

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

What do you think about it->second.count(upgrade) > 0;

@tederis
Copy link
Member

tederis commented Mar 27, 2025

Why do we need m_slots for every spawned vehicle? Couldn't it be global?

@FileEX
Copy link
Member

FileEX commented Mar 28, 2025

Could you elaborate on what exactly it fixes?

Everything, because the previous pull request broke it all

So you opened a PR without even testing its changes or checking if it works at all? 👀

@tederis
Copy link
Member

tederis commented Mar 28, 2025

Could you elaborate on what exactly it fixes?

Everything, because the previous pull request broke it all

So you opened a PR without even testing its changes or checking if it works at all? 👀

Why do I need that? Before any testing I want to understand the logic of PR.

@Nico8340
Copy link
Member Author

So you opened a PR without even testing its changes or checking if it works at all? 👀

Mistake on my end, sorry. I did it as a favour, and I was in a rush, I just implemented the same logic with the old reviews.

@Nico8340
Copy link
Member Author

Why do we need m_slots for every spawned vehicle? Couldn't it be global?

Thx for the quick review, I'll solve the issues soon.

@Dutchman101
Copy link
Member

So you opened a PR without even testing its changes or checking if it works at all? 👀

He adopted the original PR (#3261) just to comply with the last pending code reviews from TEDERIs it had. It should've been matured at that point. Btw, even after #4114 (by nightly testing) i can't really find issues, but no doubt that Nico found them due to this here.

It's not a big deal, no recent nightly was released to users.

Dutchman101 added a commit that referenced this pull request Mar 29, 2025
…" for until it's stable (to be combined with #4128 when re-introduced)

This reverts commit ed3bb71.
MTABot pushed a commit that referenced this pull request Mar 29, 2025
79dea1c Temporarily evert "Fix vehicle upgrades for assigned vehicles (#4114)" for until it's stable (to be combined with #4128 when re-introduced)
@Dutchman101
Copy link
Member

#4114 is now reverted, so that we can get stable master again

@Nico8340 please apply this fix on top of reintroducing what was reverted

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants