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

Improve attach objects module #752

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

ampersand38
Copy link
Member

@ampersand38 ampersand38 commented Oct 9, 2023

When merged this pull request will:

@ampersand38 ampersand38 added the enhancement Improves an existing feature label Oct 9, 2023
@mharis001 mharis001 added this to the 1.15.0 milestone Oct 10, 2023
@mharis001
Copy link
Member

I think maybe this should be a separate keybind. I think it would be annoying to go through the dialog every time you want to attach an object.

@ampersand38
Copy link
Member Author

I think maybe this should be a separate keybind. I think it would be annoying to go through the dialog every time you want to attach an object.

Can do. I'm leaning toward "Attach to Bone" since the description mentions "selected objects", which could get confused with "Attach to Selection".
What behaviour do you want for the attach module?

@ampersand38
Copy link
Member Author

And what do you think about attempting to translate from Czech names? https://community.bistudio.com/wiki/ArmA:_Armed_Assault:_Selection_Translations

addons/attached_objects/functions/fnc_attachBone.sqf Outdated Show resolved Hide resolved
} else {
[_x, _entity, _selection, _isRelative] call FUNC(attachToSelection);
};
} forEach _objects params ["_selectionPos", "_selectionY", "_selectionZ"];
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this actually the intention ?

^ Since _objects will be iterated before params gets to it, and params returns bool based on the success of, or use of a default value in, the creation of the mentioned vars, so even putting it as forEach (...) would give an error, IIUC, and the mentioned vars are only used after the forEach.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, yeah you're right. forEach does return the last iteration, but it's not a good use for it, and it was in the wrong place relative to the if anyway.

@mharis001
Copy link
Member

I think the module can have the "full" attach to options. If it is doable, translating the names would be good, similarly to how hitpoint names are handled.

@mharis001 mharis001 modified the milestones: 1.15.0, 1.16.0 Jul 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improves an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants