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

Quick Mount - Add Get In and Change seat action menu #5745

Merged
merged 38 commits into from
Nov 19, 2018

Conversation

Dystopian
Copy link
Contributor

When merged this pull request will:

  • add menu to take any seat in vehicle including any passenger seat;
  • add menu to change seat inside vehicle respecting game config values for that.
    1
    2

I think this menu is perfect continuation of Quick Mount component. Also it can be used to work around vanilla bug when you can change seat from passenger to driver but not in reverse order (e.g. in vanilla offroad). And it's always nice to change passenger seat closer or farther to window, to enjoy nature view or hide from bullets.

Icons are not ideal but really cute.

Unfortunately MoveTo* actions (which are intended for seat changing) don't work correctly when there is somebody else in vehicle (even AI). So for Change seat menu I had to use moveOut + moveIn* commands. Side effects: GetIn and GetOut EH on each seat change, dead bodies are ejected from their seats. But I don't think it's a big deal.

Things to discuss:

  • delete menu for static turrets?
  • change menu priority to pull it down?
  • describe in code how the game handles config values to manage To Smbdy seat actions displaying?
  • I don't really know if isTurnedOut command should be used to restrict menu. On one hand vanilla actions are disabled when you're turned out, on other hand we already break vanilla rules for taking seat with Quick Mount.

@jonpas
Copy link
Member

jonpas commented Nov 12, 2017

I like this overall, however I think we should respect compartment config of the vehicle. In some vehicles you are not able to realistically move from some seat to another, eg. front of truck to the back (let's ignore people who want to fall off).
I see it already does that, my bad, commented before I checked the code! :)

Maybe add a setting to disable that sub-menu for people who don't want extra options they might not use?

@jonpas jonpas added the kind/feature Release Notes: **ADDED:** label Nov 12, 2017
@jonpas jonpas added this to the 3.13.0 milestone Nov 12, 2017
@Dystopian
Copy link
Contributor Author

Dystopian commented Nov 12, 2017

CI tools fail because of multi-line macro.

I don't really like additional setting idea (besides implemented ace_quickmount_enabled outside of vehicle), but if you insist, I'll add it.

@commy2
Copy link
Contributor

commy2 commented Nov 12, 2017

Remove the ; from the last line of the macro and add it after GETIN_ACTIONS (GETIN_ACTIONS;).

@Dystopian
Copy link
Contributor Author

@commy2 is it for config style or to workaround sqf validator issue? If first - there also similar configs in other components, if last - it's not about config but about macro in sqf.

@commy2
Copy link
Contributor

commy2 commented Nov 12, 2017

All lines should end with ;
Even if they use a macro.
The macro itself isn't supposed to end with ;

@commy2
Copy link
Contributor

commy2 commented Nov 12, 2017

See? And now Travis is happy.

@Dystopian
Copy link
Contributor Author

travis was already happy after 8af3296 :P

@Dystopian
Copy link
Contributor Author

will add setting to disable menu and workaround for seat changing while land vehicle is moving fast (atm unit gets damage).

@Dystopian
Copy link
Contributor Author

can't fix CI fails due to #5979 :(

@Dystopian
Copy link
Contributor Author

