CivEffects

[Nightinggale]

I looked though the AI code to figure out how it handles prohibited civics because I think it would be best to share the code, hence share the bugs. If an AI bug turns up as a UI bug for the player, then it will be easier to notice.

It turns out that the AI completely ignores prohibited settings. It looks to me like it will not mind using two mutually exclusive civics at the same time. Wondering about how to fix both at the same time has resulted in a new C++ class with the "selected/wanted civics", which should then have member functions to verify that the setup is in fact valid. Getting the AI to find "the best" settings will be slow, but can be made to be as fast as the current speed if a cache is implemented. Now it has grown to be so big that I think the best solution would be to strip out prohibited checks from python and leave it at that for now. This new check can then be introduced AFTER next release. The next release should be "good enough", not perfect or we will never release. Since all submods waits for the next release, we really should avoid all non-essential addons for now.

I will add this to the todo list on sf with some remarks on how I think it should be done.

Edit: Installing a python ide now.

I use Notepad++. If you find anything, which has a working debugger, then do tell. While python debugger is mentioned in the vanilla setup, I never managed to get anything working.

 

[Kailric]

I've installed perl. Was I suppose to be able just to pull the latest version of Civeffect and it works as it still hangs at running fastdep unless i put in that modified makefile?

Also, I am getting a Failed loading XML file\CivEffects/Civ4EventsCivEffectInfos.xml then a bunch of other errors when I start the mod

I ran the updateMod perl script and it removed that comment at the start.

 

[Nightinggale]

Just a little status update for those not paying attention to the git log + some stuff not mentioned in the log.

Civ4EventsCivEffectInfos.xml is added to git (it was lying on my HD without being added, oops )

I taught Kailric how to use InfoArrays, a new data structure added for CivEffects. It's way more strict about variable types and will assert or fail to compile if somebody use BuildingClassTypes as BuildingTypes or similar. Great for avoiding bugs, but it turned out that the extra strictness required a bit of training to use it correctly.

Kailric is currently adding all the pedia text for CivEffects. This is now mainly stored in InfoArrays, hence the need to understand those.

I disabled prohibited Civics in the python civic screen and it no longer causes python errors. Do not make civics block civics until the new code for those is in place (that will take a while). I might modify the reading code for those to ensure xml can't trigger undefined (buggy) behavior with this now unimplemented feature.

The missing revolution button in the civic screen has turned out to be a real nightmare. Absolutely nothing in the screen use screen resolution, meaning it only looks good in the resolution that was used when it was made. I'm going to write a "fit to screen" engine for that file, but to do so, I need to figure out precisely how it is drawn right now. This turned out to be quite hard as I looked up the python API to figure out what the numbers in the arguments means... or rather I tried. I couldn't access the forum and investigating that problem resulted in me rebooting my router and everything worked again. That's the first time ever that my router have caused problems. This mean I have already spent hours marked to fix the resolution for that screen and I have yet to write even the first line of code for it. It seems the fit to resolution is the only real problem. Overall the code actually seems pretty decent and works well ingame. I might make it scale according to the number of civics in each category and number of civic categories to make it one python file for all xml setups, but other than that, it seems fine.

That's the status now. I figured people would be interested in seeing whats happening and certainly that something is happening.

 

[Kailric]

Kailric learn something knew. Kailric happy. Kailric go bang bang on pc somemore...


There are many missing XML values after the conversion. Like no Civics are currently allowed by Techs. I believe I saved a excel file with all the values we had before, or I can just look at an older version to determine what the changes should be.

 

[Nightinggale]

Kailric learn something knew. Kailric happy. Kailric go bang bang on pc somemore...

There are many missing XML values after the conversion. Like no Civics are currently allowed by Techs. I believe I saved a excel file with all the values we had before, or I can just look at an older version to determine what the changes should be.

I think the best approach is to use the xml files from the develop branch for comparison. I did script preserving as many tags as possible, but I know the script would be more than happy to throw out tags, which is why we need to verify everything manually. Writing the script and checking manually is still way faster than writing all the xml files manually from scratch. The format is very different, which mean a simple copy paste will not do.

 

