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

Medical & Captive Load Menu Overhaul #5519

Merged
merged 12 commits into from
Sep 29, 2017
Merged

Medical & Captive Load Menu Overhaul #5519

merged 12 commits into from
Sep 29, 2017

Conversation

654wak654
Copy link
Contributor

When merged this pull request will:

  • Add child actions to "Load Patient" and "Load Captive" for all near vehicles with seats.
  • Make captives component use more person loading code from ace_common. Although the code could still use some refactoring, I left it alone because I don't have a grip over all of it.
  • Shamelessly steal most code from Cargo load menu overhaul #4871

TODO: captive seems to rely on it's own functions and standards for
loading people. Make it more like medical & use common's functions
@jonpas jonpas added kind/documentation kind/feature Release Notes: **ADDED:** labels Sep 16, 2017
@jonpas jonpas added this to the 3.11.0 milestone Sep 16, 2017
@@ -0,0 +1,42 @@
/*
* Author: 654wak654
* Adds child actions to the "load patient" action for near vehicles
Copy link
Member

Choose a reason for hiding this comment

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

Missing final dot.

* 1: The patient <OBJECT>
*
* Return Value:
* The child actions
Copy link
Member

Choose a reason for hiding this comment

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

<ARRAY>

Also "The" is superfluous everywhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I copied most things from other medical headers so I'm keeping all the "the"s.

Copy link
Member

Choose a reason for hiding this comment

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

We should go for no "the"s, majority (I hope) is not using them, it's superfluous and quiet silly.

Copy link
Contributor

Choose a reason for hiding this comment

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

at least for non-medical files

@@ -0,0 +1,42 @@
/*
* Author: 654wak654
* Adds child actions to the "load captive" action for near vehicles
Copy link
Member

Choose a reason for hiding this comment

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

Missing final dot.

@@ -51,6 +51,7 @@ class CfgVehicles {
showDisabled = 0;
icon = QPATHTOF(UI\captive_ca.paa);
priority = 2.2;
insertChildren = QUOTE([ARR_2(_player, _target)] call DFUNC(addLoadCaptiveActions));
Copy link
Member

Choose a reason for hiding this comment

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

Just _this call... works here as well, so you don't need to do macro shenanigans.

Copy link
Contributor

Choose a reason for hiding this comment

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

_this is [_target, _player] and @654wak654 wants reversed array (I think).
Also space in ARR_2 macro. Also see @commy2 comment #5384 (review)

Copy link
Member

Choose a reason for hiding this comment

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

He is calling his own function, may as well revers the order there.

Single quotes are fine but you lose syntax highlighting because it's all a long string then, personal preference. I prefer single quotes if it really makes the thing shorter, but in this case you don't need anything but QUOTE and DFUNC.

@PabstMirror
Copy link
Contributor

PabstMirror commented Sep 16, 2017

I will try to test this later, but there are some delicate issue when loading a person into a turret or FFV "gunner" position

jonpas
jonpas previously requested changes Sep 16, 2017
Copy link
Member

@jonpas jonpas left a comment

Choose a reason for hiding this comment

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

I meant to press "request changes" above...


params ["_unit"];

private _nearVehicles = nearestObjects [_unit, ["Car", "Air", "Tank", "Ship_F", "Pod_Heli_Transport_04_crewed_base_F"], 10];
Copy link
Contributor

Choose a reason for hiding this comment

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

Making the distance configurable by allowing it to be passed along as a parameter would be more flexible.

params ["_unit", ["_distance", 10]];

private _nearVehicles = nearestObjects [_unit, ["Car", "Air", "Tank", "Ship_F", "Pod_Heli_Transport_04_crewed_base_F"], _distance];

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because of this function's very specific purpose it's good that everyone gets the same search distance IMO.

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 a common function though. Name of it implies it finds nearest no matter the distance. As a common function I agree with @thojkooi, can always simply call it with distance of 10 from medical/captives.

@jonpas jonpas dismissed their stale review September 17, 2017 22:43

Requested changes addressed.

@jonpas
Copy link
Member

jonpas commented Sep 22, 2017

Handcuffing a unit and loading (via captives) ✔️
Killing/downing a unit and loading (via medical) ✔️
Handcuffing a unit, then killing/downing a unit and loading (via captives) ✔️
Handcuffing a unit, then killing/downing a unit, then un-handcuffing a unit and loading (via either) ❌

@654wak654
Copy link
Contributor Author

Handcuffing a unit, then killing/downing a unit, then un-handcuffing a unit and loading (via either) ❌

This has been fixed by #5544

@jonpas jonpas merged commit 45a66cc into acemod:master Sep 29, 2017
@654wak654 654wak654 deleted the medical-loading branch September 29, 2017 20:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/documentation kind/feature Release Notes: **ADDED:**
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants