Phobos icon indicating copy to clipboard operation
Phobos copied to clipboard

[New Feature] Autoload Command Shortcut

Open psi-cmd opened this issue 1 year ago • 38 comments

For selected units, pair the transport with infantry/vehicle.

  1. If vehicle loadable transports selected, it won't be loaded if there are vehicles that can only be paired with infantry within selection.
  2. It will always try to load the unit type with least amount of units first, eg. 2 GI and 3 GGI, it will match 2 GI to transport first.
  3. For transport with multiple seats, it will try to fill those seats "diversely".

psi-cmd avatar Dec 14 '24 08:12 psi-cmd

(Falsely include irrelevent file that belongs to another feature)

psi-cmd avatar Dec 14 '24 09:12 psi-cmd

  1. Is it possible we can have a default shortcut key for the autoload command?
  2. This doesn't seem to support garrisonable buildings and bio reactors. However if we add such support to it, we have to tell apart grinders from bio reactors, and we have to distinguish InfantryAbsorb and UnitAbsorb.
  3. ~~This doesn't seem to support Ares features Passengers.Allowed=, Passengers.Disallowed=, Passengers.BySize=, and NoManualEnter= as well as it will still try to assign size 6 vehicles into a transport with size limit 3.~~
  4. ~~Terror drones may not be auto-loaded into Flak Tracks this way. Maybe treat smaller units like they are infantries?~~
  5. ~~It doesn't check if the passengers are mind controlled or are mind controlling something, in which case it is not allowed to enter a transport.~~

I solved 3, 4, and 5. See below for details.

Aephiex avatar Dec 14 '24 09:12 Aephiex

When I select an amphibious transport on water and units on land at a same time and press the auto load button, the amphibious transport will move ashore all the way to the land unit and stop there. The land unit doesn't get into the transport. However, this is exactly the same behavior when I call a land unit to get into an amphibious transport on water, so this isn't really an issue.

The implementation is mindful. It directly gives enter order to selected units so everything will behave the same as if the player explicitly gave an enter order. It is very convenient to use, even I can now enjoy playing the Allies.

Aephiex avatar Dec 14 '24 10:12 Aephiex

Nightly build for this pull request:

This comment is automatic and is meant to allow guests to get latest nightly builds for this pull request without registering. It is updated on every successful build.

github-actions[bot] avatar Dec 14 '24 12:12 github-actions[bot]

Hello, made Ares tag support for you, and more.

https://github.com/psi-cmd/Phobos/pull/1

Effects:

  1. Transports with NoManualEnter=true will be viewed as if they didn't have passenger slots;
  2. If a transport has Passengers.BySize=false then every potential passenger is viewed as size 1;
  3. If a transport has Passengers.Allowed= and Passengers.Disallowed= defined then passengers unable to enter it will not try to get into it, as well as the auto assignment will never try to violent the SizeLimit=;
  4. Vehicle types with size <= 2 and passenger <= 0 are viewed as standard passengers like infantry types, and infantry types with size >= 3 are viewed as larger passengers like tanks;
  5. If a standard transport can't actually load anything due to its passenger filters, then just try to load it into larger transports;
  6. Mind controlled units and units mind controlling something can't be passengers at all.

Maybe we have to detect if Ares is present before we take compatibility with Ares tags?

Aephiex avatar Dec 14 '24 12:12 Aephiex

have you test in multiplayer game?

NetsuNegi avatar Dec 14 '24 14:12 NetsuNegi

Hello, made Ares tag support for you, and more.

psi-cmd#1

Effects:

  1. Transports with NoManualEnter=true will be viewed as if they didn't have passenger slots;
  2. If a transport has Passengers.BySize=false then every potential passenger is viewed as size 1;
  3. If a transport has Passengers.Allowed= and Passengers.Disallowed= defined then passengers unable to enter it will not try to get into it, as well as the auto assignment will never try to violent the SizeLimit=;
  4. Vehicle types with size <= 2 and passenger <= 0 are viewed as standard passengers like infantry types, and infantry types with size >= 3 are viewed as larger passengers like tanks;
  5. If a standard transport can't actually load anything due to its passenger filters, then just try to load it into larger transports;
  6. Mind controlled units and units mind controlling something can't be passengers at all.

