Fix for bugged River City Connection trait

[jarcast2]

I just discovered that the logic behind the River city connection bit for Songhai is partially flawed as per this post by @azum4roll, whereby cities can be connected while not being on the same river system, while cities on the same riverine network may end up not connected.

I propose a definite bug fix that consists in 3 elements:
1) this handy lua algorithm by @whoward69 that'll go somewhere in the VP files with "Import into VFS=true".

River Connections

Lua classes, methods and functions to determine if two plots are connected by a river - see here for the general discussion A River is one or more contiguous RiverSegments. A river has a "known" headwaters and outflow (which may not be the actual headwaters/outflow if tiles have not been...

forums.civfanatics.com

2) those few sql lines to be put somewhere in a new sql file or in an existing one:

UPDATE CustomModOptions    SET Value = 1 WHERE Name = 'EVENTS_TERRAFORMING';
UPDATE CustomModOptions    SET Value = 1 WHERE Name = 'EVENTS_CITY_CONNECTIONS';

ALTER TABLE Traits ADD COLUMN 'RiverCityConnections' BOOLEAN DEFAULT 0;
UPDATE Traits SET RiverCityConnection=1 WHERE Type = 'TRAIT_AMPHIB_WARLORD';

3) The lua file attached below that will be loaded as InGameUIAddin as a normal lua file and that ties to any trait with RiverCityConnections=1 the ability to have real city connections from being in the same river system.

I can quickly create a modmod but I would like to see the fix integrated in the next VP release honestly.

Also I would like to make a request the current river city connection logic to be decoupled from Traits columns RiverTradeRoad, that has a totally different function.
Does this last request need a proposal? It seems more a technical "metacoding" issue rather than a balance one.

 

[azum4roll]

Looks like IGE+ will need to be updated as well if this is added to VP, to call riverManager:rescanMap() every time a plot/terrain/feature/river is updated.

Or better, this is all translated into C++ code and added to the DLL.

 

[N.Core]

Looks like IGE+ will need to be updated as well if this is added to VP, to call riverManager:rescanMap() every time a plot/terrain/feature/river is updated.

I wonder who's gotta be the poor person who would implement this.

 

[jarcast2]

Looks like IGE+ will need to be updated as well if this is added to VP, to call riverManager:rescanMap() every time a plot/terrain/feature/river is updated.

Or better, this is all translated into C++ code and added to the DLL.

There should be no need to update IGE+.
Thanks to EVENTS_TERRAFORMING enabled, in the lua file I added a function that call the rescan whenever a modification regarding rivers is made.

 

[KungCheops]

Please don't add any core game logic using Lua. Lua is for UI only.

Also, how will this new logic interact with roads? Previously roads and rivers could be used interchangeably to create a city connection.

 

[jarcast2]

Sorry, I would have coded it in C++ if I could, but I know only sql and lua.
That means the lua file would have to be translated in C++.

Also, how will this new logic interact with roads? Previously roads and rivers could be used interchangeably to create a city connection.

It's the same here. The code just adds an indirect connection via rivers: if A and B are on the same river then there is a road between A and C, and another between B and D, then A and D, B and C, C and D are all connected.
It's an addition, it should not affect roads in any negative way.

 

[KungCheops]

Sorry, I would have coded it in C++ if I could, but I know only sql and lua.
That means the lua file would have to be translated in C++.


It's the same here. The code just adds an indirect connection via rivers: if A and B are on the same river then there is a road between A and C, and another between B and D, then A and D, B and C, C and D are all connected.
It's an addition, it should not affect roads in any negative way.

In the existing implementation, it is possible to connect cities A and B to C by building one road in the tile marked with an X. How would roads interact with rivers in the new system?

 

[jarcast2]

