Last docking PR for release by PunkPun · Pull Request #21565 · OpenRA/OpenRA

PunkPun · 2024-08-29T18:16:39Z

Fixes #21027
supersedes #21067

Continued progress #20168
Changes taken from #20667
Changes taken from #20665

I've reduced the amount of changes to make the new docking system good enough for release

All changes contained here either have been discussed on github or discord or bug reports were submitted to discord or github

OpenRA.Mods.Common/Activities/GenericDockSequence.cs

OpenRA.Mods.Common/Activities/FindAndDeliverResources.cs

OpenRA.Mods.Common/Traits/LinkClientManager.cs

OpenRA.Mods.Common/Activities/FindAndDeliverResources.cs

OpenRA.Mods.Common/Traits/Buildings/TransformsIntoLinkClientManager.cs

OpenRA.Mods.Common/Traits/LinkClientManager.cs

OpenRA.Mods.Common/Activities/MoveToDock.cs

OpenRA.Mods.Common/Traits/Buildings/TransformsIntoLinkClientManager.cs

pchote · 2024-09-21T20:08:40Z

OpenRA.Mods.Common/Traits/Conditions/GrantConditionOnClientLink.cs


 		[Desc("Host actor type(s) leading to the condition being granted. Leave empty for allowing all hosts by default.")]
-		public readonly HashSet<string> DockHostNames = null;
+		public readonly HashSet<string> LinkHostNames = null;


Shouldn't this be using link types rather than actor names?

Yes, that was one of the issues yet to be solved. Linked and Unlinked notifications don't provide a link type, and adding it is easier said than done

pchote · 2024-09-21T20:09:17Z

OpenRA.Mods.Common/Traits/Conditions/GrantConditionOnHostLink.cs


 		[Desc("Client actor type(s) leading to the condition being granted. Leave empty for allowing all clients by default.")]
-		public readonly HashSet<string> DockClientNames = null;
+		public readonly HashSet<string> LinkClientNames = null;


Likewise here.

pchote · 2024-09-21T20:22:12Z

OpenRA.Mods.Common/TraitsInterfaces.cs

-		WPos DockPosition { get; }
-		int DockWait { get; }
-		WAngle DockAngle { get; }
+		WPos LinkPosition { get; }


These really should move out of the link interface and into the docking implementation.

It's used by FindAndDeliverResources to search resources closest to the refinery, by CarryableHarvester to order carryalls to, and LinkExts to search for the closest Link

In other words, it's exposed for outside traits to interact with the process of moving to the link or finding the closest link

Again, having a single LinkPosition is not correct for all potential types of link host. The interface should be abstracting the actions, not the implementation details (e.g. units should MoveWithinRange of a proximity host and not Move to a specific point).

Sure, but I don't know the best way to do it. This can be iterated on in the future

Isn't the problem then that FindAndDeliverResources depends on Link and not Dock?

OpenRA.Mods.Common/Activities/GenericDockSequence.cs

OpenRA.Mods.Common/Traits/LinkClientManager.cs

pchote · 2024-09-21T20:26:26Z

OpenRA.Mods.Common/Traits/Harvester.cs

