search

Return-To-The-Roots / s25client

Return To The Roots (Settlers II(R) Clone)
http://www.rttr.info
GNU General Public License v2.0
470 stars 75 forks source link

AI player doesn't build fisheries. #1653

Closed desofity closed 3 months ago

desofity commented 3 months ago

It seems that method FindPositionForBuildingAround(BuildingType type, const MapPoint& around) in AIPlayerJH.cpp always returns MapPoint::Invalid when Building::Type is BuildingType::Fishery, so AI never build fishery.

After some debugging and testing I found that code in method AIResourceMap::findBestPosition(const MapPoint& pt, BuildingQuality size, unsigned radius, int minimum) in AIResourceMap.cpp never go past the condition if(!aiMap[idx].reachable || !aiMap[idx].owned || aiMap[idx].farmed) continue; so it always returns initial value: MapPoint best = MapPoint::Invalid().

I haven't found solution yet. May be it is because fish resource (atleast that's what AI debugging window shows in corresponding tab) is always on a water cells, which are of course unreachable (can't build on water). May be resource maps for fish should be like wood or stones, so the nearest buildable point to the water have biggest rating, but not the water itself.

I've watched AI on classic and custom maps with only grass and water - was no difference.

Flamefire commented 3 months ago

After some debugging and testing I found that code in method AIResourceMap::findBestPosition(const MapPoint& pt, BuildingQuality size, unsigned radius, int minimum) in AIResourceMap.cpp never go past the condition if(!aiMap[idx].reachable || !aiMap[idx].owned || aiMap[idx].farmed) continue;

That is completely fine: It tries to find a position for building a fishery not a fish position. See the next line that checks the BQ of that point: https://github.com/Return-To-The-Roots/s25client/blob/6ddcdd5d82437ab54e0f17a6e0913c49621841f8/libs/s25main/ai/aijh/AIResourceMap.cpp#L91

The issue here is that the calculation of the AI ResourceMaps has a serious bug. Specifically the incremental update which is intended to save computation time is wrong and likely the initial computation is too (that part that uses the previously calculated value)
I always wanted to write a good test for that and fix it for good which should make the AI much better as it is currently running with an immense handicap.

See the original issue for more details: https://github.com/Return-To-The-Roots/s25client/issues/933 Closing this as a duplicate

desofity commented 3 months ago

Thanks for the clarification! I'll take a closer look on the original issue.