Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

the unit is "stuck" in the river ford #449

Closed
MrZorG33 opened this issue May 3, 2021 · 34 comments
Closed

the unit is "stuck" in the river ford #449

MrZorG33 opened this issue May 3, 2021 · 34 comments
Assignees

Comments

@MrZorG33
Copy link
Collaborator

MrZorG33 commented May 3, 2021

starting a new game with the latest update of files, I saw that the units can again "get stuck" in the river ford. this has not happened to my units yet. but the Spanish settler has been standing motionless in the river ford for several turns.

юнит застрял в броде01

large river.zip

@Nightinggale
Copy link
Contributor

Most likely not stuck as such, but more like an AI issue. My guess is for optimization purposes the AI only checks the same area when looking for a place to build a new colony. Since it's standing in the river, it checks the river and ends turn because it failed to find a valid location.

@MrZorG33
Copy link
Collaborator Author

MrZorG33 commented May 3, 2021

then, in theory, he should move on, but he stands still.

@raystuttgart raystuttgart self-assigned this May 4, 2021
@raystuttgart
Copy link
Collaborator

Ah guys, this is a settler !
I have tried to adapt settler logic for Large Rivers.
May never have considered though that a Settler actually is ON TOP of a Large River.

Will need to check.

@raystuttgart
Copy link
Collaborator

Edit, could not find a reason in my logic for this to fail.
Area check seems plausible though.

Any help is appreciated.
Will however also analyze the next days.

Until the weekend I will be pretty busy and wasted though ...

@MrZorG33
Copy link
Collaborator Author

MrZorG33 commented May 4, 2021

new data!
the Spaniards have declared war on me, but their troops also do not go beyond the river ford.

image
war.zip

moreover, they can return to their territory. but they cannot enter my territory.

@raystuttgart
Copy link
Collaborator

@Nightinggale
Could you maybe check what is happening with the Military Unit here?
I have no clue how I could fix it.

What I currently assume is that it might be related to "Amphibious Landing" or some other part of the Logic of Combat Units.
(Similar to the logic when Military Units unboard a Ship.)

@Nightinggale
Copy link
Contributor

I can't load the savegame and I can't recreate the setup where a unit is stuck on a river.

@MrZorG33
Copy link
Collaborator Author

What is the problem with loading the saved game?

@Nightinggale
Copy link
Contributor

Crash on load. I'm referring to the soldier one.

@MrZorG33
Copy link
Collaborator Author

most likely this is due to the new changes that have been made to the mod. I already had this problem. unfortunately I've already updated my files and most likely won't be able to load the saved game either.

@Nightinggale
Copy link
Contributor