Maybe we have to detect if Ares is present before we take compatibility with Ares tags?

Wow, thanks for your work. I am not famillar with other features on transport loading and you help with the corner cases. And also thanks for a clear documentation that explain things into details. 👍

For ares tags, maybe we could have the "original" default value for them. For example, NoManualEnter=False, Passengers.BySize=True are the default behavior without any MOD.

psi-cmd avatar Dec 14 '24 18:12 psi-cmd

have you test in multiplayer game?

Not yet, trying to find someone for help.

psi-cmd avatar Dec 14 '24 20:12 psi-cmd

For ares tags, maybe we could have the "original" default value for them. For example, NoManualEnter=False, Passengers.BySize=True are the default behavior without any MOD.

Yes, I defaulted every vehicle to allow maual enter and to count passengers by size.

Aephiex avatar Dec 15 '24 04:12 Aephiex

https://github.com/psi-cmd/Phobos/pull/2

I forgot to consider the passenger slot budget in my first pull request. So I'm making another pull request to fix it.

Previously: If you select an amphibious transport with 2 passenger slots remaining and a vehicle that is size 3, and press autoload command, it will try to send the vehicle into the amphibious transport.

Now: It will not try to send the vehicle into the amphibious transport.

Aephiex avatar Dec 15 '24 04:12 Aephiex

(Better to discuss here) Oh, you repaired that faster than me.

I think originally there will be a strange corner case. If you have a "size=3" passenger and "1 remainsize=2" transport, 1 remainsize=3"​ transport. Unfortunately we select the small remain transport first, and we crossout that only passenger.

One possible solution is to do the sort according to size. However size is not the only restriction, we may have other conditions to decide weather we should load that passenger. As I said in chat, it seems to be a task assignment problem if we want our solution to be cleaver enough.

psi-cmd avatar Dec 15 '24 05:12 psi-cmd

@Metadorius

Is there a specific reason that this is broken down into two halves exactly at size 2/size 3?

I'm leaving a reply here, because I somehow can't leave a reply into a file.

Why passengers and transports are split by size? In actual play you may want to load your Conscripts into Flak Tracks first, then load the Flak Tracks and any remaining infantry into amphibious transports, to make the most of passenger slots. It's a hard-coded way but it helps for most practical cases. A mod can break it by the introduction of the 4th layer of transport, that even amphibious transports can be loaded, then the logic will not be able to handle that thing, but such a giga transport doesn't really make sense.

To avoid making it too complicated to do, we make a several assumptions:

  1. A transport for standard passengers (size <=2) has size >= 3 itself, and if it somehow has size <= 2, it is treated as though it were larger and never try to load it into other standard transports;
  2. A transport that can handle size >= 3 is an amphibious transport and never try to load it into anything;
  3. The player wants to load standard passengers into standard vehicles first to make the most of passenger slots.

Aephiex avatar Dec 15 '24 05:12 Aephiex

~~Maybe we can consider adding support for bio reactors, too. To avoid complexity, we make a several assumptions:~~ ~~1. A Bio Reactor is a building with Passengers=<greater_than_zero> and InfantryAbsorb=yes;~~ ~~2. A Bio Reactor is treated as though it had Passengers.BySize=no;~~ ~~3. Only infantry units can be loaded into a Bio Reactor and they can do so even if they are mind-controlled (not if they are mind-controlling something);~~ ~~4. Consider sending selected infantry into Bio Reactors only if currently selected passengers and vehicles can't pair.~~

