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

Cargo - Support of object without `ace_cargo_size` defined in config or `-1` config value #6364

Merged
merged 7 commits into from Jun 1, 2018

Conversation

Projects
None yet
4 participants
@Vdauphin
Contributor

Vdauphin commented May 23, 2018

close #6357

Actually, if _typeofcargo does not have ace_cargo_size define in config, the _radiusOfItem of the object is 1. This inconsistency can be avoid by using the ace_cargo_fnc_getSizeItem which take an object as parameter and get the size of the object defined manually by user ( ace_cargo_fnc_setsize).

By having a _radiusOfItem more accurate, the ace_common_fnc_findUnloadPosition will find better position for unloading and avoid vehicle explosion when big objects, without ace_cargo_size defined in config, are unloaded.

I don't know if it is the best way to handle this case but at least it is a work around. Cheers.

When merged this pull request will:

  • ace_common_fnc_findUnloadPosition can take object as parameter
  • Prefer the config way instead to determine the size of _cargo directly with ace_cargo_fnc_getSizeItem (this avoid inconsistency when player ace_cargo_fnc_setsize of an object lower from the ace_cargo_size defined in config)
  • Also check if the config value is not -1 to avoid the issue: #6357 (comment)
  • If ace_cargo_size is not define in config, use the ace_cargo_fnc_getSizeItem to get object size set by player with ace_cargo_fnc_setsize
  • Update the ace_cargo_fnc_unloadItem accordingly (use the cargo object instead of the class name).

Vdauphin added some commits May 23, 2018

Add: Now findUnloadPosition support also cargo object
- use of ace_cargo_fnc_getSizeItem to determine size of item

- prefer the config way instead of manually set size

- This will also take into account object manually added to the cargo system with ace_cargo_fnc_setsize