Did you know that unit can't get in flipped vehicle even with moveIn* commands? That's bad news for Change seat feature :( I have to search for workaround.

@PabstMirror
Copy link
Contributor

I'd really rather use the action ["moveTo commands if possible.

With a player in the vic they seemed to work just fine, I only had a problem when a AI was in driver's seat. But the vanilla switch actions didn't show on the action menu either.

Maybe we can determine the canShow condition logic and replicate.

Copy link
Contributor

@dedmen dedmen left a comment

Choose a reason for hiding this comment

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

Would approve. But I'm not as I'm not concentrated enough to spot every problem now.
Looks good overall.

* Public: No
*/

#define TO_STRING(var) if !(var isEqualType "") then {var = format [ARR_2("Compartment%1",var)]}
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe call it TO_COMPARTMENT_STRING. As this is specialized.

_cargoCompartments = getArray (_vehicleConfig >> "cargoCompartments");
{
if !(_x isEqualType "") then {
_cargoCompartments set [_forEachIndex, format ["Compartment%1", _x]];
Copy link
Contributor

Choose a reason for hiding this comment

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

use apply
Also you made a macro for this. Why don't you use it now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Look how it was with apply: 9e17c18#diff-ded96a376761ae515c888f369fb06b23R55
I changed it to forEach only last days because IMHO it looks better and clearer.

Macro sets variable not array member so I can't use it here.

Copy link
Contributor

Choose a reason for hiding this comment

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

_cargoCompartments = _cargoCompartments apply {[format ["Compartment%1", _x], -x] select (_x isEqualType "")}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will test in a week

Copy link
Contributor Author

Choose a reason for hiding this comment

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

apply is slower (even without intermediate cargoCompartments assignment) and less clear ✋

Copy link
Contributor

@dedmen dedmen Oct 23, 2018

Choose a reason for hiding this comment

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

Don't understand how it can be. If you are doing the same thing, but with less commands and less variables (forEachIndex) it shouldn't possibly be slower.

Edit: Ah understood now.

If you are doing the same thing

yeah they aren't. My variant copies _x if it's already a string. Yours just does nothing.

Do you have an example vehicle where cargoCompartments isn't actually an array of strings?
Oh okey. Compartment1 is actually a enum that evaluates to 1.
Compartment2 = 2
Compartment3 = 4
Compartment4 = 8 and so on.. Meh.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

0.0083 ms

private _cargoCompartments = getArray (configFile >> "CfgVehicles" >> typeOf cursorobject >> "cargoCompartments"); 
{ 
    if !(_x isEqualType "") then { 
        _cargoCompartments set [_forEachIndex, format ["Compartment%1", _x]]; 
    }; 
} forEach _cargoCompartments; 
_cargoCompartments 

vs
0.0108 ms

private _cargoCompartments = getArray (configFile >> "CfgVehicles" >> typeOf cursorobject >> "cargoCompartments") apply {[format ["Compartment%1", _x], _x] select (_x isEqualType "")}; 
_cargoCompartments 

when cursorObject is I_Truck_02_covered_F

Copy link
Contributor Author

@Dystopian Dystopian Oct 23, 2018

Choose a reason for hiding this comment

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

@dedmen when you edit your message nobody gets notification unlike when you create message (maybe in github desktop?). I really accidentally found your question.

Do you have an example vehicle where cargoCompartments isn't actually an array of strings?

CUP_Ural_ZU23_Base, rhs_truck, rhsusf_fmtv_base and some more RHS vehicles. No in vanilla.

Copy link
Contributor

Choose a reason for hiding this comment

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

Well I expected you read the message on github. Instead of in a notification E-Mail or wherever.


// find current compartment
(
_fullCrew select (_fullCrew findIf {_player == _x select 0})
Copy link
Contributor

Choose a reason for hiding this comment

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

this will error if player is not in crew. Are you sure he is?

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, if (_isInVehicle) then { ^^ L88

@Dystopian
Copy link
Contributor Author

Dystopian commented Oct 23, 2018

Found small bug with some RHS vehicles. Will investigate.
RHS 😠

// workaround getting damage when moveout while vehicle is moving
#define MOVEOUT_AND_BLOCK_DAMAGE \
if (isDamageAllowed _player) then { \
_player allowDamage false; \
Copy link
Contributor

Choose a reason for hiding this comment

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

Might want to use
[_player, "blockDamage", QUOTE(ADDON), true] call EFUNC(common,statusEffect_set);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

EFUNC(common,statusEffect_set) would set global variable twice in 1 second. I don't think it's worth it to use it just for 1 second allowDamage.

@Dystopian
Copy link
Contributor Author

Added workaround for changing seat to FFV turret with enabledByAnimationSource. This is Arma bug not RHS.

@commy2
Copy link
Contributor

commy2 commented Nov 5, 2018

It's been a year. I feel bad for Dystopian.

@PabstMirror PabstMirror modified the milestones: 3.13.0, 3.12.4 Nov 10, 2018
@PabstMirror
Copy link
Contributor

With CH-47 in flight, switching to rear gunner seats causes me to enter standing float animation (if ramp is closed)

Eliminating the delay }, _this, DISABLED_FFV_TIMEOUT] call CBA_fnc_waitAndExecute; seems to fix, but it sounds like that's needed for something else

@Dystopian
Copy link
Contributor Author

Bugged FFV seats are disabled.
Also fixed bug with local turrets in non-local vehicle.

@jonpas
Copy link
Member

jonpas commented Nov 18, 2018

@PabstMirror should review before merge as well.

@PabstMirror PabstMirror merged commit e8c3be8 into acemod:master Nov 19, 2018
@Dystopian Dystopian deleted the getin_menu branch November 19, 2018 05:50
BaerMitUmlaut pushed a commit that referenced this pull request Aug 5, 2019
* Add quickmount GetIn menu

* Add compartment support

* Check engine, check pilot, check static turret driver

* Add doc, condition, translation

* Add hybrid config entries, fix MP issue

* Optimize condition

* Ignore Enabled setting in vehicle

* Work around SQF validator macro issue

* Fix config macro entries

* Add workaround for getting damage when seat changing while moving

* Add setting for Get In menu disabling

* Fix race when 2 players try to get the same seat

* Convert if-else to switch

* Decrease move-back timeout to 0.5s

* Check if vehicle is flipped

* Add getin statement for parent menu

* Improve canShowFreeSeats

* Apply latest trends

* Improve fnc_addFreeSeatsActions

* Change copilot and gunless turret icons

* Fix macro name

* Fix FFV icon

* Optimize turret icon code

* Extend setting to 4 values

* Fix menu is shown when vehicle is full

* Optimize UAV checking code

* Fix bug with disabled FFV turrets

* Remove bugged FFV, Add turret locality check, Add Failed message

* Replace some macros with function

* Fix validator

* Remove global variables, Add debug
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Release Notes: **ADDED:**
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants