FAForever / fa

Lua code for FAF
221 stars 228 forks source link

Assisting engineers track new construction orders #6176

Closed clyfordv closed 1 month ago

clyfordv commented 1 month ago

Continuation of #6169 which was accidentally made on the main branch.

lL1l1 commented 1 month ago

Issues: Pods/engineers don't stop construction when the leader gets a stop order.

Just keeping track of this in the new PR.

it's a pain to send a shoulder pod off to build Template X, assign some kennel pods to assist, then have those assist orders disappear as soon as construction is complete and the shoulder pod redocks.

I see, so the assist order goes away when the pod docks, which is why we need to keep it on a separate unit. I didn't notice because I had somewhat of a bug where you can assign assisters to a pod and then have them start building something, cancel the build order for the pod, have it dock, and the assist order will stay on the pod until the other builders finish their task and check if they can assist the pod again.

Another solution is giving the shoulder pod an assist order to the base station instead of the (silent) "AssistCommander" order they get now--that just prevents it from docking, and is arguably much cleaner in that it's visible to the player and doesn't involve any backend processing.

Aesthetically, I don't think the pod should be constantly flying around if it's inactive.

Because GetGuards only retrieves units that have the guard order first in queue, fixing this would require doing a search through all units to see which ones fit the profile of "repair order" -> "assist pod order" (ouch). I didn't see a way around but let me know if you have any ideas.

You have self.GuardCache for pods. How about making units add/remove themselves from the cache upon assisting/stopping assisting a pod and updating the orders through that way?

clyfordv commented 1 month ago

I didn't love the guard cache when I put it in for the pods--maybe it's not a ton of memory, but the principle (replicating a system already being tracked by the engine) doesn't appeal to me (the main reason why I preferred "shoulder pods don't dock while being assisted", though I agree with the aesthetic argument). But I'll cook up these options and somebody with a better idea of the performance situation can make the call.

clyfordv commented 1 month ago

Stop was straightforward (just a check in OnStopBuild)--discerning the stop command from a new movement command might be difficult, which is drawing me in more and more into "interrupt pathfinding locks guards to current task".

For pods, perhaps an assembly fix is possible?

clyfordv commented 1 month ago

Abort navigation now "locks" assisting engineers to their current task. Definitely open to alternatives, but I like this because:

Check it out, I'll work up the other option if you're still interested in it.

Edit: after experimenting some more, UEF SACUs (and only UEF SACUs) can properly assist shoulder pods (both theirs and UEF ACU, UEA0003 and UEA0001), but the UEF ACU can only assist UEA0001.

lL1l1 commented 1 month ago

I think it's best to keep the old assist behavior as it's what people are used to and have built their game skills on (Jip and I have both talked about how we use it frequently). Making abort navigation recheck the assist order avoids collisions just like locking to the old task, and it's even more smooth flowing because you can start a new building and then abort navigation as an automatic action instead of abort navigation and then deal with placing the new building.

Garanas commented 1 month ago

Perhaps we should also rename the 'abort navigation' action to something else.

clyfordv commented 1 month ago

Been thinking about that. Some variant of "force engineer focus" (with a good tooltip) could work. A little vague, but it's the kind of thing you experiment with anyway.

I've def grown less attached to one solution or another as I've worked on this (we'll need the caching infrastructure for pods to work correctly anyway, instead of just half assing it like they are now). I'm arriving at code-finished, I'll make some videos of the different options and put it out for further testing (definitely could be some edge cases lurking) when I do.

I think we can also save a large number of function calls* if we link a callback to OnCommandIssued and send a check assisters ping from the UI--aside from the original repair override functionality what we're responding to is always** a result of player input.

*At the moment we're relying on On[Start|Stop]Build/Reclaim/Repair, which get called quite a bit **I think, mostly, terms and conditions apply. Let me ruminate on this.

clyfordv commented 1 month ago

Assist cache is up! ("code-finished" -> famous last words)

I've identified three configurations that will likely be more clear when I get around to making the videos:

  1. Hard focus: guards will interrupt their task to follow the lead unit in all actions. Aborting navigation will lock them to their current task until it's complete. (it's my understanding that this is the desired behavior for pods)
  2. Soft focus: guards will interrupt their task to assist the lead unit on new reclaim/build/repair orders. Abort navigation acts the same as above (this is the current configuration), with the addition that it can be applied after the lead unit has wandered off (but before another build/reclaim task is started).
  3. Hotkey refocus: guards will stay on task with no regard for what the lead unit is doing (this is the current behavior). Abort navigation (or a similar hotkey*) will interrupt their current task and return focus to the lead unit.

Swapping between the above configurations is a few lines of code.

This is where the collision happens--lead unit wanders off to (for example) build a mex, you want to abort navigation when it comes in range, but* you left some assisting units behind on a previous task that you would prefer they keep doing. In this situation you either do both (abort navigation for the lead, and summon the assisters), or you have to split it onto two hotkeys. To be demonstrated.

clyfordv commented 1 month ago

Videos:

Hard focus, engineers can be told to stay put with abort navigation-- https://github.com/FAForever/fa/assets/7411478/9fc38cf0-ad93-4d94-a92c-08fe61ca0ba7

Soft focus, engineers will only interrupt for new construction/reclaim/repair orders and are likewise kept in place with abort navigation-- https://github.com/FAForever/fa/assets/7411478/00f40b83-1108-4e33-b074-a294bce7006a

Manual focus, engineers behave as they do now, but the abort navigation hotkey refocuses them on the lead unit-- https://github.com/FAForever/fa/assets/7411478/c290c6d5-7af8-4efd-aa42-6b8181de5a3d

clyfordv commented 1 month ago

Alright we're at the finish line.

*The only thing missing here is that units won't start assisting the drone if they're given the assist order mid-task (it relies on OnStartBuild etc. being called by the pod), to do so needs a callback from OnCommandIssued (or OnGuard function called by the engine, which doesn't exist).

Garanas commented 1 month ago

@MrRowey I'm not sure if it already is, but these changes should definitely be on the Game Changes discussion channel on Discord. It's a significant change to the behavior of various units.

clyfordv commented 1 month ago

I'll do a write up