[Lib.Spi't]

Kings of Bang! Kings of Bang!



Yes I do enjoy reading these things, it lets me feel like I is smarts!

I assume by now you have looked at platy's python work Night? All his screen stuff is written as resolution scaling. May be something useful in there, maybe not!?

 

[Nightinggale]

I looked at my domestic advisor screen, which scales to resolution and adjust the number of columns to resolution as well. The problem is not how to make it resize to fit the screen, but rather to decode how it is drawn right now and reuse as much as possible because all but the resize works. I don't want to recode the preview code and all that.

 

[Nightinggale]

Rewrote the placement code for the civic screen. It will not set the width of each civic option based on resolution to make sure it use the entire width of the screen and that it will always have room for everything. The bottom two lines are now locked in place relative to the bottom of the screen and they use full screen width. The help text "block" now change height to fill the gap to the bottom rows.

I made a low resolution version, which triggers if screen height is less than 800 pixels. It places the items closer to each other in height and makes the text rows (top and bottom) shorter. The result is a screen, which is a bit more cramped, but it gains more space for the help text, which mean it has a better chance of displaying everything despite the low resolution. There is more "unused" space to claim for this purpose if needed, but I didn't feel like continuing that work. It's not bad as it is.

We may consider a high resolution font version as well. Switch to a really high resolution will keep the blocks at the same size relative to the screen resolution, but the font is fixed to pixels rather than relative to screen size, meaning the higher the resolution, the smaller the font feels like and it can end up being something tiny just in the top of the blocks.

And naturally all buttons are now on the screen at any resolution. This mean any future improvements will be to improve visual quality and not functionality as full functionality is restored.

 

[Lib.Spi't]

We could one day in the future switch to the Civ Platy style civic screen which displays civics in a list down the left with two comparison effect boxes on the right. This gives 'unlimited' (scroll bars) space for any number of civics and any length of effects and help text.

Not a priority right now, but as a expandable concept tool it is really good.

 

[Nightinggale]

I'm in the progress of writing the help text for all tags used for CivEffects. This help text is used in civic and tech screens as well as in pedia. While the plan is to expand, we already have more than 100 tags, which mean I have to get this well organized or we will never get everything to display correctly. It turns out that the TXT_KEYs are a mess of semi-random names as well as missing keys.

I decided that all tags should appear in help text in the same order as they appear in xml. They are in that order in the C++ header file as well. Tags are sorted into groups with a common theme (like city modifiers, unit modifiers and so on) and sorted alphabetically in each group. The groups themselves are also sorted alphabetically. This makes everything turn up at a predictable location. However it only works as long as this system is maintained by the modders. If somebody adds new stuff at random locations, then... well only a witch would do something that evil and we all know what to do with witches

I also decided to rename the TXT_KEYs. They now have the prefix TXT_KEY_CIVEFFECT_. They should be written in the same order as they are used, which mean they use the system as mentioned already. It goes positive, negative if two strings are needed, such as allows and disallows.

The strings are placed in a new text xml file called DLL_CivEffect_Help.xml. I decided on the prefix DLL_ as it indicates that all the strings inside it are used by the DLL and as such should be kept by other mods. If we eventually sorts all strings into DLL_, EXE_, PYTHON_ and (whatever we call the mod specific ones), then it is more clear what goes where and what to touch and not touch as a pure xml modder. It's a big task to sort all the strings, but starting with CivEffects is not bad.

EDIT:
The TXT_KEYs just gets worse and worse. TXT_KEY_INVENTION_DISALLOWS_CENSURE is used for... yeah you guessed it, to tell which BUILDING class is disallowed by the CivEffect
Something tells me that Kailric reused this string at some point, which made sense back then. However not anymore and now it is just confusing.

 

[Kailric]

Everything I did back then was confusing to everyone and me. I would have just been excited it worked

 

[Nightinggale]

Everything I did back then was confusing to everyone and me. I would have just been excited it worked

Fair enough. I guess that's the most important part anyway. Also don't add 72 smilies when there is a limit on 30 in each post

The string "Obsoletes name" is used multiple times. However I decided to only use each string once. This allows for more interesting text at a later date and it can be added with xml editing only.

 

[Kailric]

Hmm, 72 smilies, I sent that from iPhone and only did one smilie

 

[Nightinggale]

Going through the help text tag by tag appears to be a major task. I managed 3 tags within the last 24 hours and at that speed it will take 3 months to get through all of them. Granted I certainly didn't work for all 24 hours, but...

I noticed that most tags require sort of the same setup. It shouldn't be impossible to make a function, which would allow a one tag, one line approach. Reusing the code also reduce the risk of bugs. This would also force us to always use the arguments for GetText in the same order, meaning it will be easier to write the text xml. After all the description is always 1, change is 2 or whatever it ends up to be. We could even add bogus arguments to make a text string using argument 1 and 3 to preserve this consistency with numbers.

Getting all possible buildings using a specific building class results in lengthy and ugly code. I plan on adding a function to CvBuildingClassInfo where it returns an InfoArray with all the buildings, which use the building class in question. It should have a CivilizationTypes as argument with the default NO_CIVILIZATION to set a filter. It would likely be a good idea to do with units as well, but so far I only needed it for buildings.

Adding the text to the same xml file turned out to be a really good idea. I review the text while moving/writing it, which mean it doubles as telling what each tag does. We lack proper documentation for this and while I planned for this and tried to document it, the documentation I did so far didn't really work out. Also I still plan on copying the FTTW page with tag descriptions at some point, but one thing at a time.

 

[Kailric]

One thing at a time, yes. This weekend is pretty busy but starting next week I should be able to start knocking out that todo list I posted.

 

[Nightinggale]

I have been thinking about the messy approach to CivEffect Help text generation as the vanilla approach is hard to read, time consuming to write and prone to bugs. Looking at what I had written earlier and what Kailric had written recently, I realized it was mainly copy paste and then changing a few variables. C++ is an objective programming language and it should be possible to write generic code, which can then be called over and over with different arguments. I started reading up on pointers to functions and template functions (the combo that is). The idea for implementation started taking shape and with what I just pushed to git, the code to write looks like this:

///
/// TagGroupAllow
///

	printInfoArray      (szHelpText, pCivEffectInfo->getAllowsBonuses()                       , &CvGlobals::getBonusInfo              , L"ALLOWS_BONUS"                       );
	printBuildingArray  (szHelpText, pCivEffectInfo->getAllowsBuildingTypes()                 , pCivInfo                              , L"ALLOWS_BUILDING"                    );
	printInfoArray      (szHelpText, pCivEffectInfo->getAllowsCivics()                        , &CvGlobals::getCivicInfo              , L"ALLOWS_CIVIC"                       );
	printInfoArray      (szHelpText, pCivEffectInfo->getAllowsHurry()                         , &CvGlobals::getHurryInfo              , L"ALLOWS_HURRY"                       );
	printInfoArray      (szHelpText, pCivEffectInfo->getAllowsImprovements()                  , &CvGlobals::getImprovementInfo        , L"ALLOWS_IMPROVEMENT"                 );
	printInfoArray      (szHelpText, pCivEffectInfo->getAllowsProfessions()                   , &CvGlobals::getProfessionInfo         , L"ALLOWS_PROFESSION"                  );
	printInfoArray      (szHelpText, pCivEffectInfo->getAllowsPromotions()                    , &CvGlobals::getPromotionInfo          , L"ALLOWS_PROMOTION"                   );
	printInfoArray      (szHelpText, pCivEffectInfo->getAllowsRoutes()                        , &CvGlobals::getRouteInfo              , L"ALLOWS_ROUTE"                       );
	printInfoArray      (szHelpText, pCivEffectInfo->getAllowsTradeScreens()                  , &CvGlobals::getEuropeInfo             , L"ALLOWS_TRADE_ROUTE"                 );
	printUnitArray      (szHelpText, pCivEffectInfo->getAllowsUnitClasses()                   , pCivInfo                              , L"ALLOWS_UNIT"                        );
	printUnitArray      (szHelpText, pCivEffectInfo->getAllowsUnitClassImmigration()          , pCivInfo                              , L"ALLOWS_UNIT_IMMIGRATION"            );
	printInfoArray      (szHelpText, pCivEffectInfo->getAllowsYieldsClass()                   , &CvGlobals::getYieldInfo              , L"ALLOWS_YIELD"                       );

///
/// TagGroupCityModifiers
///

	printBuildingArray  (szHelpText, pCivEffectInfo->getBuildingProductionModifier()          , pCivInfo                              , L"CITY_BUILDING_PRODUCTION_MODIFIER"  );
	printInfoArray      (szHelpText, pCivEffectInfo->getBuildingRequiredYieldModifier()       , &CvGlobals::getYieldInfo              , L"CITY_BUILDING_REQ_YIELD"            );
	printBuildingArray  (szHelpText, pCivEffectInfo->isFreeBuildingClass()                    , pCivInfo                              , L"CITY_FREE_BUILDING"                 );
	printNumber         (szHelpText, pCivEffectInfo->getMaxCityPopulationChanges()                                                    , L"CITY_POLULATION_CHANGE"             );
	printInfoArray      (szHelpText, pCivEffectInfo->getMaxYieldChanges()                     , &CvGlobals::getYieldInfo              , L"CITY_CHANGE_YIELD_CAPACITY"         );

Now it should be fairly easy to add more. Take the first for instance. It reads the date from getAllowsBonuses(), use CvGlobals::getBonusInfo() to get the description string and getChar and use "ALLOWS_BONUS" as string to be printed. The string then gets a TXT_KEY_CIVEFFECT_ prefix and it generates two versions with the postfixes _POSITIVE and _NEGATIVE depending on if the number goes up or down.

It's still very strict on types, even though they are sort of hidden from the list. getAllowsBonuses() will only work with BonusTypes and CvGlobals::getBonusInfo takes a BonusType as argument. If those two mismatch, then the game will assert.

When calling GetText, it always supply the same 3 arguments in this order: iChange, name, char.
iChange is the number involved, meaning it can be ±1 for allow and -50 for a modifier and so on. Name is the result of getDescription from the get*Info pointer and the char is getChar for classes, which have chars. Not all arguments are used in each case. For instance isFreeBuildingClass() will result in first argument to always be 1 and the char will be an invisible one. The second is the name of the building, which is likely just what we need.

I didn't test every single string with this system. We will have to do that later. Right now I just checked that each of the functions behave as expected. I did notice that units will not always get the right chars. I will look into that problem when the branches are merged because it looks like my GameFont debug code is in another branch. Until then we will have to live with the catapult being displayed as a smiley and stuff like that.

I'm very happy with the result. It is a solution, which really makes good use of C++ and it is a candidate to be the best to reuse code in any mod. At least I haven't seen anything being even close. Surprisingly it only took 4 hours to code this. The planning and research stage took way longer than that, despite the fact that I have already used both pointers to functions and templates in M:C before. The research wasn't "hello world" for pointers to functions

One thing I'm wondering about is connecting to CvPlayer. It would be cool if techs in "requires" strings change color depending on if the player has them, like "already got it, can research, is blocked". However since we do not actually have a pointer to CvPlayer, it might be too much work to get something like that to work.

 

[Kailric]

Awesome work Night! I was gonna do something similar but that's close enough Heh, yeah right, I would have just continued on with the general chaos as always

 

[Lib.Spi't]

General Chaos

 

[Nightinggale]

Just converted the next two tag groups to the new format. My speed is greatly increased by the fact that Kailric gathered the existing TXT_KEYs meaning I will not have to search wildly for those. I ran into 5 tags, which doesn't fit into the new functions. I added a new one line overloaded function to handle one of them, but 4 remain. I'm considering changing 3 of them to InfoArrays, which will always have length 1 or 0 as this will make them fit into the generic functions, not just for help, but in general. I just left them as is for now and we can clean it up later. In fact there is a bunch of code, which can be cleaned up in the CivEffect branch, like unused trait code and stuff like that. It's not urgent and can wait until after the next release.

General Chaos

He is way more productive than Rear Admiral I'm-not-touching-that

 

[Nightinggale]

Kailric did something clever and I completely missed it

If you set a civic to give combat bonus against another civic, it will do that and that is what I made CivEffects do. However at least according to the old help text, if the civic points to itself, it will not provide a bonus towards itself, but instead it will provide a bonus against all players NOT having the civic in question.

I kind of like the idea, but I have no idea how to implement it now. It fails really badly with the cache system and the help system. If we are to keep this ability, we would have to make a new tag for this.

I think I need to add this as a new tag and then add a new InfoArray to store combat bonuses in. It should not be read from xlm, but rather autogenerated to say +X% against all civics of the same civic option group except the civic itself and then add this new InfoArray to the same cache.

Damn you Kailric and your ideas for reducing the number of tags. It's giving me problems each time you do that

EDIT: the more I think about it, the more complex it becomes. The real solution would be to make a 2D InfoArray of length max 1 to store civic and bonus. This will allow non-civic CivEffects to use this ability too.

There is a big technical problem though. Civics should know their civic option type prior to CivEffects being read, but that isn't the case. Civics do not know their type prior to Civics being read. For this to work, CivicOptions should be read before civics and then civics should be modified to read their type during the pre-read reading round (the one reading the types). It is likely doable, but the task grows into something I really don't want to mess with right now.

EDIT2: I'm halfway through the plot taggroup. After that I just need production and unit groups. Even though they are bigger than average, I can see an end to this and just a few days ago I was working at a rate, which would make this take 3 months. The new generic code approach really is a timesaver

I'm seriously considering disabling the route movement bonus tag until further notice. Right now it works, but the internals work quite.... odd and the xml modder can't really predict which numbers to use. Even worse is if multiple CivEffects provides bonus to the same route type. The movement cost is 60 internally, which mean the internal number should be (60, 30, 20, 15, 12) for 1 to 5 plots for each movement point. Now the question is: what happens when a CivEffect provides -10?

I wrote about changing this tag to +1 provides one plot more for each movement point regardless of existing movement costs. That would be a system xml modders can figure out. However this big change will likely have to wait until after next release as it change how movement cost is handled.