Vinifera-Developers / Vinifera

Vinifera is a C&C: Tiberian Sun engine extension implementing new logics and fixing bugs.
GNU General Public License v3.0
44 stars 10 forks source link

[Enhancement] Smarter harvester refinery-seeking algorithm #201

Open Rampastring opened 3 years ago

Rampastring commented 3 years ago

Description:

The vanilla TS harvester refinery-seeking algorithm is very dumb. It finds the closest free refinery, but will never consider occupied refineries. If you are playing on a 200x200 map and have a refinery on two corners of the map, your harvester will rather traverse through hundreds of cells for the free refinery than queue for the occupied refinery (on which it could get to unload much faster).

Possible Implementation:

The harvester's refinery-seeking algorithm should consider both free refineries and occupied refineries when figuring out which refinery to unload at. If the closest occipied refinery is much closer to the harvester than the closest free refinery, the harvester should queue for the occipied refinery instead of going for the free refinery.

Preferably the difference in distance for preferring free refineries should be configurable through some Rules.ini key.

Additional Files:

CnCNet ts-patches implementation (smarter_harvesters.asm line 26 at the time of writing this): https://github.com/CnCNet/ts-patches/blob/master/src/mods/smarter_harvesters.asm#L26

Red Alert 1 implementation (in my More QoL Improvements mod for RA Remastered): https://github.com/Rampastring/CnC_Remastered_Collection/blob/f48a193c9762733d7d0d8a0229e2d0d8767e0cb4/REDALERT/UNIT.CPP#L2913

The CnCNet implementation also fixes a bug in Westwood's original algorithm, where harvesters discriminated against Dock= buildings specified last on the list. The game would loop through the Dock buildings and select the first free refinery found, meaning that if we had Dock=GAREFN,NAREFN for a harvester and there was a free GAREFN 200 cells away from the harvester and a free NAREFN 10 cells away from the harvester, the harvester preferred the GAREFN.

CCHyper commented 3 years ago

Just to open the discussion, is there any issues with this change of behaviour? One could argue that it's a good candidate for QoL improvement, but it must be almost bug-free...

Metadorius commented 3 years ago

AFAIK this is included in DTA public release and I've yet to see any complaints.

Crimsonum commented 3 years ago

I've noticed individual Harvesters sometimes stopping from doing anything in TI after implementing this patch, but I can't exactly rule out other potential factors...

Rampastring commented 3 years ago

This has been in DTA since last summer and I haven't noticed any issues.

Together with some other harvester algorithm improvements that I'm about to post, you can see the impact here: https://www.youtube.com/watch?v=7G_IQiQ346Q

Metadorius commented 3 years ago

I've noticed Harvesters sometimes stopping from doing anything in TI after implementing this patch, but I can't exactly rule out other potential factors...

Are you sure that's not a vanilla game bug? There's a bug in YR when there's a lot of miners and a few refs and some of the harvesters just stop altogether.

Crimsonum commented 3 years ago

Are you sure that's not a vanilla game bug? There's a bug in YR when there's a lot of miners and a few refs and some of the harvesters just stop altogether.

I'm certain it didn't have this issue before, but it could have a similar cause in the way harvs queue for a single refinery now. Luckily it doesn't happen too often. And the AI seems immune to it.

Metadorius commented 3 years ago

Well, we can "cure" that by porting a harvester counter and a "Next Idle Harvester" hotkey. /s

Rampastring commented 3 years ago

I updated the description, realized that the CnCNet implementation also fixes an issue where harvesters discriminate against Dock= buildings specified later on the list.

CCHyper commented 3 years ago

I have observed that some of the competitive Tiberian Sun players prefer the original harvester behaviour. Based on this, I'm happy for this to stay as a QoL upgrade, and also introduce a Session value to disable this behaviour, allowing players to turn it off if they choose so.