Ahhh, now I see what you mean.
Whoward's utility produces N tables containing the distinct river bank plots of the N rivers on the map. It doesn't coalesce them as the current method does.
The utility would consider the tile with the X as part of the second river river banks plots so B and C are automatically connected.
On the other hand, A is on another river compared to B and C, so to connect A, a full road either between A and C or between A and B is needed (red line). This is the case because my lua file is simple and detect only if 2 cities are on the same river without checking if there are roads complementing the rivers.
However, in the dll coding one may optimize the path by making roads and river fully complement so that in order to connect A with the other two cities you need one of the two purple roads connecting the 2 river systems.

EIDT: Although the simplest implementation is to make a table number 3 containing the plots from both table 1 (river 1) and table 2 (river 2) so not event the purple roads are needed like in the current method, however the purple roads would be the most realistic way to do it, although a bit more complicated to code.

 

[KungCheops]

Since I've already done quite some work on road building logic, I could see if I'd be able to implement the purple solution in the DLL. (And properly account for it in route pathfinding).

Only allowing direct river connections is a bit too limiting in my opinion.

 

[Rekk]

I do think that the road should have to be in the tile that is adjacent to the river in order to connect to said river.

 

[KungCheops]

I do think that the road should have to be in the tile that is adjacent to the river in order to connect to said river.

That's what he suggested in the image above with the purple road plots.

I don't see how anything else could make sense with the rework.

 

[Rekk]

Is it also planned to link via lakes?

 

[jarcast2]

Since I've already done quite some work on road building logic, I could see if I'd be able to implement the purple solution in the DLL. (And properly account for it in route pathfinding).

Only allowing direct river connections is a bit too limiting in my opinion.

That would be awesome. Thank you for taking your time on such non-trivial task (I would have no idea how to code it honestly).

If you could make edits to the dll, can you disentangle river connection from Traits column RiverTradeRoad and use a newly defined column instead (like 'RiverCityConnections' or whatever)?
There is gwennog's Kyivan Rus' modmod that uses RiverTradeRoad= 1 as originally intended (i.e. "Rivers extend the length of Land Trade routes as if they were roads."), but because Songhai river connections is linked to that column it also ports the "Rivers create City Connections." bit.
If proposal (7-65) passes, however, the link to a specific trait would be eliminated.

 

[KungCheops]

That would be awesome. Thank you for taking your time on such non-trivial task (I would have no idea how to code it honestly).

If you could make edits to the dll, can you disentangle river connection from Traits column RiverTradeRoad and use a newly defined column instead (like 'RiverCityConnections' or whatever)?
There is gwennog's Kyivan Rus' modmod that uses RiverTradeRoad= 1 as originally intended (i.e. "Rivers extend the length of Land Trade routes as if they were roads."), but because Songhai river connections is linked to that column it also ports the "Rivers create City Connections." bit.
If proposal (7-65) passes, however, the link to a specific trait would be eliminated.

Replacing it with a define seems reasonable. Should Askia still get the bonus if the define is off though?

Edit: or did you mean adding a trait that all civs have? I think maybe a mod option would be better.

 

[jarcast2]

If river connections become generic and not anymore unique to Songhai then a custom mod option is better, yes.

 

[hokath]

A mod option would be very cool.

 

[Rekk]

Replacing it with a define seems reasonable. Should Askia still get the bonus if the define is off though?

Edit: or did you mean adding a trait that all civs have? I think maybe a mod option would be better.

I think he means an additional column in the Traits table.

 

[Tekamthi]

its not directly related but I've put a request for some access to the river plots as modder for promos, in case this requires the unbundling of askia ability anyway -- see https://github.com/LoneGazebo/Community-Patch-DLL/issues/10429

 

[azum4roll]

Is it also planned to link via lakes?

Ideally, a lake should (only) be counted as part of the river system if there are both inflow and outflow rivers connected to it. Current implementation seems to be counting all lakes?

 

[Rekk]

I'm fine with allowing connections on two sides of a lake, is a river that flows into a lake. Can't make a lighthouse on it, anyway.