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

Wrong vehicle selling on chopshop FIX #568

Merged
merged 8 commits into from Aug 4, 2019

Conversation

Casperento
Copy link
Contributor

Resolves #567

Changes proposed in this pull request:

  • Get vehicle object by it's netId

  • fn_chopShopMenu.sqf updated

  • fn_chopShopSell.sqf updated

  • I have tested my changes and corrected any errors found

@Casperento Casperento closed this Jul 12, 2019
@Casperento Casperento reopened this Jul 12, 2019
klmunday
klmunday previously approved these changes Jul 17, 2019
Copy link
Contributor

@klmunday klmunday left a comment

Choose a reason for hiding this comment

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

seems good

DomT602
DomT602 previously approved these changes Jul 21, 2019
Copy link
Member

@DomT602 DomT602 left a comment

Choose a reason for hiding this comment

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

@Casperento happy with the changes, although while we're here, is it worth optimising the rest of the files you've changed?

@Casperento
Copy link
Contributor Author

Yes, I think so

@Casperento Casperento dismissed stale reviews from DomT602 and klmunday via 8bc0232 July 22, 2019 16:57
Copy link
Member

@BoGuu BoGuu left a comment

Choose a reason for hiding this comment

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

@@ -2,25 +2,33 @@
/*
File: fn_chopShopMenu.sqf
Author: Bryan "Tonic" Boardwine
Modified: Casperento
Copy link
Member

Choose a reason for hiding this comment

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

See the comment on the conversation here: https://github.com/AsYetUntitled/Framework/pull/564/files/a33681c53b384b2aaa97286df87bbae6fc5f2491#diff-480bd50ccc3ed591fc3ed0bd4b402585

I've made the same point a few times in recent PRs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No problem, i'll remove these 'Modified' comments


private _control = CONTROL(39400,39402);
private _price = _control lbValue (lbCurSel _control);
private _vehicle = _control lbData (lbCurSel _control);
_vehicle = call compile format ["%1", _vehicle];
Copy link
Member

Choose a reason for hiding this comment

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

Would probably be best to remove this

Copy link
Contributor

@Leigham Leigham Jul 22, 2019

Choose a reason for hiding this comment

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

i would personally use setVariable, instead of lbDate when using anything but a string.
_control setVariable [format ["lbData_%1", _index], anything];

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thx for suggesting

if (isNull _vehicle) exitWith {};

hint localize "STR_Shop_ChopShopSelling";
life_action_inUse = true;
_price2 = CASH + _price;
[0] call SOCK_fnc_updatePartial;
Copy link
Member

Choose a reason for hiding this comment

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

Not required anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope

Copy link
Member

Choose a reason for hiding this comment

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

(talking about the updatePartial so we're on the same page - can remove that)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, sorry my bad


if (_price > 0) then {
CASH = CASH + _price;
[1,"STR_NOTF_ChopSoldCar",true,[_displayName,[_price] call life_fnc_numberText]] call life_fnc_broadcast;
Copy link
Member

Choose a reason for hiding this comment

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

Maybe more elegant to just do this manually?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, sure

life_hc/MySQL/Vehicles/fn_chopShopSell.sqf Show resolved Hide resolved
life_hc/MySQL/Vehicles/fn_chopShopSell.sqf Show resolved Hide resolved
@Casperento
Copy link
Contributor Author

Yeah, that's true @Leigham...already fixed for the next commit

@Leigham
Copy link
Contributor

Leigham commented Jul 23, 2019

it just removes the need for call compile _something

@Casperento
Copy link
Contributor Author

Ya, i came up with this in the end:
private _vehicle = objectFromNetId (_control getVariable[format["lbData_%1",(lbCurSel _control)],""]);

@Leigham
Copy link
Contributor

Leigham commented Jul 23, 2019

just pass through the object

@Casperento
Copy link
Contributor Author

Casperento commented Jul 23, 2019

Yes, well reminded..but i think it's better keep lbData anyway

Copy link
Contributor

@klmunday klmunday left a comment

Choose a reason for hiding this comment

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

Looks good, just need to make sure it passes the tests, seems CI picked up a few tabs see: here

klmunday
klmunday previously approved these changes Aug 3, 2019
@Leigham
Copy link
Contributor

Leigham commented Aug 3, 2019

just looked through, _control lbSetData [(lbSize _control)-1,(netId _x)]; might want to double check this, lbSetData only allows strings.

@DomT602
Copy link
Member

DomT602 commented Aug 3, 2019

just looked through, _control lbSetData [(lbSize _control)-1,(netId _x)]; might want to double check this, lbSetData only allows strings.

netID returns a string so that should be fine

Copy link
Member

@DomT602 DomT602 left a comment

Choose a reason for hiding this comment

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

A few changes but overall good work!

*/
params [
["_price",-1,[-1]],
"_displayName"
Copy link
Member

Choose a reason for hiding this comment

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

displayName can be given a default of "" and allowed data types of "" also

@@ -6,14 +6,16 @@
Displays the pricing for the currently selected vehicle.
*/
disableSerialization;
Copy link
Member

Choose a reason for hiding this comment

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

Serialization doesn't need to be disabled here

_control = CONTROL(39400,39402);
private _control = CONTROL(39400,39402);
private "_className";
private "_classNameLife";
Copy link
Member

Choose a reason for hiding this comment

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

If we invert usage of _classname,
if (!isClass (missionConfigFile >> "LifeCfgVehicles" >> _classNameLife)) then {
diag_log format ["%1: LifeCfgVehicles class doesn't exist",_className];
_className = "Default"; //Use Default class if it doesn't exist
};

Copy link
Member

@DomT602 DomT602 left a comment

Choose a reason for hiding this comment

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

Thanks for the updates @Casperento. Just the last 2 things from me!


private _chopable = LIFE_SETTINGS(getArray,"chopShop_vehicles");
private _nearVehicles = nearestObjects [getMarkerPos (_this select 3),_chopable,25];
private _nearUnits = (nearestObjects[player,["CAManBase"],5]) arrayIntersect playableUnits;
if (count _nearUnits > 1) exitWith {hint localize "STR_NOTF_PlayerNear"};

life_chopShop = _this select 3;
//Error check
if (count _nearVehicles isEqualTo 0) exitWith {titleText[localize "STR_Shop_NoVehNear","PLAIN"];};
Copy link
Member

Choose a reason for hiding this comment

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

Slight improvement for this;
if (_nearVehicles isEqualTo []) exitWith {blah};

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok

Copy link
Member

@DomT602 DomT602 left a comment

Choose a reason for hiding this comment

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

Looks good! Thanks for the PR.

@DomT602 DomT602 merged commit 11408d6 into AsYetUntitled:master Aug 4, 2019
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.

Wrong vehicle selling on chopshop
5 participants