DLL coding thread

[Nightinggale]

You'll have to explain this one, because I can't see how this is different than any of the ones you added already. All Builds are available right from the start, until you throw in the Civics, and it only checks in one place if you can build?

The stuff, which is cached in CvPlayer relies entirely on civics and some non-changing data (such as which unit you get for a certain UnitClass). This mean we know the cache is correct as long as we do not change civics. If we change, then we recalculate.

What you wrote makes it sound a bit more complex. If it depends on route and improvement, then it has to be stored in the plot, not the player and the cache goes out of date by a lot of different data being changed.

We might be able to cache something in this regard in CvPlayer, but CvUnit::canBuild() will need to check route, improvement and stuff like that for the specific plot. We can't really completely cache the output.

This is actually in vanilla code if we are talking about the same thing.

Right. It's one of those leftovers from civ4, which vanilla disabled in XML only. Just like the engine is designed to improve production from mines in civ4, we can make routes improve yield production. To my knowledge no colo mod ever used that, but the code is in vanilla.

I keep forgetting about the unused features in vanilla for some reason.

 

[Kailric]

The stuff, which is cached in CvPlayer relies entirely on civics and some non-changing data (such as which unit you get for a certain UnitClass). This mean we know the cache is correct as long as we do not change civics. If we change, then we recalculate.

What you wrote makes it sound a bit more complex. If it depends on route and improvement, then it has to be stored in the plot, not the player and the cache goes out of date by a lot of different data being changed.

We might be able to cache something in this regard in CvPlayer, but CvUnit::canBuild() will need to check route, improvement and stuff like that for the specific plot. We can't really completely cache the output.

What I think we would need is a jit array for all the Improvements and one for Routes, then the Player can unlock them with Techs/Civics etc. The canBuild function checks for Improvements or Routes, if the player has it unlocked then it will allow it to build it for the civic requirement part. It then goes on to check other things like appropriate plot and such.

 

[Nightinggale]

We already have CvPlayer::canUseRoute() and I just added one for improvements. Notice how little code it takes to add more caches: https://sourceforge.net/p/colonizat.../ci/5cde9e4b4392c7aebf4092538ffb0ed0d48d3c33/
It's not coding time, which prevents us from adding more caches like this.

There is one issue though, which I didn't fix. CvCivicInfo::getAllowsBuildTypes() appears to tell if it enables Improvements. If that is really the case, then we should rename it to avoid confusion, both in DLL and XML.

I don't get CvUnit::canBuildInt(). It's not exported to the exe or python and it is only used in CvDLLWidgetData:arseActionHelp(). I can't really figure out what the intended purpose is (ok, I think I can, but it would take too long) and there are no comments. Also it can return -1, 0 and 3, though it's not clear what either of those mean. It smells a lot like it could use an enum.

I would prefer the display code and the actual build code to rely entirely on CvUnit::canBuild() as this will make consistent behavior. Right now we risk help text says the improvement can be build while it can't. General cleanup of that region of code would also be ok. Maybe we should add CvUnit::canBuildImprovement() to make canBuild less messy and less nested if-else.

 

[Kailric]

Correct, as I was saying, I started out (actually years ago now when it took me 20+ minutes to build the dll) I wanted to have BuildTypes, but it couldn't read those, but now it can. Since you've added Improvements then yeah, it should be changed to represent that.

Sometimes I feel sorry for you Night, you have to deal with all my beginners crazy programing code and all the work arounds I had to come up with in order to get something to seemingly work.

The canBuildInt() returns an int that represents the reason why you can't build. This so the help text lets you know what you need to do in order to build the Improvement. The Improvement will be shaded out in your Units commands. This also lets you know what builds you have on your Units so new Players can see them, even if they don't have the right requirement yet, like a Number of Sheep in the City. I am sure you can find a better way to do this.

Speaking of how easy it is to make a new jit array, I think I need one for "Yields that Player can Trade, but can not make himself." Like for PlateArmor, the player can buy it from a Trade Screen but at times he cant not make it himself. So, when he gets home with the PlateArmor and unloads it, he can not see it in the City. So, perhaps the Jit arrray can set to True as soon as the Player loads or recieves into his City such a Yield. This will action rebuild the Resource Table and allow you to see the Yield, but still not allow you to Produce the Yield Yourself. I have a rudimentary version of this working but not with a Jit array.

 

[Nightinggale]

The canBuildInt() returns an int that represents the reason why you can't build. This so the help text lets you know what you need to do in order to build the Improvement. The Improvement will be shaded out in your Units commands. This also lets you know what builds you have on your Units so new Players can see them, even if they don't have the right requirement yet, like a Number of Sheep in the City. I am sure you can find a better way to do this.

