Cargo load menu overhaul #4871

Dystopian · 2017-02-04T11:25:17Z

add submenu with vehicles to cargo load menu
rewrite Load menu action and add condition
clean code

Vehicles with alive crew are suffixed with its name (in brackets).

There are also some questions to resolve:

should statement for Load menu be deleted? ATM it works as usually - loads to the nearest vehicle, while submenu items load to the specified vehicle;
should Load menu be renamed to "Load to" e.g.? If yes statement should be deleted (previous question);
should show condition be modified to take into account the nearest vehicles?

commy2 · 2017-02-04T16:49:24Z

addons/cargo/functions/fnc_addCargoVehiclesActions.sqf

+    _action = [format ["%1", _x], _name, _icon, _statement, {true}, {}, [_x]] call ace_interact_menu_fnc_createAction;
+    _actions pushBack [_action, [], _target];
+    false
+} count (_player nearEntities [["Car", "Air", "Tank", "Ship", "Cargo_base_F"], MAX_LOAD_DISTANCE]);


Not all vehicles can load stuff. I don't see that filtered here.

I mean by that, what happens if you're near a vehicle that is Car, but cannot load anything? (e.g. a Kart)

"could not be loaded" message appears as earlier in "Load" action. And I think it's OK, in RL you can't magically see which vehicles have enough room.
As for Kart - maybe it has sense to check vehicles cargo space config in show condition. But this behavior differs from current "Load" action, I didn't want to change it. If you insist I'll add it and remove "Load" action.

commy2 · 2017-02-04T16:50:40Z

addons/cargo/functions/fnc_addCargoVehiclesActions.sqf

+#include "script_component.hpp"
+
+params ["_target", "_player"];
+private ["_statement", "_actions", "_name", "_ownerName", "_icon", "_action"];


please use private keyword. ~~The variables from L31 onwards aren't even in the same script instance!~~

jonpas · 2017-02-04T18:30:06Z

should statement for Load menu be deleted? ATM it works as usually - loads to the nearest vehicle, while submenu items load to the specified vehicle;

I'd keep it.

should Load menu be renamed to "Load to" e.g.? If yes statement should be deleted (previous question);

I don't think so, as it has it's own action as well I think it's good as is.

should show condition be modified to take into account the nearest vehicles?

That probably wouldn't be bad if you can sort by distance easily without losing on performance.

commy2 · 2017-02-04T22:11:05Z

addons/cargo/functions/fnc_startLoadIn.sqf

+TRACE_3("params",_player,_object,_vehicle);
+
+if (isNull _vehicle) then {
+    _vehicle = [_player, _object] call FUNC(findNearestVehicle);


Please don't reuse variable names in this way. Name it differently and set it to private.

If you mean "_vehicle" I don't reuse it. I need it set to vehicle from parameters or from nearest vehicles.
If you mean something else I don't understand.

Or you mean I shouldn't reuse variable which is from params? OK, I'll fix it.

Also I'll add new argument description.

commy2 · 2017-02-04T22:12:45Z

addons/cargo/functions/fnc_addCargoVehiclesActions.sqf

+        _name = format ["%1 (%2)", _name, _ownerName];
+    };
+    private _icon = (getText (configFile >> "CfgVehicles" >> typeOf _x >> "icon")) call BIS_fnc_textureVehicleIcon;
+    private _action = [format ["%1", _x], _name, _icon, _statement, {true}, {}, [_x]] call ace_interact_menu_fnc_createAction;


EFUNC(interact_menu,createAction)

commy2 · 2017-02-04T22:13:17Z

addons/cargo/functions/fnc_startLoadIn.sqf

