Idle Workboats (+Duplicate Fishing Boat Entries)

[stackpoint]

In my games, I've noticed that the AI (city-states mostly) usually had 1-2 idle workboats around their cities. I believe this is because of the CreateWorkboats function in CET_Events.lua and the fact that the cultural borders of cities (city-states in particular) either never or take a while to reach out to a sea resource three tiles away.

I would suggest reducing the check/free workboat radius to just 2 tiles away to reduce the number of idle workboats:

                    for plot in Plot_GetPlotsInCircle(city:Plot(), 1, 2) do

[GrantSP]

I have noticed this phenomenon too, although I would add it is more a case of the check for workable sites produces new boats even after the site is worked. Many times a CS may have workable sea tiles that are already improved yet there are still work boats anchored over those tiles and others cruising around the boundaries. Sometimes the CS use their work boats as explorers.

[Thalassicus]

We could definitely check to see if the plot in question already has fishing boats. Reducing the range to 2 tiles also makes sense.

[stackpoint]

Correct me if I'm wrong, but the way you wrote the code seems to exclude the possibility of workboats already being on the improvement when they're spawned.

I've tested two games with this change earlier this week and there seems to be a reduction in idle workboats so I committed the range change into the master branch. I'll close this issue for now.

[stackpoint]

I noticed in the logs of my latest run that the workboat check was being run twice for each civ:

[34935.597] MT_Initialize: INFO   Turn 75  Checking for workboats Isabella TECH_SAILING IMPROVEMENT_FISHING_BOATS
[34935.597] MT_Initialize: INFO   Turn 75    Madrid spawned for RESOURCE_PEARLS
[34935.597] MT_Initialize: INFO   Turn 75    Madrid spawned for RESOURCE_FISH
[34935.612] MT_Initialize: INFO   Turn 75  Checking for workboats Isabella TECH_SAILING IMPROVEMENT_FISHING_BOATS
[34935.628] MT_Initialize: INFO   Turn 75    Madrid spawned for RESOURCE_PEARLS
[34935.628] MT_Initialize: INFO   Turn 75    Madrid spawned for RESOURCE_FISH

When I checked the sailing technology, it listed the fishing boat improvement twice. This is apparently a bug related to the samurai's ability to create fishing boats: http://forums.civfanatics.com/showthread.php?p=12823178

Furthermore, this was apparently fixed in the latest patch: "UI: There were two fishing actions visible on the Sailing tech. This is now corrected." Though it seems to remain a problem in CEP.

Since the code to spawn workboats uses the improvement itself as a reference, this could be the reason why the code is run twice. I'll try to look up more information about this tomorrow when I have more time. Though, I still stand by my suggestion to reduce the improvement range check to 2.

[GrantSP]

I've been trying to find that duplicated fishing boats code for days now. Where did you see it. After reading the patch release notes about the fix, and playing a vanilla game to confirm it, I just couldn't see where the fault lay. Your log does clearly show it happens. I also concur with your reduction in plot range check.

[stackpoint]

I actually didn't find any code regarding the duplication yet. I just saw the duplication in the in game tech tree.

[GrantSP]

Ahh... my mistake. I'll keep looking. Trying to understand why we also allow fishing boats on ice! I don't believe there are any resources on ice.

[stackpoint]

Here's the code in the Builds table in CIV5Builds_Expansion2.xml:

        <Row>
            <Type>BUILD_FISHING_BOATS_NO_KILL</Type>
            <PrereqTech>TECH_SAILING</PrereqTech>
            <ImprovementType>IMPROVEMENT_FISHING_BOATS</ImprovementType>
            <Description>TXT_KEY_BUILD_FISHING_BOATS</Description>
            <Water>true</Water>
            <EntityEvent>ENTITY_EVENT_BUILD</EntityEvent>
            <HotKey>KB_F</HotKey>
            <OrderPriority>98</OrderPriority>
            <HotKeyPriority>1</HotKeyPriority>
            <IconIndex>35</IconIndex>
            <IconAtlas>UNIT_ACTION_ATLAS</IconAtlas>
      <ShowInPedia>false</ShowInPedia>
      <ShowInTechTree>false</ShowInTechTree>
        </Row>
    </Builds>

There must be some code in CEP that conflicts with this somehow.