I would rename canBuild to canBuildWithReason (or something) and that one returns an int, which is an enum value. I will then make a new function for all the calls to canBuild.

bool CvUnit::canBuild(arg)
{
    return canBuildWithReason(arg) == BUILDREPLY_OK;
}

That will make a single function for determining reply and reason while we do not have to touch any of the current calls to canBuild().

Adding a reason is a good idea btw. It just need to be done in a more readable way, not to mention using only one approach to determine the output. Right now they can get out of sync.

Speaking of how easy it is to make a new jit array, I think I need one for "Yields that Player can Trade, but can not make himself." Like for PlateArmor, the player can buy it from a Trade Screen but at times he cant not make it himself. So, when he gets home with the PlateArmor and unloads it, he can not see it in the City. So, perhaps the Jit arrray can set to True as soon as the Player loads or recieves into his City such a Yield. This will action rebuild the Resource Table and allow you to see the Yield, but still not allow you to Produce the Yield Yourself. I have a rudimentary version of this working but not with a Jit array.

We could do some lengthy setup where we loop professions and make a list of yields we can produce with currently available professions. That would require a bunch of custom code though.

Instead we can add another yield array to XML, which tells which yields you can produce.
This would allow us to make the cache as easily as I just did with improvements. However if we add allowed_yields as an extra argument to updateInventionEffectCacheSingleArray(), it will skip looking at yields not allowed by that array. As a result, the resulting array will only be true if produce and allow are both enabled by XML.

This will follow the standard rules meaning if a yield production has no civic, which enables production, production will be enabled once the yield itself is enabled. In other words, enabling producing a yield is always on and can be ignored unless it is explicitly specified otherwise in XML.


However I don't want to mess too much with the player cache right now. I modified it in the prospecting_setting branch because it contain the start of the code to disable prospectors. If I mess too much with this function, I risk conflicts. I would rather merge that branch back into AI_traders before making any more player caches.

I think I skipped mentioning this, but when looping professions to find prospectors, it disables all professions dealing with yields, which have yet to be invented. In the producing loop, if it checks against produceable yields rather than allowed yields, it will disable professions producing the yields you can't produce yet. We would still need to modify "background production", such as buildings and centre plot, but just disabling the professions and we are more than half way there to have production eliminated.

Btw the produceable yield idea has a decent sideeffect. It opens up for col2071 where you for instance have to get uranium from the natives to research it before you can produce it (in last release). That can't be done right now, but if the uranium tech enables production instead of general allowing uranium, it can be done.

 

[Kailric]

Well, I want to continue my saved game so I'll work around it for now.

Well in the instance of Uranium and the city Resource Bar, do we want such Yields visible? I'd think not, like with Plate Armor, the early Dark Age Europeans would have no idea what that was so to have such yeilds on their resource bar seems out of place(Especially for WHM). So, the question is when does it appear on the Resource Bar? To make things simple perhaps when the Player first loads the Yield, or Receives it in a City (in the case of Native gifts) it will reset the Resource Bar and add the New Yield.

 

[Nightinggale]

Well in the instance of Uranium and the city Resource Bar, do we want such Yields visible? I'd think not, like with Plate Armor, the early Dark Age Europeans would have no idea what that was so to have such yeilds on their resource bar seems out of place(Especially for WHM). So, the question is when does it appear on the Resource Bar? To make things simple perhaps when the Player first loads the Yield, or Receives it in a City (in the case of Native gifts) it will reset the Resource Bar and add the New Yield.

I think the best solution is to make a civic, which allows this yield. Nothing else, just allow a yield (in this case uranium). YieldInfo then contain a civic ID to the one enabling it.

Next step is whenever natives offer or donates a yield, the player is granted the civic listed in YieldInfo and the yield will start appearing in the yield rows as the player is made aware of this.

We could also simply add a bool to yields, telling if they can be learned from natives. No civic and the XML modder will have an easier time setting it up. Internally in the DLL, it could be done by adding dummy yields, which are hidden and does nothing but add that yield, in which case the DLL code will be more or less the same as the other approach.

We could also introduce a yield bool telling if natives have taught the player the yield in question. We do have a few options.

 

[orlanth]

I think the best solution is to make a civic, which allows this yield. Nothing else, just allow a yield (in this case uranium). YieldInfo then contain a civic ID to the one enabling it.

Maybe I'm missing something but if I remember accurately (at least when 2071 branch was working) I think the system actually is already working well for what you guys want, via <AllowsYields> in CivicInfos. In 2071 I had several yields which you had to unlock by discovering a tech, in order to access/load/trade/store that yield and have it show up in the resource bar.

