1. We have added a Gift Upgrades feature that allows you to gift an account upgrade to another member, just in time for the holiday season. You can see the gift option when going to the Account Upgrades screen, or on any user profile screen.
    Dismiss Notice

Cannot Build Fishing Boats

Discussion in 'Bugs and Crashes' started by miccal2000, Jul 1, 2019.

  1. Anq

    Anq Prince

    Joined:
    Apr 14, 2019
    Messages:
    351
    Gender:
    Male
    Location:
    Anser (geese) HQ
    Since build types available to the great farmer are defined in the xml, and all sorts of worker units also define build types in xml, why would there be a need to double-check granular callback from python? Isn't this redundant?
     
  2. Anq

    Anq Prince

    Joined:
    Apr 14, 2019
    Messages:
    351
    Gender:
    Male
    Location:
    Anser (geese) HQ
    Is is possible to use <PlaceBonusTypes> tag in the animalplacing buildinfo xml?
    Instead of <ImprovementType>, let great farmers place bare bonus without improvement using <PlaceBonusType>.
    Code exists for placing bonus, and it recognizes a sort of nested structure:
    Code:
    if(pXML->TryMoveToXmlFirstChild(L"PlaceBonusTypes"))
    {
            int i = 0;
            int iNum = pXML->GetXmlChildrenNumber(L"PlaceBonusType" );
            m_aPlaceBonusTypes.resize(iNum); // Important to keep the delayed resolution pointers correct
    
            if(pXML->TryMoveToXmlFirstChild())
            {
    
                    if (pXML->TryMoveToXmlFirstOfSiblings(L"PlaceBonusType"))
                    {
                            do
                            {
                                    pXML->GetChildXmlValByName(szTextVal, L"BonusType");
                                    pXML->GetChildXmlValByName(&(m_aPlaceBonusTypes[i].iProbability), L"iProbability");
                                    pXML->GetChildXmlValByName(&(m_aPlaceBonusTypes[i].bRequiresAccess), L"bRequiresAccess");
                                    pXML->GetChildXmlValByName(szTextVal2, L"TerrainType");
                                    pXML->GetChildXmlValByName(szTextVal3, L"FeatureType");
                                    pXML->GetChildXmlValByName(szTextVal4, L"MapCategoryType");
                                    pXML->GetChildXmlValByName(szTextVal5, L"TechType");
                                    GC.addDelayedResolution((int*)&(m_aPlaceBonusTypes[i].eBonus), szTextVal);
                                    m_aPlaceBonusTypes[i].ePrereqTerrain = (TerrainTypes)pXML->GetInfoClass(szTextVal2);
                                    m_aPlaceBonusTypes[i].ePrereqFeature = (FeatureTypes)pXML->GetInfoClass(szTextVal3);
                                    m_aPlaceBonusTypes[i].ePrereqMapCategory = (MapCategoryTypes)pXML->GetInfoClass(szTextVal4);
                                    m_aPlaceBonusTypes[i].ePrereqTech = (TechTypes)pXML->GetInfoClass(szTextVal5);
                                    i++;
                            } while(pXML->TryMoveToXmlNextSibling(L"PlaceBonusType"));
                    }
                    pXML->MoveToXmlParent();
            }
            pXML->MoveToXmlParent();
    }
     
  3. Toffer90

    Toffer90 C2C Modder

    Joined:
    Oct 16, 2011
    Messages:
    6,630
    Location:
    Norway
    I'm not sure what you are talking about...
    The granular callback system should be removed as it serves no good purpose. The vanilla boolean callback define is a good enough solution.

    I'm saying we should clean the code:

    Change:
    GC.getUSE_CAN_BUILD_CALLBACK(eBuild) // from
    GC.getUSE_CAN_BUILD_CALLBACK() // to

    Delete this function:
    int cvInternalGlobals::getUSE_CAN_BUILD_CALLBACK(BuildTypes eBuild)
    {
    return (m_iUSE_CAN_BUILD_CALLBACK || m_pythonCallbackController.IsBuildCallbackEnabled(CALLBACK_TYPE_CAN_BUILD, eBuild));​
    }
    Only use this function:
    int cvInternalGlobals::getUSE_CAN_BUILD_CALLBACK()
    {
    return m_iUSE_CAN_BUILD_CALLBACK;​
    }

    Eradicate this define:
    Code:
    <Civ4Defines xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="..\..\..\Xml\Schema\Caveman2Cosmos.xsd">
        <Define>
            <DefineName>USE_CAN_BUILD_CALLBACK_GRANULAR</DefineName>
            <DefineTextVal>BUILD_BONUS_DONKEY,BUILD_BONUS_HORSE,BUILD_BONUS_ELEPHANT,BUILD_BONUS_BISON,BUILD_BONUS_CAMEL,BUILD_BONUS_COW,BUILD_BONUS_DEER,BUILD_BONUS_FUR,BUILD_BONUS_KANGAROO,BUILD_BONUS_LLAMA,BUILD_BONUS_MAMMOTH,BUILD_BONUS_PIG,BUILD_BONUS_POULTRY,BUILD_BONUS_RABBIT,BUILD_BONUS_SEA_LIONS,BUILD_BONUS_SHEEP,BUILD_BONUS_WALRUS,BUILD_BONUS_GUINEA_PIG,BUILD_BONUS_PARROTS,BUILD_BONUS_BARLEY,BUILD_BONUS_CORN,BUILD_BONUS_FLAX,BUILD_BONUS_MELON,BUILD_BONUS_POTATO,BUILD_BONUS_PUMPKIN,BUILD_BONUS_RICE,BUILD_BONUS_SQUASH,BUILD_BONUS_WHEAT,BUILD_BONUS_ALMONDS,BUILD_BONUS_APPLE,BUILD_BONUS_BANANA,BUILD_BONUS_COCONUT,BUILD_BONUS_DATES,BUILD_BONUS_FIG,BUILD_BONUS_LEMON,BUILD_BONUS_MANGO,BUILD_BONUS_OLIVES,BUILD_BONUS_PAPAYA,BUILD_BONUS_PISTACHIO,BUILD_BONUS_WINE,BUILD_BONUS_HENNA,BUILD_BONUS_INDIGO,BUILD_BONUS_RUBBER,BUILD_BONUS_TIMBER,BUILD_BONUS_CANNABIS,BUILD_BONUS_COCA,BUILD_BONUS_COCOA,BUILD_BONUS_COFFEE,BUILD_BONUS_COTTON,BUILD_BONUS_INCENSE,BUILD_BONUS_MUSHROOMS,BUILD_BONUS_OPIUM,BUILD_BONUS_PAPYRUS,BUILD_BONUS_PEYOTE,BUILD_BONUS_PRICKLY_PEAR,BUILD_BONUS_RESIN,BUILD_BONUS_SILK,BUILD_BONUS_SPICES,BUILD_BONUS_SUGAR,BUILD_BONUS_TEA,BUILD_BONUS_TOBACCO,BUILD_BONUS_VANILLA,BUILD_BONUS_POMEGRANATE,BUILD_BONUS_KAVA,BUILD_BONUS_GUAVA</DefineTextVal>
        </Define>
    </Civ4Defines>
    And do all this also for:
    USE_CAN_TRAIN_CALLBACK_GRANULAR
    USE_CANNOT_TRAIN_CALLBACK_GRANULAR

    Spoiler And remove this stuff :
    Code:
        #ifdef GRANULAR_CALLBACK_CONTROL
                    //    KOSHLING - granular control over Python callbacks - so far implements
                    //        CanTrain
                    //        CannotTrain
                    //        CanBuild
                    const char* entityList;
                    //    CanTrain
                    if ( GC.getDefinesVarSystem()->GetValue("USE_CAN_TRAIN_CALLBACK_GRANULAR", entityList) )
                    {
                        cvInternalGlobals::getInstance().m_pythonCallbackController.RegisterUnitCallback(CALLBACK_TYPE_CAN_TRAIN, entityList);
    
                        GC.getDefinesVarSystem()->RemValue("USE_CAN_TRAIN_CALLBACK_GRANULAR");
                    }
                    //    CannotTrain
                    if ( GC.getDefinesVarSystem()->GetValue("USE_CANNOT_TRAIN_CALLBACK_GRANULAR", entityList) )
                    {
                        cvInternalGlobals::getInstance().m_pythonCallbackController.RegisterUnitCallback(CALLBACK_TYPE_CANNOT_TRAIN, entityList);
    
                        GC.getDefinesVarSystem()->RemValue("USE_CANNOT_TRAIN_CALLBACK_GRANULAR");
                    }
                    //    CanBuild
                    if ( GC.getDefinesVarSystem()->GetValue("USE_CAN_BUILD_CALLBACK_GRANULAR", entityList) )
                    {
                        cvInternalGlobals::getInstance().m_pythonCallbackController.RegisterBuildCallback(CALLBACK_TYPE_CAN_BUILD, entityList);
    
                        GC.getDefinesVarSystem()->RemValue("USE_CAN_BUILD_CALLBACK_GRANULAR");
                    }

    All of this is an ugly solution to an imaginary problem.
     
  4. Thunderbrd

    Thunderbrd C2C War Dog

    Joined:
    Jan 2, 2010
    Messages:
    27,264
    Gender:
    Male
    Location:
    Las Vegas
    I made that tag and it's intended for use with a lot more than just the Farmer, though he's intended to be included. The problem is that:
    1) There is no display coding for it
    and
    2) There is no AI to support it

    If we fixed that, this whole mechanism would be valid for use and would be something we could easily add XML for on the Great Farmer. So many more concepts to go with that as well.

    At the moment, pre-release and technically in a freeze period, I'm just hoping we can recover what we had with the farmer before we move forward in the next release cycle.
     
  5. Toffer90

    Toffer90 C2C Modder

    Joined:
    Oct 16, 2011
    Messages:
    6,630
    Location:
    Norway
    A couple of tags would be missing for that to work for the great farmer.

    The requirements set for the bonus in CIV4BonusInfos.xml would have to be the requirements that allows the build, this is sensible to let the canBuild python function check.
    Though you could make a new build tag called something like bPlotCanHavePrereqBonus and add in code to the dll version of the canBuild function that runs the canPlotHave(eBonus) function with the bonus requirement as input.

    The requirement to have access to the bonus is not straight forward, python currently use this isPlotGroupConnectedBonus(ePlayer, eBonus).
    So the build would need a new tag that makes the dll version of canBuild check isPlotGroupConnectedBonus, which is different from what it does if you simply set a PrereqBonusType for the build.
    This can easily be added in xml with the current setup for bonus placement.

    We could simply add some high yields to the bogus improvement that the build places, the improvement is instantly removed and replaced by a bonus when its built, but the AI think it will get the improvement when it chooses to build it.

    The yield stats for the improvement can be the same as the yield stat for the bonus with an added extra on top perhaps.

    The AI adds +1 value to the improvement if it has a bonus discovery chance so we could add in a discovery chance for the bonus that it will actually create.
    In the dll we could improve the AI code that evaluates the value of improvement by giving a decent bonus value to improvements that can discover bonuses that the player don't have access to, which are revealed and enabled by the same player; like +10 instead of only +1.
    Relaevant improvement evaluation code in AI_bestPlotBuild, AI_getImprovementValue, and AI_plotValue. The last one only concerns the AI value of improvement that are actually on the map, but it should reflect the changes in the other two functions.
    Work boats, great farmer, and other workers are working correctly now.
     
    Last edited: Jul 3, 2019
  6. Thunderbrd

    Thunderbrd C2C War Dog

    Joined:
    Jan 2, 2010
    Messages:
    27,264
    Gender:
    Male
    Location:
    Las Vegas
    The build itself, outside of this tag, is capable of establishing all of those prerequisites. This is a tag for Build Infos as it is, not a new mechanism. All other build tags can be employed to further craft things - and some of the tags here should probably not even be used in the nested tag unless there's a very good reason for it (which my imagination allowed for.) They should often, instead, be established on the rest of the build's tags.

    Setting a prereq bonus type for a build that places a bonus, rather than an improvement, which is what this tag is meant to allow (you don't HAVE to have an improvement on this build) would be self-defeating since the tag automatically ensures there are no bonuses present on the plot before it enables any build using this tag to work on that plot.

    That could work to get the AI to use it BUT it would simply get the AI to spam the most valuable one or the first of the most valuable ones or something equally repeatable. What it needs to consider when considering placing a bonus is:
    1) Will doing this give me access to a bonus I don't already have except by trade so I can be assured of having future access to what I'm currently trading for?
    2) Will doing this open up my ability to build any new culture wonders or other unusual resource combination buildings in any city this plot is in range of?
    3) Will this expand my access to those resources I currently have the least of?
    4) Will this make my corporation(s) more powerful?
    5) Would this bonus (or likely bonus if it's not certain which would result) promote the most powerful possible use for that tile (such as a mineral on a hill, a plant on plains, etc...)

    If none of these are compelling enough, to some threshold, it should NOT even bother, because that would then leave room for future other better placements that may eventually become more valuable.
     
  7. Toffer90

    Toffer90 C2C Modder

    Joined:
    Oct 16, 2011
    Messages:
    6,630
    Location:
    Norway
    Can the build xml establish iMinAreaSize, iMinLatitude, and iMaxLatitude requirements?
    Can the build xml establish a requirement that only allows a specific terrain if it has a specific feature on it like the bonus requirement can?
    e.g. Can be built on grass and plain terrains, cannot be built on desert, but can be built on desert if there's a flood plain feature there.

    No, it's imo easier to directly use the bonus xml entry information as the build requirement like it currently does (sorry, it only does this in PPIO, the original GF python code on the SVN doesn't care about bonus placement requirements, but could easily be made to do so.).
    Yeah I get that, as Anq initially asked if it was possible to not define improvements at all for great farmer builds by using the PlaceBonusType build tag.
    That is pretty much what I suggested we do, that the improvements value is increased if they have a iDiscoveryChance value, it could do something like:
    "iValue += (CvPlayerAI->AI_bonusVal(...)) / 10;" (where only the one bonus with the highest value is added to the improvement value if there are more than one bonus that can be discovered by it.)

    It seems reasonable to me to have the same yields on the improvement as the yields that are on the bonus that it will actually become. The different bonus value evaluations will automatically make the AI place different ones once in a while, but we could add in a small randomness to the added value from iDiscoverChance improvements to create some AI flavour when it comes to the usage of the GF.
    That one is harder to account for as a value modifier in the AI improvement value evaluation... Perhaps it could be as easy as to reduce the value added from iDiscovery chance based on how many of the bonus that the player have with a minimum value of 1 after reduction.
     
  8. Thunderbrd

    Thunderbrd C2C War Dog

    Joined:
    Jan 2, 2010
    Messages:
    27,264
    Gender:
    Male
    Location:
    Las Vegas
    I'd have to review the build infos tags to know the answer to that. There are a lot of prereqs available to builds, some rather complex, so some of it may be possible and others may require new tags. I believe that IS possible as is, though it may require multiple builds be defined. That said, I'd think it better to add such prereqs AS new build tags rather than to the bonus placements specifically since those are really only to be used in special situations I imagined for them. The goal of this tag was, somewhat, to make it possible to make the land and random chance decide what the farmer ends up placing, rather than allowing it to be completely player choice. Or to enable the farmer (or any other worker) to earn promotions to enable more pre-determined bonus placement builds.

    Since this tag is not intended to be used with an improvement at all, how would a non-referenced improvement's discovery chance play into it? We COULD reference an improvement I suppose, but requiring it seems to run counter to the point of the tag, and if you did, you wouldn't want to ALWAYS have to have the improvement it refers to be destroyed.



    Speaking of Min latitude and Max Latitude... we really need a new way to declare our climate zones. The space maps and other strange map concepts are starting to demand that.
     
  9. Thunderbrd

    Thunderbrd C2C War Dog

    Joined:
    Jan 2, 2010
    Messages:
    27,264
    Gender:
    Male
    Location:
    Las Vegas
    I have a little time to give this deeper thought:
    These would need to be new tags - and if we're going to consider climate, as stated in the previous post, it's about time we did that. The map script could establish climates by latitudes (or whatever it wants to establish them by) but I'd want the map script or direct scenario map to make that determination. The problem I'd have in setting that up on my end would be any interaction with world builder is out of my ability completely. Maybe as a py programmer you could do something with that if there were some basic functions for setting ClimateType on plot? I can see how later on some map change effects may refer to plot ClimateTypes, and it could be useful for spawn infos for sure.

    If I'm going to work down that route soon, I'd like to finish the Biome setting and utilization project as well. I could see some overlaps there.

    As for iMinAreaSize, this probably should be nested, right? Because it would apply directly to the likelihood of a particular result. I assume that we'd be talking about marine applications for this usually, with larger bodies of water having larger chances for different types of things, smaller ones having higher chances for other types of things to result. EX: Clams very unlikely, possibly even impossible, in a small body of water, while Fish are equally likely in a large body of water as a small one. (This is the purpose of nested prereqs for the placebonus tag rather than placing the prereqs on the build itself. One build can list many possible results, all varying based on the different factors indicated in those nested tags.) I'd have to check the coding in use elsewhere on iMinAreaSize but I think it should be quite includable.

    Ok, so the BUILD can't directly, but this tag sure can limit the application of a given entry to a specific terrain AND feature (it's an AND relationship, not OR when you apply both a terraintype and featuretype to a placebonus entry).

    I should start a thread on exactly how to use this tag... I wrote it out in a PM discussion but it could use some review.

    As for the AI, not only is it difficult to implement the 'give up if it's not compelling enough' quality, which is super important because you don't want them filling all their available plots with resources as fast as they can as soon as they can because more access may open up later to resources they need more of and now they've choked themselves off - furthermore you don't want them farming all their hills when they should be mining them - and you don't want them missing out on non-bonus improvement types just because they 'have' to put bonuses wherever they can. Certainly, non-bonus based improvements will probably still need to be further empowered if we're really opening up a lot of ability to place bonuses. We might even want to consider making non-naturally placed bonuses (Goods) be map placeable by workers (without any natural benefit until accessed) that can be accessed and enhance the plot by a town type improvement, or lumbermill or windmill or watermill, etc... That would take some graphic work of course, even if very minor token stuff where we mostly rely on the bonus pointers to show where those have been placed.

    We could also demand that if you're going to have a farm at all, you must have it on a resource. Or a mine. etc... There has to be a definition to the product being obtained at such plots.

    All very long term thinking on this subject in terms of that kind of thing. Great Farmers may not be all that becomes capable of placing bonuses. We could even make some currently map based bonuses, like Corn, for example, only introduced by man, as they actually were. In fact, bonus placing units that search for the value of a land plot, like prospectors, for example, could be required to 'find' the workable bonus that exists in a plot, and NO map places bonuses at the beginning at all. Possibly.

    Anyhow, the AI would also have to account for the fixed vs random methods this tag allows, and that's an even bigger AI math challenge! All that to compare against the possible other builds that can take place in a plot... Lots to consider.

    I've often thought the display shouldn't even try to be built, but rather just apply a help text to each instance we program for this tag.
     
  10. Toffer90

    Toffer90 C2C Modder

    Joined:
    Oct 16, 2011
    Messages:
    6,630
    Location:
    Norway
    Why add new tags when all the tags exist for the bonus in question, why set the same values in the build as what is in the bonus when we could simply use the information from the bonus directly to evaluate the build.
    I like the idea behind the placebonustag for builds, my point was only that it is not yet ready to replace the current great farmer system which is what Anq asked about. The random part is a good idea, although that could be done in python too during the improvement to bonus switcharoo.
    Sorry for being a bit unclear. You misunderstood my posts about this, I meant "the current system" used for great farmers can mostly deal with the AI evaluations already. My point being that the problem you brought up about missing AI code for great farmers was easiest to solve by simply tweaking how the improvement value is evaluated.
    The tweaks I suggested should really be done anyway to make the AI appreciate improvements that can provide new resources over time through iDiscoverChance more than it currently does, currently it doesn't mean much to the AI that a mine can discover iron when the AI does not have access to iron; it means exactly the same as that a farm can discover wheat when the AI does have access to wheat.
    I remember how to use this tag from the original discussions about its implementation, others might like to get a guide though.

    Why not add a bUseBonusRequirements tag to the nested structure of placeBonusType?
     
    Last edited: Jul 3, 2019
  11. Thunderbrd

    Thunderbrd C2C War Dog

    Joined:
    Jan 2, 2010
    Messages:
    27,264
    Gender:
    Male
    Location:
    Las Vegas
    If those are on the bonus, then yes, conveniently we could simply refer to those as something that must be observed during the canBuild evaluation without having to add new tags.

    Well, I was making that point myself - the tag is not quite ready for use and there are some damned complicated things to work out before we can that I'm not looking forward to having to sort out. Certainly not something to be working on during a freeze for sure.

    There may be some of that in the code. I don't know.

    Could... though do you think we can assume such a tag would be used? Or are we saying that we might want a build that allows us to plant potatoes on Mars soil where the map would normally never allow it so we should include such a bool? Now that I'm thinking of it from that angle, I think you're right that adding that tag would be useful.


    You said you remember from the original conversation about how the tag would be used - hell I can barely remember from that conversation how it would be used so it probably would be good to write out a reference on it. The trap mechanism I built is in a similar boat.
     
    Toffer90 likes this.
  12. Dancing Hoskuld

    Dancing Hoskuld Deity

    Joined:
    Jul 5, 2004
    Messages:
    23,511
    Gender:
    Male
    Location:
    Canberra, Australia
    Sorry I have not read this thread. The GRANULAR in the CALLBACKS does have a problem but it takes seconds of the call back functions when playing the game, well maybe tenths of seconds. The code was, and is, extremely inefficient. It almost looks like it may have been done deliberately as part of the C++/Python wars. (only slightly :mischief:) Done to make Python look slower than C.

    If someone is looking at this then the I could only get the granular to work in one place. It would be useful for other call backs as well but that is another issue.
     

Share This Page