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

changed interaction to only one interaction point #406

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

y0014984
Copy link
Owner

@y0014984 y0014984 commented Dec 21, 2023

Merging this Pull Request will merge the different AE3 interaction points into one interaction menu. So there are no more doubled "Laptop" interaction points and also the interaction to access armaOS or attach USB flash drive were merged into the main interaction menu.

@y0014984 y0014984 self-assigned this Dec 21, 2023
@y0014984 y0014984 added enhancement New feature or request interaction labels Dec 21, 2023
@y0014984 y0014984 linked an issue Dec 21, 2023 that may be closed by this pull request
@y0014984 y0014984 marked this pull request as ready for review December 21, 2023 10:30
Copy link
Collaborator

@GermanHydrogen GermanHydrogen left a comment

Choose a reason for hiding this comment

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

Pulling all the action functions inside the !isDedicated conditionals worsens the readability of the overall functions. The performance price for that should be negligible.

addons/flashdrive/functions/fnc_initInterface.sqf Outdated Show resolved Hide resolved
addons/flashdrive/functions/fnc_initInterface.sqf Outdated Show resolved Hide resolved
addons/network/functions/fnc_initRouter.sqf Show resolved Hide resolved
addons/power/functions/fnc_initBattery.sqf Show resolved Hide resolved
addons/power/functions/fnc_initGenerator.sqf Show resolved Hide resolved
addons/power/functions/fnc_initSolarPanel.sqf Show resolved Hide resolved
@y0014984
Copy link
Owner Author

y0014984 commented Jan 8, 2024

See my changes for review.

Now that all interactions are in one place we can decide what is better: Option A (all together) or Option B (grouped by Addon). How do you think about it @GermanHydrogen

AE3-Interactions-Decision

Copy link
Collaborator

@GermanHydrogen GermanHydrogen left a comment

Choose a reason for hiding this comment

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

The one commend applies for all power device init functions

_handle = [_battery] spawn AE3_power_fnc_checkBatteryLevelAction;
},
{alive _target},
{},
[_battery]
] call ace_interact_menu_fnc_createAction;

[_entity, 0, ["ACE_MainActions", "AE3_DeviceAction"], _check] call ace_interact_menu_fnc_addActionToObject;
private _parentActionPath = _entity getVariable ["AE3_parentActionPath", ""];
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is the default a valid input for ace_interact_menu_fnc_addActionToObject?

We should decide if we want to recover here if AE3_parentActionPath is not set, so to just use the "ACE_MainActions" as a parent, or if we want to throw an exception in this case

_actions;
};
params ["_target", "_player", "_params"];
(_params select 0) params ["_index", "_name", "_rel_pos", "_rot_yaw", "_rot_pitch", "_rot_roll"];

Choose a reason for hiding this comment

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

Suggested change
(_params select 0) params ["_index", "_name", "_rel_pos", "_rot_yaw", "_rot_pitch", "_rot_roll"];
private _driveInfo = _params select 0;
if !(_driveInfo isEqualType []) then {
_driveInfo = _params select 1;
};
_driveInfo params ["_index", "_name", "_rel_pos", "_rot_yaw", "_rot_pitch", "_rot_roll"];

This change fixes #414, details here.

Choose a reason for hiding this comment

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

Thanks to Dystopian's acemod/ACE3#9946, this rough fix won't be necessary, the next ACE3 Patch version will resolve the issue.

Choose a reason for hiding this comment

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

This has now been fixed publicly by ACE v3.17.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request interaction
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

merge all AE3 interaction menus
3 participants