If you want to be able to discover new access to a Yield through Natives, the Natives can start with the Tech that Allows it, and you can trade with them for this knowledge of it. (you can also let there be an alternate way to discover this on your own through research). The Yield will then be unlocked and they could start gifting you units of the Yield if they want. (logically they would at least be willing to trust you with the knowledge of what it is before gifting you units of it lol).

Unlocking being able to make it yourself also seemed to be working great through <AllowsProfessions>. You can have one or more production Professions get unlocked separately from the AllowsYield. This seems the simplest way to unlock production, unless prospectors stopped this from working somehow? (potentially you can let a late tier tech unlock a better profession to produce that yield with an added % modifier, and obsolete the old profession, which could be cool).


About unlocking Improvements/Routes those also are definitely cool to have, but is there a way to enable unlocks for Remove Feature as well? (ie be able to have a tech unlock Chop Forest or Chop Jungle like in Civ). Vanilla BuildInfos is weird is that it makes you have entries for all possible Feature removals with every Build command for an Improvement or Route, which complicates things. But I guess you could unlock the ability to remove a given Feature, and then when doing a build command that requires removing that feature it could check if you had unlocked that.

 

[Kailric]

Maybe I'm missing something

Below is the issue we where referring too, and I dont think it is present in 2071.

Speaking of how easy it is to make a new jit array, I think I need one for "Yields that Player can Trade, but can not make himself." Like for PlateArmor, the player can buy it from a Trade Screen but at times he cant not make it himself. So, when he gets home with the PlateArmor and unloads it, he can not see it in the City. So, perhaps the Jit arrray can set to True as soon as the Player loads or recieves into his City such a Yield. This will action rebuild the Resource Table and allow you to see the Yield, but still not allow you to Produce the Yield Yourself. I have a rudimentary version of this working but not with a Jit array.

About unlocking Improvements/Routes those also are definitely cool to have, but is there a way to enable unlocks for Remove Feature as well? (ie be able to have a tech unlock Chop Forest or Chop Jungle like in Civ). Vanilla BuildInfos is weird is that it makes you have entries for all possible Feature removals with every Build command for an Improvement or Route, which complicates things. But I guess you could unlock the ability to remove a given Feature, and then when doing a build command that requires removing that feature it could check if you had unlocked that.

Yeah, this needs to be in there. I've added increased speed for these things somehow.

 

[Nightinggale]

You are right, that adding an array for allowing production of a yield can be done by enabling the profession producing it. However now that there is code to ban all professions producing, consuming or equipping an unallowed yield, it is possible to make the profession available from the start and then hiding it by not being able to produce the yield.

If the yield hides the profession, then you can have multiple professions producing the yield. Imagine luxury food. There are multiple professions producing it and they are unlocked by different civics. If we have a civic for allowing production of luxury food, it would be like they have requirements for two civics.

About unlocking Improvements/Routes those also are definitely cool to have, but is there a way to enable unlocks for Remove Feature as well? (ie be able to have a tech unlock Chop Forest or Chop Jungle like in Civ). Vanilla BuildInfos is weird is that it makes you have entries for all possible Feature removals with every Build command for an Improvement or Route, which complicates things. But I guess you could unlock the ability to remove a given Feature, and then when doing a build command that requires removing that feature it could check if you had unlocked that.