+		public override bool CanLinkTo(Actor hostActor, ILinkHost host, bool forceEnter = false, bool ignoreOccupancy = false)
+		{
+			return base.CanLinkTo(hostActor, host, forceEnter, ignoreOccupancy)
+				&& (self.Owner == hostActor.Owner || (ignoreOccupancy && self.Owner.IsAlliedWith(hostActor.Owner)));


Wouldn't it make more sense to require force enter to unload at an allied refinery?

why? Imo you should be able just directly order the harvester. IgnoreOccupancy is used for manual orders

pchote · 2024-09-21T20:50:19Z

The "link" interfaces still expose a lot of docking-specific things. If this really can't be fixed then I think it would be best to abandon the idea of renaming to link and just stick with docking: the abstraction issues remain, but at least it won't have the silly names.

PunkPun · 2024-10-11T21:48:10Z

The "link" interfaces still expose a lot of docking-specific things. If this really can't be fixed then I think it would be best to abandon the idea of renaming to link and just stick with docking: the abstraction issues remain, but at least it won't have the silly names.

idea abandoned.

Incase we decide otherwise this is the commit d082518

pchote

LGTM. The controversial parts have been removed, and this is a clear improvement (focusing on bugfixes) over current bleed. Scope creeps/bugfixes can be handled in followup PRs.

pchote · 2024-10-13T13:36:58Z

https://github.com/OpenRA/OpenRA/wiki/Changelog-(bleed)/_compare/00c7c61bffad6ea921311e575a842a486abc5469

PunkPun added this to the Next Release milestone Aug 29, 2024

Mailaender reviewed Sep 8, 2024

View reviewed changes

OpenRA.Mods.Common/Activities/GenericDockSequence.cs Show resolved Hide resolved

Mailaender reviewed Sep 8, 2024

View reviewed changes

OpenRA.Mods.Common/Activities/FindAndDeliverResources.cs Outdated Show resolved Hide resolved

michaeldgg2 reviewed Sep 8, 2024

View reviewed changes

OpenRA.Mods.Common/Traits/LinkClientManager.cs Outdated Show resolved Hide resolved

PunkPun force-pushed the docking-release branch from 4c69c5e to 441b265 Compare September 9, 2024 19:38

RoosterDragon reviewed Sep 13, 2024

View reviewed changes

OpenRA.Mods.Common/Activities/FindAndDeliverResources.cs Outdated Show resolved Hide resolved

OpenRA.Mods.Common/Traits/Buildings/TransformsIntoLinkClientManager.cs Outdated Show resolved Hide resolved

OpenRA.Mods.Common/Traits/LinkClientManager.cs Outdated Show resolved Hide resolved

PunkPun force-pushed the docking-release branch from 441b265 to 4fd1598 Compare September 14, 2024 10:46

RoosterDragon approved these changes Sep 14, 2024

View reviewed changes

PunkPun added the PR: Needs +2 label Sep 14, 2024

PunkPun force-pushed the docking-release branch from 4fd1598 to fcd74b5 Compare September 14, 2024 21:11

pchote reviewed Sep 21, 2024

View reviewed changes

PunkPun force-pushed the docking-release branch 2 times, most recently from 972b7e0 to 1c6d0b2 Compare September 22, 2024 11:03

abcdefg30 added the PR: Conceptual concerns label Oct 5, 2024

Fix refineries not uncloaking on dock

f38ae21

PunkPun force-pushed the docking-release branch from 1c6d0b2 to 46ae665 Compare October 11, 2024 20:22

PunkPun added 4 commits October 12, 2024 00:04

We should notify host even when it's not in world

8f3510a

Add RequireForceMoveCondition to DockClientManager

e55f35a

Simplify IDockHost interface

91b8af5

Only allow docking to allied refineries if directly ordered

47f3c55

PunkPun force-pushed the docking-release branch from 46ae665 to 055f617 Compare October 11, 2024 21:47

PunkPun and others added 3 commits October 12, 2024 14:36

Fix clients acquiring hosts on order creations

617cb22

Add support for not displaying target lines

747d409

Don't crash if 2 DockHosts overlap

b7c4bff

PunkPun force-pushed the docking-release branch from 055f617 to b7c4bff Compare October 12, 2024 11:37

pchote removed the PR: Conceptual concerns label Oct 13, 2024

pchote approved these changes Oct 13, 2024

View reviewed changes

pchote merged commit 67855f2 into OpenRA:bleed Oct 13, 2024

PunkPun deleted the docking-release branch October 14, 2024 20:17

PunkPun mentioned this pull request Oct 15, 2024

Fix Harvester Automatically Unload On Allied Refinery #21067

Closed

Uh oh!

Conversation

PunkPun commented Aug 29, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PunkPun Sep 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pchote commented Sep 21, 2024

Uh oh!

PunkPun commented Oct 11, 2024

Uh oh!

pchote left a comment

Choose a reason for hiding this comment

Uh oh!

pchote commented Oct 13, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

PunkPun Sep 22, 2024 •

edited

Loading