Ok, I got it to load using an old version. Now there are 3 issues to investigate:

  • why does the AI decide that ACTIVITY_HOLD is the best option on that plot?
  • why can't the savegame be loaded even though (to my knowledge) no savegame breaking changes have been made?
  • checkPower assertion failure (AI can't determine power of players correctly)

@MrZorG33
Copy link
Collaborator Author

Note again: the Spanish troops cannot get out of the river ford into my territory, but they can into their territory.

@Nightinggale Nightinggale reopened this May 16, 2021
Nightinggale added a commit that referenced this issue May 21, 2021
Previous fix didn't work in all cases. This is a fundamentally different approach.
Instead of spot fixing, this alters area() and getArea() in CvUnit and CvPlot.
CvUnit now always assume it's on a land plot next to the river if it's on a large river.
This is true even if the function calling CvUnit::area() and getArea() isn't updated.

Calls to area and getArea in CvPlot needs updating for this to activate.
@Nightinggale
Copy link
Contributor

The fix should work this time.

@raystuttgart
Copy link
Collaborator

Awesome !!!
How did you fix it?

@Nightinggale
Copy link
Contributor

Fix is in the git log 437b652

@raystuttgart
Copy link
Collaborator

raystuttgart commented May 21, 2021

Ah ok, sou you really added the parameter "Domain" to getArea calls !
And then you check also in getArea for Large Rivers

I once thought about it but I was afraid to mess up performance ...
Does it not impact performance?


Well ok, you did more.
(Have not yet fully understood the rest.)

@Nightinggale
Copy link
Contributor

I didn't measure performance, but I don't think it will. If you check the two new functions they are vanilla code except if it's DOMAIN_LAND and TERRAIN_LARGE_RIVERS. That combo is rare enough to not trigger often. Secondly when it triggers, it will loop 9 plots, but it stops at the first land plot, which is usually within the first 3, and not unlikely the first. Performance is good enough, but can be made better with #464. That will however take quite a while to code and it's not urgent.

@MrZorG33
Copy link
Collaborator Author

MrZorG33 commented Nov 5, 2022

while testing the "new hope" assembly, after some time I ran into an old bug (unfortunately I could not find a similar issue. perhaps the conversation was in a working chat) - automated transports "stuck" in the river ford. moreover, for some time they successfully used it, but after a while I noticed their absence (and the lack of goods in warehouses, hehe), I started checking the map and found them all in one place.
I assume that the root of the problem is the same as in the initial issue. so i posted it here.
image

update November 3rd 15:45 (utc+3) (before Ray added new music))
NWGiganticMy.zip

if it fails to open this save, I will update the mod and start the test again.

@MrZorG33 MrZorG33 reopened this Nov 5, 2022
@raystuttgart
Copy link
Collaborator

raystuttgart commented Nov 5, 2022

@Nightinggale
Could you maybe check it at some point?
I have no idea what the issue with "Automated Trade Routes" may be.

@raystuttgart
Copy link
Collaborator

raystuttgart commented Dec 17, 2022

@Nightinggale
I think I found the bug for the "get stuck on River Ford" of Automated Transports !

It is in here:
bool CvSelectionGroupAI::AI_tradeRoutes()

In particular I think it is this section:

if ((pSourceCity != NULL) && ((domainType != DOMAIN_SEA) || (pSourceWaterArea != NULL)))
{
	int iSourceArea = (domainType == DOMAIN_SEA) ? pSourceWaterArea->getID() : pSourceCity->getArea();
	if (domainType == DOMAIN_SEA ? plot()->isAdjacentToArea(iSourceArea) : (iSourceArea == getArea()) ||
		domainType == DOMAIN_LAND && plot()->getTerrainType() == TERRAIN_LARGE_RIVERS && plot()->isAdjacentToArea(pSourceCity->getArea()))
	{
		if ((domainType == DOMAIN_SEA) || (pRoute->getDestinationCity() != kEurope))
		{
			processTradeRoute(pRoute, cityValues, routes, routeValues, yieldsDelivered, yieldsToUnload);
		}
	}
	// TODO: Need to path find if the area check fails like below
}

I think this also needs to have the call with getArea(DOMAIN_LAND).
What do you think ?

@Nightinggale
Copy link
Contributor

After a bunch of debugging it seems that the problem is that all destinations are either full or unreachable. Interestingly not unreachable from the plot the units are on, but rather unreachable when making a path from source colony to destination colony.

Usually transports will then hide in colonies until they have something to do. However that part of the code seems to only find colonies in the same area. The large rivers are different areas and in fact the stagecoach to the west is also stuck because it is on an "island" surrounded by large rivers, hence a different area than the colonies.

In short: the bug is that they park in the wrong location when idle rather than transports failing to do work they are supposed to do. Still not ideal as the vanilla code moves them to colonies for protection.

@MrZorG33
Copy link
Collaborator Author

well, my scout is stuck on an island surrounded by LR. he explored the territory in automatic mode.
image
qwerty.zip

if you look at the map (globe), you will see that he still has a lot of work to do. but, apparently, he was tired and there is a beautiful view))

