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

Add fast mouse actions to refuel, add refuel eden attributes #5418

Merged
merged 14 commits into from
Sep 10, 2017

Conversation

Dystopian
Copy link
Contributor

@Dystopian Dystopian commented Aug 11, 2017

When merged this pull request will:

  • title
    1 2
  • remove mouse wheel actions added with addAction;
  • prevent nozzle connecting to static weapons and electric vehicles (e.g. darter, SDV);
  • add new isNotRefueling interaction condition;
  • allow refuel on ladder;
  • fix AI unit can't run if player at its slot disconnected while nozzle carrying;
  • fix Make Refuel capabilities dynamic #5173;
  • add refuel Eden attributes.

Also:

  • fnc_takeNozzle is rewritten (no code duplicating);
  • some event handlers are replaced with PFH checking (no code dup and faster);
  • some code is optimized;
  • DefaultAction workaround is not used. Unit can't shoot with nozzle in hands, so I decided it will be easier to handle mouse clicking. Tell me if I'm wrong;
  • take a look at recent getCursorObjectParams command using, it allowed me to avoid some *intersect* code. Maybe it could be useful in other modules.

Here I have some points I would like (or be able) to implement but I need your opinion. At least I want you to say "yes", "no", "yes in another PR".

  • add AnimChanged handler, e.g. to drop nozzle when car pushes unit and he falls without dying;
  • replace setVar [..., objNull] with setVar [..., nil]. I think it's more logical to remove variable than set it to objNull;
  • unify variable names in all functions (_unit, _nozzle, _source (not _target), _sink look good to me);
  • remove ACE Connect action along with fnc_connectNozzle. ATM it runs another PFH with fast mouse actions, so it's not fully compatible with my PR. At least my way is faster and more obvious;
  • move all refuel actions to Interactions menu not Refuel submenu. In most cases all I want from refuel objects (truck, station, nozzle) is using Refuel actions, and submenu is overkill;
  • remove all reset functions. I think it's not good to transfer problems from devs to users with such functions, it's better to fix issues;
  • replace static menu for refuel vehicles with only some GVARs (like existing fuelCargo) to simplify 3rd party mods support and allow mission maker to assign his own refueler;
  • (next time) rework jerrycans, because attaching it to vehicle looks very odd;
  • maybe rename fnc_startNozzleInHandsPFH (I have problems with names);
  • maybe handle map/gear/etc opening and then RMB clicking - atm unit drops the nozzle on RMB clicking despite any dialog is opened;
  • maybe implement showHint fast updating. ATM there is a small fade in on hint changing (0.5s), I think it's OK. In explosive attach code there is a piece which do it immediately with direct control updating;
  • (someday) maybe implement digital display on fuel station :);
  • inform CUP devs about changes.

@jonpas jonpas added kind/enhancement Release Notes: **IMPROVED:** status/review-pending labels Aug 11, 2017
@jonpas jonpas added this to the 3.11.0 milestone Aug 11, 2017
@Dystopian
Copy link
Contributor Author

I assume there are no objections for all points.

@jonpas
Copy link
Member

jonpas commented Aug 13, 2017

move all refuel actions to Interactions menu not Refuel submenu. In most cases all I want from refuel objects (truck, station, nozzle) is using Refuel actions, and submenu is overkill;

I don't think that would be too wise, especially not on fuel trucks where you have a few refuel actions (taking nozzle, checking amount left, etc).

@Dystopian
Copy link
Contributor Author

@jonpas maybe fuel objects/stations not trucks?

@jonpas
Copy link
Member

jonpas commented Aug 14, 2017

They are using same set of actions, splitting them up would cause more issues than solve problems IMO, especially since any objects might get more actions in the future.

@Dystopian Dystopian changed the title Add fast mouse actions to refuel Add fast mouse actions to refuel, add refuel eden attributes Aug 26, 2017

if (isNull (_unit getVariable [QGVAR(nozzle), objNull])) exitWith {};

[_unit, "forceWalk", "ACE_refuel", false] call EFUNC(common,statusEffect_set);
Copy link
Contributor

Choose a reason for hiding this comment

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

statusEffect should take care of respawns automatically
or was this explicitly needed?

Copy link
Contributor Author

@Dystopian Dystopian Aug 27, 2017

Choose a reason for hiding this comment

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

it was in the original XEH_respawn.sqf, I didn't check.
You're right, I deleted it

@jonpas
Copy link
Member

jonpas commented Sep 7, 2017

@Dystopian Is this considered finished from your side? Also, are old interactions defined in config still compatible with this PR (thinking of 3rd party mods that use them for example) or will they have to update for compatibility?

@PabstMirror
Copy link
Contributor

I had an idea to prevent duplicate actions shown, will post something later today

@Dystopian
Copy link
Contributor Author

@jonpas it's done.
Old interactions aren't compatible atm. I thought nobody use ACE refuel configs except CUP (I'll inform them).
I can add additional check if @PabstMirror doesn't do it earlier.
Will be at my pc in 1 day.

@jonpas
Copy link
Member

jonpas commented Sep 7, 2017

It is likely some other 3rd party mod also uses it. I am sure CUP will update it ASAP, not worried about them. But if possible we should try to do backwards compatibility, and then remove it after 2 versions as is standard with ACE3.

Great work on this though! Especially for fulfilling my dynamic interaction wishes!

@Dystopian Dystopian force-pushed the add_refuel_fast_actions branch from 29ee376 to ff0b56e Compare September 9, 2017 19:00
@Dystopian Dystopian force-pushed the add_refuel_fast_actions branch from ff0b56e to 61e6def Compare September 9, 2017 19:08
@Dystopian
Copy link
Contributor Author

@jonpas done

@jonpas
Copy link
Member

jonpas commented Sep 10, 2017

That just prints deprecation message to RPT, but is the old interaction menu still compatible with the rest of the rework done in this PR? There is no point saying it's deprecated and will be removed in 3.13.0 if it doesn't work with 3.11.0 in the first place.

@jonpas
Copy link
Member

jonpas commented Sep 10, 2017

Clarified on Slack, old interaction menu still works, so backwards compatibility is there. 👍

Copy link
Contributor

@PabstMirror PabstMirror left a comment

Choose a reason for hiding this comment

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

Tested working
Skipping a interaction menu on the target makes this feel much smoother to use

@jonpas
Copy link
Member

jonpas commented Sep 10, 2017

What @PabstMirror said, also dynamic actions! 👍

Would be great if we can get framework documentation for that up as well. This is what we have currently. Then that can just be linked to CUP and whoever else uses it. Disregard, I am blind.

@PabstMirror PabstMirror merged commit 559a498 into acemod:master Sep 10, 2017
@Dystopian Dystopian deleted the add_refuel_fast_actions branch September 10, 2017 23:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Release Notes: **IMPROVED:**
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make Refuel capabilities dynamic
3 participants