[stackpoint]

I took a look at the database and it has both ShowInPedia and ShowInTechTree as 0 for this improvement so it may be that the modified TechTree and Civilopedia is ignoring the properties (at least for improvements).

I also added a check in the CreateWorkboats function whether the improvement is visible in the tech tree and it worked in my tests (line 85):

    for buildInfo in GameInfo.Builds(string.format("ImprovementType IS NOT NULL AND ShowInTechTree AND PrereqTech = '%s'", techType)) do

So that works out the problems with the CreateWorkboats function. Just need to fix the duplicate fishing boat entries in the Civilopedia and TechTree.

[GrantSP]

Okay, when I said I couldn't find the code, I meant the duplicate stuff in our code. Never mind.

I believe I found the culprit. Maybe it is the same thing you found.

The modified CivilopediaScreen.lua at line 2403 is: for thisBuildInfo in GameInfo.Builds( prereqCondition ) do

the vanilla version has a ShowInPedia check like this: for thisBuildInfo in GameInfo.Builds{PrereqTech = techType, ShowInPedia = 1} do

If we simply change that line it should be fixed. I suspect that that was the patch change Firaxis made when they realized the duplication, we were still basing our code from the pre-patched file.

On 8 November 2013 08:57, stackpoint notifications@github.com wrote:

I took a look at the database and it has both ShowInPedia and ShowInTechTree as 0 so it may be that the modified TechTree and Civilopedia is ignoring the property (at least for improvements).

I also added a check whether the improvement is visible in the tech tree and it worked in my tests:

    for buildInfo in GameInfo.Builds(string.format("ImprovementType IS NOT NULL AND ShowInTechTree AND PrereqTech = '%s'", techType)) do

— Reply to this email directly or view it on GitHubhttps://github.com/Thalassicus/cepbasic/issues/6#issuecomment-28010635 .

"There are 10 types of people in the world: those who understand binary, and those who don't."

[stackpoint]

That's probably it for the Civilopedia duplicate. I checked the TechButtonInclude.lua file and it has a similar missing check (replace CEP line 141):

    for thisBuildInfo in GameInfo.Builds{PrereqTech = techType, ShowInTechTree  = 1} do

I ran a comparison check a couple weeks back after the patch but I couldn't tell where the CEP code began and the new Firaxis code ended.

[GrantSP]

So I guess that means both those files are based off the pre-patch versions and are missing the necessary checks. What do say, good catch all round or not? I can't do any testing at the moment, I have some merge conflicts on gitHub to contend with before I start to use all this code again.

On 8 November 2013 09:47, stackpoint notifications@github.com wrote:

That's probably it for the Civilopedia duplicate. I checked the TechButtonInclude.lua file and it has a similar missing check:

for thisBuildInfo in GameInfo.Builds{PrereqTech = techType, ShowInTechTree  = 1} do

I ran a comparison check a couple weeks back after the patch but I couldn't tell where the CEP code began and the new Firaxis code ended.

— Reply to this email directly or view it on GitHubhttps://github.com/Thalassicus/cepbasic/issues/6#issuecomment-28014702 .

"There are 10 types of people in the world: those who understand binary, and those who don't."

[stackpoint]

I tested both fixes and there were no duplicates in the tech tree or pedia. Looks good, so I'll probably commit the changes after you finish merging your branch.

[GrantSP]

It appears like we are good to go. A few small hiccups, but I believe we are all on the same code base now. Most of my commits are rather small and don't usually involve a lot of different files, but if you notice anything weird, look to my edits first.

I'm glad that fishing boats is resolved, small glitches like that really put a dampener on the whole experience. Well done.

On 8 November 2013 10:10, stackpoint notifications@github.com wrote:

I tested both fixes and there were no duplicates in the tech tree or pedia. Looks good so I'll probably commit the changes after you finish merging your branch.

— Reply to this email directly or view it on GitHubhttps://github.com/Thalassicus/cepbasic/issues/6#issuecomment-28016404 .

"There are 10 types of people in the world: those who understand binary, and those who don't."

[stackpoint]

Commit notes: CAT - Fixed duplicate fishing boat entries in Civilopedia and Tech Tree with their relevant checks CEG - Added a ShowInTechTree check before spawning workboats

Closing issue for now.