- use the cargo object for ace_cargo_fnc_unloadItem.
private _itemSize = if (isNumber (configFile >> "CfgVehicles" >> _typeOfCargo >> QEGVAR(cargo,size))) then {
getNumber (configFile >> "CfgVehicles" >> _typeOfCargo >> QEGVAR(cargo,size));
} else {
[_cargo] call EFUNC(cargo,getSizeItem);

This comment has been minimized.

@Dystopian

Dystopian May 23, 2018

Contributor

there is error here if cargo module is no loaded

This comment has been minimized.

@Vdauphin

Vdauphin May 23, 2018

Contributor

[[_cargo] call EFUNC(cargo,getSizeItem)] param [0, _radiusOfItem]; could resolve that issue.

This comment has been minimized.

@Dystopian

Dystopian May 24, 2018

Contributor

I meant if cargo module is not loaded there is no EFUNC(cargo,getSizeItem) and you call non-existing function

This comment has been minimized.

@Vdauphin

Vdauphin May 24, 2018

Contributor

I just learned in scheduled EFUNC(cargo,getSizeItem) will trow an undefined variables error which is not the case in unscheduled.
If you guys are using an other approach, let me know, I will use it for consistency.

This comment has been minimized.

@jonpas

jonpas May 24, 2018

Member

if (["ace_backpacks"] call EFUNC(common,isModLoaded)) then {

This comment has been minimized.

@Vdauphin

Vdauphin May 24, 2018

Contributor

Thanks!

@PabstMirror

This comment has been minimized.

Collaborator

PabstMirror commented May 23, 2018

For safety we could take the max of both numbers

Vdauphin added some commits May 24, 2018

As suggested by @orbis2358
This fix handle the case when cargo module is not loaded for scheduled and unscheduled environnement.
if (isNumber (configFile >> "CfgVehicles" >> _typeOfCargo >> QEGVAR(cargo,size))) then {
_radiusOfItem = (((getNumber (configFile >> "CfgVehicles" >> _typeOfCargo >> QEGVAR(cargo,size))) ^ 0.35) max 0.75);
private _typeOfCargo = if (_cargo isEqualType "") then {_cargo} else {typeOf _cargo};
private _itemSize = if (isNumber (configFile >> "CfgVehicles" >> _typeOfCargo >> QEGVAR(cargo,size))) then {

This comment has been minimized.

@Dystopian

Dystopian May 25, 2018

Contributor

this code piece already exists in EFUNC(cargo,getSizeItem), so can be optimized well

This comment has been minimized.

@Vdauphin

Vdauphin May 25, 2018

Contributor

I did a choice here as explain in the PR description:

Prefer the config way instead to determine the size of _cargo directly with ace_cargo_fnc_getSizeItem (this avoid inconsistency when player ace_cargo_fnc_setsize of an object lower from the ace_cargo_size defined in config)

I can add a parameter in EFUNC(cargo,getSizeItem) to prefer the config if exist or prefer varaible if exist

This comment has been minimized.

@Vdauphin

Vdauphin May 25, 2018

Contributor

I am supposing that ace_cargo_size in config is correct so if I am calling EFUNC(cargo,getSizeItem) and if player has ace_cargo_fnc_setsize an other value (a smaller one for exemple) the function will return a bad position base on a wrong value changed by player. This avoid that case and also cover the objective of this PR where ace_cargo_size in config is not defined.

private _itemSize = if (isNumber (configFile >> "CfgVehicles" >> _typeOfCargo >> QEGVAR(cargo,size))) then {
getNumber (configFile >> "CfgVehicles" >> _typeOfCargo >> QEGVAR(cargo,size));
} else {
if (["ace_cargo"] call EFUNC(common,isModLoaded)) then {

This comment has been minimized.

@Dystopian

Dystopian May 25, 2018

Contributor

EFUNC(common,isModLoaded) -> FUNC(isModLoaded)

This comment has been minimized.

@Vdauphin

Vdauphin May 25, 2018

Contributor

EFUNC(common,isModLoaded) -> ace_common_fnc_isModload
FUNC(isModLoaded) -> ace_cargo_fnc_isModload

Because the second doesn't exist I am using the first one as suggested by @jonpas here: #6364 (comment)

This comment has been minimized.

@Dystopian

Dystopian May 25, 2018

Contributor

here we are in common module (addons/common/functions/fnc_findUnloadPosition) so FUNC is ace_common_fnc

This comment has been minimized.

@Vdauphin

Vdauphin May 25, 2018

Contributor

woups!!!

_radiusOfItem;
};
};
if !(_itemSize isEqualTo -1) then {

This comment has been minimized.

@Dystopian

Dystopian May 25, 2018

Contributor

just _itemSize != -1

This comment has been minimized.

@Vdauphin

Vdauphin May 25, 2018

Contributor

With isEqualTo you can compare different type like: nil isEqualTo -1 so the function will never fail

It doesn't throw error when comparing different types, i.e. ("eleven" isEqualTo 11)
But, if ace use it as convention I will change it.

This comment has been minimized.

@Dystopian

Dystopian May 25, 2018

Contributor

_itemSize here is always number

};
};
if !(_itemSize isEqualTo -1) then {
_radiusOfItem = (_itemSize ^ 0.35) max 0.75;

This comment has been minimized.

@Dystopian

Dystopian May 25, 2018

Contributor

most significant change: before this PR if cargo module is not loaded _radiusOfItem was 1 and now it becomes (1 ^ 0.35) max 0.75.

This comment has been minimized.

@Vdauphin

Vdauphin May 25, 2018

Contributor

Ahah! I didn't know we could make joke in review!
Thanks a lot for the time you take to review this :-), I appreciate your feedback!

This comment has been minimized.

@Vdauphin

Vdauphin May 25, 2018

Contributor

(1 ^ 0.35) max 0.75 = 1 max 0.75 = 1

This comment has been minimized.

@Vdauphin

Vdauphin May 25, 2018

Contributor

Sorry I thought you were ironical :/

FIX: EFUNC and isEqualto
- _itemSize here is always number
- here we are in common module (addons/common/functions/fnc_findUnloadPosition) so FUNC is ace_common_fnc

@Vdauphin Vdauphin referenced this pull request May 25, 2018

Merged

Add: ACE cargo #540

10 of 10 tasks complete
@Vdauphin

This comment has been minimized.

Contributor

Vdauphin commented May 26, 2018

For safety we could take the max of both numbers

@PabstMirror
I presume the config value is the perfect value for the ace_common_fnc_findUnloadPosition.
If user change to a higher, the object could be no more unloadable because the ace_common_fnc_findUnloadPosition will return an [].

The value give by user should only be use if no config value is available.

Vdauphin added a commit to Vdauphin/HeartsAndMinds that referenced this pull request May 30, 2018

FIX: objects unloading without ace_cargo_size config value
- this is correcting the unloading position with the ace_cargo_size define as variable
- this can be removed when: acemod/ACE3#6364  is merged
FIX case where config value is `-1`
- if the config value is -1, get the `ace_cargo_size` from the `ace_cargo_fnc_getSizeItem
-
 #6357 (comment)

@Vdauphin Vdauphin changed the title from Cargo - Support of object without `ace_cargo_size` defined in config to Cargo - Support of object without `ace_cargo_size` defined in config or `-1` config value May 31, 2018

@PabstMirror PabstMirror added this to the 3.13.0 milestone Jun 1, 2018

@PabstMirror

This comment has been minimized.

Collaborator

PabstMirror commented Jun 1, 2018

looks good, it does require that people set reasonable cargo sizes for items they load

@PabstMirror PabstMirror merged commit f994365 into acemod:master Jun 1, 2018

1 check passed

ci/circleci Your tests passed on CircleCI!
Details

@Vdauphin Vdauphin deleted the Vdauphin:Extend_findUnloadPosition_to_object branch Jun 1, 2018

@PabstMirror PabstMirror modified the milestones: 3.13.0, 3.12.3 Jul 5, 2018

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