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

Aircraft - Drone "Follow Unit" Waypoint Action #9889

Merged
merged 20 commits into from
Jul 2, 2024

Conversation

mrschick
Copy link
Contributor

@mrschick mrschick commented Mar 31, 2024

When merged this pull request will:

  • Add a "Follow Unit" Waypoint Action to Drones, to emulate the vanilla "Follow Unit" behaviour from the UAV Terminal, which is unfortunately only available on other drones unless "friendly map content" is enabled;
    • Follow action is available when pointing at a man/vehicle (currently via cursorTarget) and creates a persistent "follow" type waypoint;
      • Works from UAVs;
      • Works from UGVs;
    • Follow action permits selecting a min distance to follow to, just like the vanilla interface (unclear how that logic works);
  • Fix the "recharge" action being shown on destroyed drones;

@mrschick
Copy link
Contributor Author

Looks like UGV Follow code is broken and a known issue. I guess an all-round solution will require some sort of follow-mode PFH that updates a move/hold waypoint, which should also allow scripting a "min distance to follow" behaviour, though a native scripting method would be preferable.

@mrschick
Copy link
Contributor Author

mrschick commented Apr 16, 2024

Ended up implementing the selectable follow distance via a simple distance check in the PFH, should be pretty efficient already but could be optimized further.
I guess the interaction to set follow distance could be more user friendly too.

Feedback appreciated.

@mrschick mrschick marked this pull request as ready for review April 16, 2024 09:00
@mrschick
Copy link
Contributor Author

I guess the current implementation of a separate "Follow Distance" interaction (just like for Loiter Altitude) is best.
It still needs a stringtable entry for localization, Loiter Altitude uses the vanilla $STR_3den_waypoint_attribute_loiteraltitude_displayname, so far I haven't found anything similar for Follow Distance. I doubt there is though, so it will probably need to be added.

@mrschick
Copy link
Contributor Author

mrschick commented Apr 19, 2024

I think this is pretty much complete.
Maybe some Air vehicles should also be followable, might be useful when tracking other small drones with a drone 🤔

@LinkIsGrim LinkIsGrim added the kind/enhancement Release Notes: **IMPROVED:** label Jun 23, 2024
@LinkIsGrim LinkIsGrim added this to the 3.18.0 milestone Jun 23, 2024
@PabstMirror PabstMirror self-requested a review June 23, 2024 23:11
@johnb432
Copy link
Contributor

Looks like UGV Follow code is broken and a known issue. [...]

I want to see that ticket be included in the code, explaining why a PFH even needs to be added.

@johnb432
Copy link
Contributor

I think this is pretty much complete. Maybe some Air vehicles should also be followable, might be useful when tracking other small drones with a drone 🤔

I really don't know: I don't use drones at all and frankly I don't care for them. We need other opinions here.

@LinkIsGrim
Copy link
Contributor

I'll test later this week. Code looks good at a glance post-changes, but I haven't looked too hard.

@PabstMirror
Copy link
Contributor

I could see someone saying this is too strong because the drone will continue to follow even if it doesn't have line of sight
e.g. vehicle in forest or human inside of a building
but there is always a way to disable with getVariable [QGVAR(droneWaypoints)
and I think most use of this will be perfectly fine

@mrschick
Copy link
Contributor Author

The vanilla following was similarly OP, it would get stuck sometimes if the drone's turret wasn't looking at the target, but once you'd lock it with Ctrl+T and "revealed it" with T, the AI would know about it even when in buildings and follow with few interruptions.
Also I think most use-cases (especially with UGV) will be following of the drone operator, which the drone should always know where to find due to the terminal's DL.

Copy link
Contributor

@LinkIsGrim LinkIsGrim left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@johnb432 johnb432 left a comment

Choose a reason for hiding this comment

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

@mrschick I can't push any changes on your branch.
I believe it's because you don't allow contributors to commit to PR branches directly. If you enable it, it allows us to merge master when we want to test and makes it easier to commit small changes.

I want to merge the master branch, along side with some minor changes (code cleanup, header fix) and I don't want to have to do this via the website, so please allow me to push changes.

@mrschick
Copy link
Contributor Author

@johnb432 Unfortunately maintainer edits cannot be enabled for organization PRs.
https://github.com/orgs/community/discussions/5634

@mrschick mrschick changed the base branch from master to empty-mags June 30, 2024 21:58
@mrschick mrschick changed the base branch from empty-mags to master June 30, 2024 21:58
@mrschick
Copy link
Contributor Author

Running git merge --squash master to merge while keeping linear history caused the PR to show with 731 files modified. Fixed by rebasing and force pushing.
I guess the "best" way would be running git merge master to not break any history, but it makes commit history messy IMO.

addons/aircraft/functions/fnc_droneAddActions.sqf Outdated Show resolved Hide resolved
addons/aircraft/functions/fnc_droneAddActions.sqf Outdated Show resolved Hide resolved
addons/aircraft/functions/fnc_droneAddActions.sqf Outdated Show resolved Hide resolved
addons/aircraft/functions/fnc_droneAddActions.sqf Outdated Show resolved Hide resolved
addons/aircraft/functions/fnc_droneAddActions.sqf Outdated Show resolved Hide resolved
addons/aircraft/functions/fnc_droneSetWaypoint.sqf Outdated Show resolved Hide resolved
Co-Authored-By: johnb432 <58661205+johnb432@users.noreply.github.com>
@johnb432 johnb432 merged commit aecafe6 into acemod:master Jul 2, 2024
5 checks passed
@mrschick mrschick deleted the feature/drone-improvements branch July 2, 2024 09:53
blake8090 pushed a commit to blake8090/ACE3 that referenced this pull request Aug 18, 2024
* Fix "Recharge" interaction showing on destroyed drone

* Add "Follow Unit" action

* Improve condition check

* UGV Following via PFH that updates WP Pos

* Use HOLD WP for all Follow Actions

Since FOLLOW WP would stop working on AI Soldiers after some time.

* Allow selecting a follow distance

* Follow Distance under separate interaction, just like Loiter Alt

Only visible when a HOLD waypoint is selected, which is pretty much always going to have been created by the "Follow" interaction.

* Localize "Follow" Interaction

* Show structuredText Hint when following/changing distance

* Variable for cursorTarget Reuse

* Better isKindOf condition use

* Make "Ship"-kind vehicles followable

* Clean up Comments and systemChat Debugs

* Comment explanation for custom PFH solution over vanilla "Follow"-WP

* Trim excess brackets from setWaypointPosition argument

Co-Authored-By: johnb432 <58661205+johnb432@users.noreply.github.com>

* Broader determination for UGV follow distances

Co-Authored-By: PabstMirror <pabstmirror@gmail.com>

* Prevent infinite PFH loop if follow target is deleted

Co-Authored-By: PabstMirror <pabstmirror@gmail.com>

* Delete Follow WP when PFH terminates

* The ternary rules

Co-authored-by: johnb432 <58661205+johnb432@users.noreply.github.com>

* Various requested changes

Co-Authored-By: johnb432 <58661205+johnb432@users.noreply.github.com>

---------

Co-authored-by: johnb432 <58661205+johnb432@users.noreply.github.com>
Co-authored-by: PabstMirror <pabstmirror@gmail.com>
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.

4 participants