~~And for the Tank Bunker support:~~ ~~1. A Tank Bunker is a building with Bunker=yes and is not yet docked;~~ ~~2. A Tank Bunker is treated as though it had Passengers=1, SizeLimit=<infinite>, and Passengers.BySize=no;~~ ~~3. Only vehicles with Turret=yes and Bunkerable=yes can be loaded into a Tank Bunker and they can do so even if they are mind-controlled or mind-controlling something;~~ ~~4. Consider sending selected vehicles into Tank Bunkers only if currently selected passengers and vehicles can't pair.~~

Never mind, already implemented Bio Reactor and Tank Bunker support in my latest pull request.

Regarding garrisonable structures, we can't select our own troops and a neutral structure at a same time, meaning we have to read the user's intention when a garrisonable structure is nearby. However, we can't tell the user's intention when the user selects 10 Conscripts and 2 Flak Tracks just close to a garrisonable structure and presses autoload command. Plus, we can't decide for the user how far is "nearby". So I suggest to not add garrisonable structure support here.

Aephiex avatar Dec 15 '24 06:12 Aephiex

I did not test it in game, but as far as I understand, it will cause desync. Maybe you should use ClickedMission.

TaranDahl avatar Dec 15 '24 06:12 TaranDahl

https://github.com/user-attachments/assets/3e96e258-4570-489e-a902-a650f7d83cbc

Added Bio Reactor and Tank Bunker support. Don't worry, your troops will not be sent into grinders!

The super weapon side bar is made by user NetsuNegi.

https://github.com/psi-cmd/Phobos/pull/3

Aephiex avatar Dec 15 '24 07:12 Aephiex

That is good, after this feature is complete. I will make another pull request that implement the new T selection. At that time besides select screen/all units with same passengers, select screen/all Tank Bunkers or Bio Reactors may also be implemented.

psi-cmd avatar Dec 15 '24 07:12 psi-cmd

I did not test it in game, but as far as I understand, it will cause desync. Maybe you should use ClickedMission.

Just tried that in my local build, and my Conscript attacks the Flak Track instead of getting into it. Maybe we should investigate.

Aephiex avatar Dec 15 '24 07:12 Aephiex

That is good, after this feature is complete. I will make another pull request that implement the new T selection. At that time besides select screen/all units with same passengers, select screen/all Tank Bunkers or Bio Reactors may also be implemented.

That's a great quality of life, I'm looking forward.

Aephiex avatar Dec 15 '24 09:12 Aephiex

@Metadorius

Is there a specific reason that this is broken down into two halves exactly at size 2/size 3?

Why passengers and transports are split by size? In actual play you may want to load your Conscripts into Flak Tracks first, then load the Flak Tracks and any remaining infantry into amphibious transports, to make the most of passenger slots. It's a hard-coded way but it helps for most practical cases. A mod can break it by the introduction of the 4th layer of transport, that even amphibious transports can be loaded, then the logic will not be able to handle that thing, but such a giga transport doesn't really make sense.

Well, the greatest algorithms are the most robust. Imagine if a mod makes everything bigger for more precise control, and normal infantry will take 3 slots, then this algorithm fails.

Or imagine a campaign mega unit that is such a transport. A player clicks our new hotkey, expecting it to work as intended, and it doesn't work properly, causing frustration for the player.

I think it is necessary to not make such assumptions about the size here, and I am not sure if this split is necessary, can't you do a sorted vector instead?

Metadorius avatar Dec 15 '24 11:12 Metadorius

Well, the greatest algorithms are the most robust. Imagine if a mod makes everything bigger for more precise control, and normal infantry will take 3 slots, then this algorithm fails.

Or imagine a campaign mega unit that is such a transport. A player clicks our new hotkey, expecting it to work as intended, and it doesn't work properly, causing frustration for the player.

I think it is necessary to not make such assumptions about the size here, and I am not sure if this split is necessary, can't you do a sorted vector instead?

I have a pseudo code in mind.

