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.

Sign up for GitHub

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

Jump to bottom

Prevent ACEX HC transfer before unloading #479

Merged
merged 10 commits into from
Oct 20, 2019
Merged

Prevent ACEX HC transfer before unloading #479

merged 10 commits into from
Oct 20, 2019

Conversation

ampersand38
Copy link
Contributor

Reinforcement groups ownership transfer can break the unload waypoint, causing the infantry to fail to disembark.

This PR checks for presence of ACEX headless, blacklists the vehicle and infantry groups, and removes the blacklist after they reach their waypoints.

@ampersand38
Prevent ACEX HC transfer before unloading
e90c48f 
Reinforcement groups ownership transfer can break the unload waypoint, causing the infantry to fail to disembark.

This PR checks for presence of ACEX headless, blacklists the vehicle and infantry groups, and removes the blacklist after they reach their waypoints.
@CreepPork
Copy link
Member

Did you test it with and without ACEX?

@CreepPork CreepPork added feature PR that adds a new feature to Achilles. fix PR that is a fixes a bug. priority/medium Issue or PR that impacts some portions of the user base and should be resolved with due time. 3rd party mod Issue or PR that involves 3rd party mods. testing/not tested Indicates that the PR's latest commit has not been tested and needs testing before merging. labels Mar 28, 2019
@CreepPork CreepPork added this to the 1.3.0 milestone Mar 28, 2019
@ampersand38
Copy link
Contributor Author

ampersand38 commented Mar 28, 2019 •
edited
Loading

Actually, I copied in from the wrong editor window. The setWaypointStatements should have single quotes around acex_headless_blacklist. Should I close this one and re-submit?
Edit: I found where to fix it.

@ampersand38
Copy link
Contributor Author

ampersand38 commented Mar 29, 2019 •
edited
Loading

I don't know how the heck I uploaded such a wrong file.
Here's the unsigned pbo I tested with.
modules_f_ares.zip

@neilzar @ampersand38
Update @AresModAchillesExpansion/addons/modules_f_ares/Reinforcements…
7b68dec 
…/functions/fn_ReinforcementsCreateUnits.sqf

Co-Authored-By: ampersand38 <ampersand38@gmail.com>
@CreepPork CreepPork requested a review from neilzar April 4, 2019 14:58
CreepPork
CreepPork previously approved these changes Apr 4, 2019
View reviewed changes
Kexanone
Kexanone reviewed Apr 22, 2019
View reviewed changes
Copy link
Member

@Kexanone Kexanone left a comment

Choose a reason for hiding this comment

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

Could suffer from a race condition when the waypoint for some reason gets completed before setting the waypoint statement. The current scheduled framework does not make life easy.

@CreepPork CreepPork added testing/not tested Indicates that the PR's latest commit has not been tested and needs testing before merging. and removed testing/not tested Indicates that the PR's latest commit has not been tested and needs testing before merging. labels Apr 27, 2019
@CreepPork CreepPork added the no response Issue that has no response from the original author of said issue. label Aug 28, 2019
@CreepPork
Copy link
Member

@ampersand38
Will the issue be resolved (pull request comment) or can I close the pull request?

@ampersand38
Copy link
Contributor Author

Will investigate further on Friday.

@ampersand38
Copy link
Contributor Author

Could suffer from a race condition when the waypoint for some reason gets completed before setting the waypoint statement. The current scheduled framework does not make life easy.

Is this possible because addWaypoint and setWaypointStatements are quite far apart? Would the chance be reduced if they were consecutive commands?

@ampersand38
Update fn_ReinforcementsCreateUnits.sqf
6da31d4 
Moved `setWaypointStatements` closer to `addWaypoint` to reduce chance of race condition.
@CreepPork
Copy link
Member

@oOKexOo ^^

@ampersand38
Copy link
Contributor Author

Tested on dedi server with this pbo.
modules_f_ares.zip

@CreepPork CreepPork removed the no response Issue that has no response from the original author of said issue. label Sep 1, 2019
@CreepPork CreepPork requested a review from Kexanone September 1, 2019 17:43
@Kexanone
Copy link
Member

Is this possible because addWaypoint and setWaypointStatements are quite far apart? Would the chance be reduced if they were consecutive commands?

Yes it would, but the cleaner way is to put them in a unscheduled block (e.g. isNil {...} or CBA_fnc_directCall.

@CreepPork CreepPork merged commit ad0edee into ArmaAchilles:master Oct 20, 2019
@ampersand38 ampersand38 deleted the patch-1 branch October 21, 2019 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@CreepPork CreepPork CreepPork approved these changes

@neilzar neilzar Awaiting requested review from neilzar

@Kexanone Kexanone Awaiting requested review from Kexanone

Assignees

@ampersand38 ampersand38

Labels
3rd party mod Issue or PR that involves 3rd party mods. feature PR that adds a new feature to Achilles. fix PR that is a fixes a bug. priority/medium Issue or PR that impacts some portions of the user base and should be resolved with due time. testing/not tested Indicates that the PR's latest commit has not been tested and needs testing before merging.
Projects
None yet
Milestone
1.3.0
Development

Successfully merging this pull request may close these issues.
4 participants
@ampersand38 @CreepPork @Kexanone @neilzar