+    if ((isNull _vehicle) || {_vehicle isKindOf "Cargo_Base_F"}) then {
+        {
+            if ([_object, _x] call FUNC(canLoadItemIn)) exitWith {_vehicle = _x};
+        } forEach (nearestObjects [_player, ["Cargo_base_F", "Land_PaperBox_closed_F"], MAX_LOAD_DISTANCE]);


Not a fan of magic numbers in code.

It's not my code. I just put it inside if-then. Anyway I don't understand about magic numbers.

Hard coded classnames like "Cargo_base_F" and especially "Land_PaperBox_closed_F".
This will make it harder to add compatibility with new objects later, because you have to alter lines buried in the code instead of a separate config and it will make it almost impossible for 3rd party mods.

Dystopian · 2017-02-06T22:27:58Z

WIP

Dystopian · 2017-02-08T21:37:43Z

All done, please review

PabstMirror

Tried this out and like it a lot

Good simplification of the code, while adding a feature
Also like how the base action "Load" still works and will just select the closest vehicle automatically (could possibly not show sub actions if there is only 1 vic in range, but that might be confusing as well?)

kymckay · 2017-05-28T22:39:44Z

Yeah I'd definitely still show the sub action when only one is in range for consistency

Add submenu with vehicles to cargo load menu

0251a26

commy2 reviewed Feb 4, 2017

View reviewed changes

replace private ARRAY with keyword

734b9e7

commy2 reviewed Feb 4, 2017

View reviewed changes

Dystopian added 4 commits February 5, 2017 13:13

fix ace function macro using

31ee49a

filter vehicles without cargo

1bb4678

add Load condition, clean params, fix param reusing

8859e64

replace nearEntities with nearestObjects, add macro

c4c9df1

Dystopian added 2 commits February 8, 2017 23:33

optimize, del magic, replace count with forEach

e808f36

del unused functions

74934bc

jonpas added kind/feature Release Notes: **ADDED:** status/WIP labels Feb 8, 2017

jonpas added this to the 3.10.0 milestone Feb 8, 2017

Dystopian changed the title ~~Add submenu with vehicles to cargo load menu~~ Cargo load menu overhaul Feb 8, 2017

PabstMirror added status/needs-testing and removed status/WIP labels Feb 8, 2017

del useless _this parameter

af97b80

PabstMirror approved these changes May 26, 2017

View reviewed changes

PabstMirror added kind/enhancement Release Notes: **IMPROVED:** and removed status/needs-testing labels May 26, 2017

thojkooi merged commit 9f291c8 into acemod:master May 31, 2017

Dystopian deleted the cargo-vehicles-submenu branch August 16, 2017 22:03

654wak654 mentioned this pull request Sep 16, 2017

Medical & Captive Load Menu Overhaul #5519

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cargo load menu overhaul #4871

Cargo load menu overhaul #4871

Dystopian commented Feb 4, 2017 •

edited

Loading

commy2 Feb 4, 2017

commy2 Feb 4, 2017

Dystopian Feb 4, 2017

commy2 Feb 4, 2017 •

edited

Loading

jonpas commented Feb 4, 2017 •

edited

Loading

commy2 Feb 4, 2017

Dystopian Feb 4, 2017

Dystopian Feb 4, 2017

Dystopian Feb 4, 2017

commy2 Feb 4, 2017

Dystopian Feb 4, 2017

commy2 Feb 4, 2017

Dystopian Feb 4, 2017

commy2 Feb 5, 2017 •

edited

Loading

Dystopian commented Feb 6, 2017

Dystopian commented Feb 8, 2017

PabstMirror left a comment

kymckay commented May 28, 2017

Cargo load menu overhaul #4871

Cargo load menu overhaul #4871

Conversation

Dystopian commented Feb 4, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

commy2 Feb 4, 2017 • edited Loading

Choose a reason for hiding this comment

jonpas commented Feb 4, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

commy2 Feb 5, 2017 • edited Loading

Choose a reason for hiding this comment

Dystopian commented Feb 6, 2017

Dystopian commented Feb 8, 2017

PabstMirror left a comment

Choose a reason for hiding this comment

kymckay commented May 28, 2017

Dystopian commented Feb 4, 2017 •

edited

Loading

commy2 Feb 4, 2017 •

edited

Loading

jonpas commented Feb 4, 2017 •

edited

Loading

commy2 Feb 5, 2017 •

edited

Loading