Map<int, List<Unit>> passengerMap; // unit size is the key
Map<int, List<Unit>> transportMap; // size limit is the key
Map<int, List<Unit>> ambiguousMap; // unit size is the key

for (Unit a : selectedUnits) {
	if (a is an infantry, or is a vehicle that is either Passengers <= 0, no manual entry, or is fully loaded) {
		passengerMap[a.size].add(a);
	}
}

for (Unit a : selectedUnits) {
	if (a is a vehicle with Passengers >= 1, manual entry allowed, and is not already fully loaded) {
		if (passengerMap.isNotEmpty() && a.canLoadAnyAmong(passengerMap)) {
			transportMap[a.sizeLimit].add(a);
		} else {
			ambiguousMap[a.size].add(a);
		}
	}
}

if (ambiguousMap is not empty) {
	List<Unit> passengersNew;

	for (Entry<int, List<Unit>> entry : ambiguousMap) {
		for (Unit b : entry.getValue()) {
			if (passengersNew.isEmpty() || !b.canLoadAnyAmong(passengersNew)) {
				passengerMap[b.size].add(b);
				passengersNew.add(b);
			} else {
				transportMap[b.sizeLimit].add(b);
			}
		}
	}

	// At this point we have no ambiguousity and every unit has its position as a passenger or transport.
}
  1. Units that are clearly not transports are viewed as passengers;
  2. Units that are transports are viewed as transports, however if they have nothing to load then they are put into ambiguousity;
  3. Units in ambiguousity are added to the passenger list from the smallest to the largest, and if a unit in ambiguousity find itself able to load any previous one added to the passenger list from ambiguousity, it goes to the transports list instead;
  4. At this point every unit is supposed to have been added to either the passengers or the transports, so we just search from the smallest passenger to the largest, from the smallest transport size limit to the largest;
  5. If no loading can happen in the last step, go find Bio Reactors and Tank Bunkers.

This program should be able to handle such a case you mentioned, and should be able to support any layers of transports and any mixed selections of them.

Aephiex avatar Dec 15 '24 15:12 Aephiex

Couldn't help to work on it, so I just work on it. :-D

https://github.com/psi-cmd/Phobos/pull/3

Local tests passed, it is now working based on the pseudo code posted above.

@psi-mod Maybe you will take a look?

Aephiex avatar Dec 15 '24 19:12 Aephiex

Maybe we should load from larger units first.

Imagine we have 2 amphious transports (Passengers=12), one MCV (Size=6), and 14 Conscripts, in this case it makes more sense if the MCV is loaded first, or the conscripts will take 7 each from the amphibious transports leaving no space for the MCV.

EDIT: Done! See the pull request, it is now larger passengers get loaded first.

Aephiex avatar Dec 16 '24 12:12 Aephiex

I did not test it in game, but as far as I understand, it will cause desync. Maybe you should use ClickedMission.

Just tried the following implementation.

auto crd = pTransport->GetCoords();
auto crdNear = MapClass::GetRandomCoordsNear(crd, 1, false);
pPassenger->ClickedMission(Mission::Enter, pTransport, MapClass::Instance->GetCellAt(crd), MapClass::Instance->GetCellAt(crdNear));

And it still desynced. Maybe I shouldn't use the GetRandomCoordsNear?

probably need some kind of NetEvent instead , so all clent can sync the command .

Otamaa avatar Dec 16 '24 14:12 Otamaa

Okay, let's summarize the status of this pull request.

The features of auto load command:

  1. Can handle any layers of cascading transports and any mixed selection of them;
  2. Can handle Ares's passenger whitelist and blacklist feature;
  3. Passengers are loaded from larger to smaller;
  4. Transports are used from smaller size limit to larger size limit;
  5. If no auto loading can happen between selected units, then try to load selected units into selected Bio Reactors and Tank Bunkers.

The only thing we left behind is garrisonable structures, but we can't select our own troops and a neutral garrisonable structure at a same time, meaning we need other approaches to do it.