My addition was adding a cache for an existing XML value, which makes it quick and easy to tell if a player can build improvements. It was already possible to set up in XML (which was poorly named, but that's another story).

We can always add more, both caches and XML values. It's just a matter of getting the ideas. One thing I started thinking about right now is that we allow units, not unitclasses. While it allows enabling units of the same class with different civics, it might also be a bit messy if there are say 4 units in a class and they should all be enabled at the same time. I'm not sure if it would be best to change to unitclasses, allow both or do nothing about it

 

[Kailric]

One thing I started thinking about right now is that we allow units, not unitclasses. While it allows enabling units of the same class with different civics, it might also be a bit messy if there are say 4 units in a class and they should all be enabled at the same time. I'm not sure if it would be best to change to unitclasses, allow both or do nothing about it

Currently, its just the opposite, we allow UnitClasses, but like you say it would be neat to allow units and have cool civic changes.

 

[Nightinggale]

Currently, its just the opposite, we allow UnitClasses, but like you say it would be neat to allow units and have cool civic changes.

You are right. I got it mixed up because the cache actually caches if a player can use specific units rather than unit classes. This mean CvPlayer::canUseUnit() will not only tell if it's allowed by civics, but also if it is allowed by civInfo+unitClass.

I better write some comments on the checking code because I was looking at it and wondered why I had written it like that. What it does is this:

iUnit =- the unit to test for
iClass - the UnitClass iUnit belongs to
iTest - the civ's unit for iClass

The player can't use the unit unless iUnit == iTest. The problem is that I didn't write any comments and the variables are not that well named because it's generic code. In fact the same code works on buildings and building classes. There is also some checking on professions (check native only and such).

I like that part of the code even better now than when I wrote it, but it really need comments. Not only should the code be easy to read, but the canUse*() functions should have written precisely what they check for in the cache.

 

[orlanth]

IMO in most cases it's best overall to go with unlocking UnitClasses and BuildingClasses since each civ has one unit for each class; the chief role for these is so you can mod a specific Civ to have a Unique Unit or Unique Building that replaces the generic one, but overarching game elements like Techs/Civics/FFs can have consistent effects on the Class which contains the unique and generic versions. I remember in an old version of the inventor modcomp that unlocked Units instead of UnitClasses, this caused all kinds of Tech issues/crashes, which I eventually found happened when civs had unit A in a class and got access to unit B from that class simultaneously

However, if you wanted to add a possibility of switching which Unit in a Unitclass a civ has, that could potentially be a cool added feature if it wouldn't cause insurmountable problems. Also it could be cool to have a unique Unit to be granted by an Event.

 

[Nightinggale]

I don't think we will change how units are enabled. It's way too much work on a system which actually works well as it is. The talk about it was a rare glitch from my part

Also it could be cool to have a unique Unit to be granted by an Event.

If events can give you a civic, then that would be possible with the current code. The civic enables the unit and then it is provided to the first to discover that civic. It would be an issue if it should be possible to do repeatably though as that would clash with the first to discover part, not to mention enabling a civic you already enabled is an issue.

Regardless of possible glitches (which can be fixed), the concept of allowing an event to provide a civic, either temporally or permanent will more or less open pandora's box of event options with everything we have planned for civics.

 

[Kailric]

Well, I can't push to AI_Traders without pulling first and that would mess up my saved games correct?

I could create a new branch if I need to push now. I am working on Economy stuff so perhaps a new Branch of that would work.

 

[Nightinggale]

Since your last commit to AI_Traders, all what I did was finishing consumption multiplier and add CvPlayer::canUseImprovement(). None of those affects savegames.

I did break savegames in the develop branch though.

 

[Kailric]

So, to create a loop for None Cargo Yield Types I would do this..

for (int iYield = NUM_CARGO_YIELD_TYPES; iYield < NUM_YIELD_TYPES; iYield++)

 

[Nightinggale]

So, to create a loop for None Cargo Yield Types I would do this..

for (int iYield = NUM_CARGO_YIELD_TYPES; iYield < NUM_YIELD_TYPES; iYield++)

That should work just fine. In fact I think it is in use in multiple locations already.

 

[Kailric]

Night and I talked about setting up the Trade Point system all to its self so it doesn't have to be a FF Category. The FF's are set up to reward points based on actions defined in the FatherPointInfos.xml. Trade Techs use the same Point system and it would be good to keep using that same point system, so that when the dll calls a changeFatherPoints, it will also changeTradePoints.

The problem is when ever a count is done for the number of FatherPointInfos it is always one too many as Trade Points is a Category that isn't used for FFs. So, what could be the simplest solution so that the FF screens will be compatible for all mods, as currently the FF python screen has hard coded values because of the Trade Points.

For now I am looking for the simplest solution, so could we setup a system so that when the dll calls getNumFatherPointInfos(), that the correct number of Infos is returned. We could add a new attribute to FatherPointInfos so that infos marked with the bSkippedInCount are not counted for in the total number of FF Categories. Of course, those that are Skipped must be placed at the end of the list.

 

[Nightinggale]

Now that I think about it, I kind of like having trade points together with the FF points. It mean we can use civics to provide a production bonus to a single type if we like and the same code would take care of both FF and trade points.

For now I am looking for the simplest solution, so could we setup a system so that when the dll calls getNumFatherPointInfos(), that the correct number of Infos is returned. We could add a new attribute to FatherPointInfos so that infos marked with the bSkippedInCount are not counted for in the total number of FF Categories. Of course, those that are Skipped must be placed at the end of the list.

We have a function, which is executed right after all XML files are read. We could make that one loop and assert if bSkippedInCount is false after it has been true. The same function can then count the number of not-skipped point types.

Whenever somebody edits XML, it should be with asserts enabled as there are tons of such sanity checks all over. One more makes no difference.