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

#278: fix blip code inconsistencies and clamp size value for backwards compatibility #279

Merged
merged 3 commits into from Aug 2, 2018
Merged

#278: fix blip code inconsistencies and clamp size value for backwards compatibility #279

merged 3 commits into from Aug 2, 2018

Conversation

patrikjuvonen
Copy link
Contributor

@patrikjuvonen patrikjuvonen commented Jul 29, 2018

GitHub issue:
#278

Summary:

  • Discovered by zHooP on Discord. Thanks.
  • This bug has been in the client code forever.
  • Code never checked if blip icon is valid.
  • Code allowed any blip size.
  • Made backwards compatible by clamping size value between 0 and 25 instead of returning false upon invalid size.
  • If size is out of bounds client-side, it will output a debug warning about clamping that might affect behavior.

@patrikjuvonen patrikjuvonen added the bug Something isn't working label Jul 29, 2018
@patrikjuvonen patrikjuvonen added this to the 1.5.6 milestone Jul 29, 2018
@patrikjuvonen patrikjuvonen added this to In progress in release/v1.5.6 via automation Jul 29, 2018
@Citizen01
Copy link
Member

FYI: The AppVeyor build failed because it says the build took more than 60 minutes when the logs clearly shows it got stopped after ~12 minutes. Someone has to rerun the build.

@@ -60,14 +60,14 @@ int CLuaBlipDefs::CreateBlip(lua_State* luaVM)
{
CVector vecPosition;
unsigned char ucIcon = 0;
unsigned char ucSize = 2;
int iSize = 2;
Copy link
Contributor

Choose a reason for hiding this comment

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

has the size param changed from an unsigned char to int to satisfy Clamp?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That, but also to make sure if you passed a negative value as the argument, it would take that negative value and clamp it to 0 instead of some random value between the min and max depending on the integer overflow amount.

@qaisjp qaisjp merged commit 86a9e34 into multitheftauto:master Aug 2, 2018
release/v1.5.6 automation moved this from In progress to Done Aug 2, 2018
@patrikjuvonen patrikjuvonen deleted the github-issue-278 branch August 5, 2018 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants