Allow checking if calling "Capture" would succeed

[azarus]

Currently calling engineer.Capture(target) will cause lua to throw an exception when a building is a state between "selling" or transforming to from an MCV to ConYard.
There is nothing critical about having capture fail. So it should return a boolean letting the user know that the requested succeeded or not and don't throw an exception.

Also it since this is something that could fail, we should expose a functionality to just check if the action is even possible.

[abcdefg30]

This change is not necessary now that you introduced the CanCapture method, which covers the return false case here. We cannot guarantee that the CaptureActor activity succeeds, so the return true case is unreliable.

[azarus]

There might be a use case where one might want to check if the capture would fail. For example searching for an actor that could be captured, and send the actor to that location without actually capturing the target.

Capturing adds an activity to the queue, which be wanted by the user. It's up to the user to decide.

Also since the request might fail it is necessary to return if it succeeded or not. How else would we know?

I vote for letting the user decide whether the return value is used or not. If it doesn't hurt us. it might be helpful to expose the result in both.

[abcdefg30]

Also since the request might fail it is necessary to return if it succeeded or not. How else would we know?

The problem is that the message does not tell the user if it worked or not, just if the activity is queued. Reporting a wrong result is Imho worse in this case, especially if we already have an extra method to cover if capturing is possible.

[azarus]

Also since the request might fail it is necessary to return if it succeeded or not. How else would we know?

The problem is that the message does not tell the user if it worked or not, just if the activity is queued. Reporting a wrong result is Imho worse in this case, especially if we already have an extra method to cover if capturing is possible.

How is it a good practice to have a method being called that potentially might do nothing? The user has no way of knowing whether the action requested was queued. Hence the return value. Where is it reporting a wrong result?

As i explained I do propose the have an extra method for other use cases.
User might want to query if an actor is capturable and send 10 rifle infantry there, and 60 seconds later if the actor is still capturable then send the engineer. Such thing would require additional calls to implement if we're unable to check the ability.

It is a good practice in general to allow checking if an action would succeed before you make the call. Even better practice to return if the action requested was done or not.

Anyways, I'm tired of arguing about what's standard what's not standard. What's bad practice what's not a bad practice. I've closed the PR. You're absolutely free not to accept it, and do it by yourself then. Thanks for your time. Good luck.

[abcdefg30]

1. When the return value is correct, it is redundant to the return value of the new method.
2. This might return true even if no capturing ever happens. The method just queues a capture activity which can fail itself:

   OpenRA/OpenRA.Mods.Common/Activities/CaptureActor.cs

   Lines 52 to 58 in c529137

   	// Make sure we can still capture the target before entering
   	// (but not before, because this may stop the actor in the middle of nowhere)
   	if (enterCaptureManager == null || !enterCaptureManager.CanBeTargetedBy(enterActor, self, manager))
   	{
   	Cancel(self, true);
   	return false;
   	}

   
   (Note that although this is the same conditional, it runs later when the capturing actor approaches the target, and the target might have changed between queueing and running of the activity.)

[PunkPun]

Eluant namespace is no longer used

You'll also need to squish the commits

[azarus]

@PunkPun

Eluant namespace is no longer used

This Eluant namespace is not in the scope of the issue here.

You'll also need to squish the commits

Maintainer can do that when they decide to merge you select "squash and merge" from the drop down.

[PunkPun]

This Eluant namespace is not in the scope of the issue here.

Yes it is, you are removing LuaException

[abcdefg30]

Maintainer can do that when they decide to merge you select "squash and merge" from the drop down.

This option is disabled, we only do rebasing as using merge commits is bad practice.