Known issues that must be solved:

  1. pPassenger->QueueMission(Mission::Enter, false); causes multiplayer desync;
  2. pPassenger->ClickedMission(Mission::Enter, pTransport, ... makes passengers attack the transport as well as it causes multiplayer desync.

We must find a way to give directives to passengers without causing multiplayer desync.

Aephiex avatar Dec 16 '24 15:12 Aephiex

Okay, let's summarize the status of this pull request.

The features of auto load command:

  1. Can handle any layers of cascading transports and any mixed selection of them;
  2. Can handle Ares's passenger whitelist and blacklist feature;
  3. Passengers are loaded from larger to smaller;
  4. Transports are used from smaller size limit to larger size limit;
  5. If no auto loading can happen between selected units, then try to load selected units into selected Bio Reactors and Tank Bunkers.

The only thing we left behind is garrisonable structures, but we can't select our own troops and a neutral garrisonable structure at a same time, meaning we need other approaches to do it.

Known issues that must be solved:

  1. pPassenger->QueueMission(Mission::Enter, false); causes multiplayer desync;
  2. pPassenger->ClickedMission(Mission::Enter, pTransport, ... makes passengers attack the transport as well as it causes multiplayer desync.

We must find a way to give directives to passengers without causing multiplayer desync.

You may try using ObjectClickedAction, just like https://github.com/Phobos-developers/Phobos/pull/1453 did. By the way, as far as I understand, is what this pr wants to do similar to pr1453?

TaranDahl avatar Dec 16 '24 16:12 TaranDahl

You may try using ObjectClickedAction, just like https://github.com/Phobos-developers/Phobos/pull/1453 did. By the way, as far as I understand, is what this pr wants to do similar to pr1453?

Thank you, ObjectClickedAction worked. It doesn't cause multiplayer desync.

The said pull request is more generalist that it can handle many stuff, more than just loading units into vehicles, but it isn't quite clear what are "similar" to the initial target, and I don't know how it is going to handle passenger whitelists and blacklists.

Aephiex avatar Dec 16 '24 16:12 Aephiex

@psi-cmd Sorry to disturb you again. Multiplayer desync is solved.

https://github.com/psi-cmd/Phobos/pull/4

I didn't find the function myself, but someone else did and is already using it, clearly far before I get to know such a function existed. My feeling like a dumb student, was only told the correct answer but have no idea how to get it.

Aephiex avatar Dec 16 '24 16:12 Aephiex

I was doing final project these days and sorry for delayed cache up. Nevermind, there are really some people knows too much about how everything should works. Just not be depressed about that, what you have done is faster and more enthusiatic than I think.

psi-cmd avatar Dec 16 '24 17:12 psi-cmd

You may try using ObjectClickedAction, just like https://github.com/Phobos-developers/Phobos/pull/1453 did. By the way, as far as I understand, is what this pr wants to do similar to pr1453?

https://github.com/CrimRecya/Phobos/blob/325989ff2df243b632da2275d7eef337e8db4eec/src/Commands/DistributionMode.cpp#L159-L168

The pr 1453 implement a more general batch operation by intercept the target selection. Our PR spend a lot of code on equally and recursive matching of selected units. So there are differences.

psi-cmd avatar Dec 16 '24 17:12 psi-cmd

@psi-cmd Sorry to disturb you again. Multiplayer desync is solved.

psi-cmd#4

I didn't find the function myself, but someone else did and is already using it, clearly far before I get to know such a function existed. My feeling like a dumb student, was only told the correct answer but have no idea how to get it.

Oh we really need investigate, the troops are attacking transports now.

Edit: the reason for attacking is because we set our shortcut to ctrl+sth, so the virtual click leads to force attack.

So if we change pPassenger->ObjectClickedAction(Action::Enter, pTransport, false); to true, and bind hotkey to ctrl+sth, then everything works. Because that parameter is ignore force.

psi-cmd avatar Dec 16 '24 18:12 psi-cmd