@Nightinggale
Copy link
Contributor

I have a pretty good idea of what has gone wrong here and a complete fix for the previous problem should fix this too. It's however not trivial to fix so I want to finish other tasks first.

The real issue is that the island is a different area because it's surrounded by water and the scout is looking for places to explore in the same area, hence it searches the island for places to explore or colonies to move to. There aren't any so it's just stuck. The area code still has some flaws regarding large rivers.

@devolution79
Copy link
Member

devolution79 commented Dec 27, 2022

We had a discussion a while ago (sadly lost to the slack history paywall 😢 ) where we brainstormed (myself and @Nightinggale) potential solutions to this and related issues. I cannot recall all the details of our exchange but core issue is that large rivers should not "break apart" land areas. If they are allowed to do so, we end up with a bunch of problems and ad-hoc workarounds that result in all sort of weird bugs. My solution is to modify the land area generator so that land areas containing large rivers stay as a single area.
Update: I've implemented this and currently testing the result

@devolution79
Copy link
Member

devolution79 commented Dec 27, 2022

Just to clarify, large rivers do remain part of the "overall" water area it's just that the land areas on both sides will be in the same area. This should end the unit ai confusion since it will realize that it can simply pathfind over the nearest river ford to continue its traversal of the area. This also means that the military / native AI has been un-nerfed and they can now consider crossing any large river in order to carry out their attack (without this fix they would have to undertake an intercontinental invasion, something the AI is poorly coded to carry out)

@raystuttgart
Copy link
Collaborator

Thanks for giving this a try to fix it. 👍

@Nightinggale had explained me as well that the "Large Rivers not splitting Areas" would be the ideal solution he would also try.
So what you are going to implement or already have implemented sounds just perfect and logical.

@Nightinggale
Copy link
Contributor

Nightinggale commented Dec 27, 2022

We had a discussion a while ago (sadly lost to the slack history paywall 😢 ) where we brainstormed (myself and @Nightinggale) potential solutions to this and related issues.

The idea was to make an array of areas in each plot instead of just one. This way each plot can hold a land and a water area. Cities and large rivers can then merge both types. If there is one area for each domain prior to DOMAIN_IMMOBILE, then adding more domains later will be way easier (should we ever need that).

That's pretty much all I remember from the now lost discussion, but at least it carry the essence of what the plan was/is.

@raystuttgart
Copy link
Collaborator

@devolution79
Yesterday you wrote that you had already implemented a solution to "Large Rivers not splitting Areas".
Are you still testing it or did you decide to do some redesign after the discussion with @Nightinggale ?

@devolution79
Copy link
Member

I need to be able to generate random maps with large rivers. Alternatively, somebody could supply me with test-maps that are known to be problematic!

@devolution79
Copy link
Member

A save would be helpful too, but I think that is problematic since New_Hope is undergoing rapid development and the save may not be compatible (unless we agree on which revision was used)

@raystuttgart
Copy link
Collaborator

raystuttgart commented Dec 28, 2022

@devolution79
Then maybe just ask @Ramstormp to give you the MapScript he had merged / created from your MapScript and the MapScript of FlaviusBelisarius. 👍

Alternatively you could simply use one of the existing Maps to test.
All of the have Large Rivers already and most have already been adjusted to "New Hope".

What is the difference if the Map was created by a MapScript or by a modder manually?
For your tests it should hardly matter if it was randomly generated or made by hand.

devolution79 added a commit that referenced this issue Jan 2, 2023
This means that units will no fail to realize that they can simply cross the large river rather than acting as if they have been stranded on an island
@devolution79
Copy link
Member

I've committed the new land area generator. Please help test this change!

@raystuttgart
Copy link
Collaborator

Awesome !

@MrZorG33
Copy link
Collaborator Author

I tested the fix on the Two Continents map (now 1690). despite the abundance of LRs and river fords, Scouts and automated units move without problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants