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 Advanced Rappelling compatibility #700

Merged
merged 10 commits into from
Apr 11, 2023
Merged

Conversation

Kexanone
Copy link
Member

@Kexanone Kexanone commented Jan 14, 2023

When merged this pull request will:

  • Add compatibility addon for using Duda's Advanced Rappelling for fast-roping waypoint and reinforcements.
  • Slightly refactor fastroping in main addon

@Kexanone Kexanone added the feature Adds a new feature label Jan 14, 2023
@Kexanone
Copy link
Member Author

Kexanone commented Jan 14, 2023

To Do: Get reinforcements working

@mharis001
Copy link
Member

Maybe these should be two separate waypoints (Fastrope and Advanced Rappel) because the mods could be run together. We could add conditions to waypoints so that they only show if the required mods are loaded.

@Kexanone
Copy link
Member Author

Kexanone commented Jan 19, 2023

Sounds like a good alternative approach. Would you propose the AR waypoint in a compat. or part of the main addon? If I remember correctly we decided against having anything but ACE as soft dependency in the main addon, that's why I went the compat. route in the first place.

@mharis001
Copy link
Member

Yeah, I think we can revisit that decision for some other big mods. I think both a new component (separation of features/responsibilities) or just in the ai component are good options. With waypoint conditions, this feature could be added without a hard dependency.

@mharis001 mharis001 added this to the 1.14.0 milestone Apr 3, 2023
@mharis001
Copy link
Member

I separated out the waypoints into "Fastrope" and "Rappel" and reworked how reinforcements handles the different insertion methods for air vehicles. I think we should keep the Advanced Rapelling compatibility as a separate optional component.

@mharis001
Copy link
Member

@Kexanone, I think this is good to go. Please look over my changes and merge when ready.

@Kexanone
Copy link
Member Author

Kexanone commented Apr 11, 2023

Looks good to me, but still needs to be tested if you haven't already.

@mharis001
Copy link
Member

I did the usual testing, and everything seemed okay. I think we are good to merge.

@Kexanone Kexanone merged commit 0dca83f into master Apr 11, 2023
@Kexanone Kexanone deleted the advanced-rappelling-compat branch April 11, 2023 20:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Adds a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants