The Medieval Economy

Savegame please. This sounds like a serious bug, which would be great to fix. I fail to see a connection to plotgroups through, but that doesnt make it any less important.
Actually, it's prolly not all that serious. I remember this one, it has something to do with the new censures and the old code. I can Polly fix this without a save when I get the chance.
 
Next line will use eYield to read from an array, which mean the game will crash. I think any bug, which is is able to crash the game should be considered serious. The fact that it might be trivial to fix isn't the same as the bug isn't serious.

Well, you under estimate my amateur coding skills:lol: More than likely the next line doesn't use eYield, rather the assert was left over code, or eYield want be used in that instance. The game didn't crash so I suspect one of those :) The censure code uses the same code for Tea Parties. Tea Parties require the eYield but Censures do not use the eYield.

Also, I mentioned already my plans for the plotgroups. Things like Making trading deals that require the same plot group and such.
 
I haven't encountered crashes so far after redownload :king:, but only up to turn 120 or so.

Re units splitting plotgroups; it might be best to have civilians not have this effect (ie Professions with <bUnarmed> or Units with <bOnlyDefensive>). Otherwise you'd be leery of letting in any traders due to huge route disruption. Also with Open Borders, the civs in the agreement likely wouldn't break each other's plotgroups when permitted to march through.

If you're editing the Censures, that's something else that would be realllly great to eventually become more modular. Ie be able to mod the criteria for when a censure type would be enacted or expire. You could even get positive ones like a temporary Blessing or Boon for reaching the top relationship level, as well as negative censures for being bad. The consequences for each censure are already nicely moddable in civicinfos, but it uses a set of predefined hardcoded censures where you can't adjust or see what triggers them. IDK but it might be possible to use something like civ4eventtriggerinfos.xml to let those existing XML-moddable trigger criteria invoke a censure, like it already works for triggers to start an Event. But I digress.. I've definitely marched into another plotgroup :crazyeye:
 
If you're editing the Censures, that's something else that would be realllly great to eventually become more modular. Ie be able to mod the criteria for when a censure type would be enacted or expire. You could even get positive ones like a temporary Blessing or Boon for reaching the top relationship level, as well as negative censures for being bad. The consequences for each censure are already nicely moddable in civicinfos, but it uses a set of predefined hardcoded censures where you can't adjust or see what triggers them. IDK but it might be possible to use something like civ4eventtriggerinfos.xml to let those existing XML-moddable trigger criteria invoke a censure, like it already works for triggers to start an Event. But I digress.. I've definitely marched into another plotgroup :crazyeye:

All in due time my friend, but this is an excellent idea. From now on when I add new features I'll have to make sure it is all moddable. Before, I hardcoded so much not anticipating requests for otherwise:crazyeye:
 
Well, you under estimate my amateur coding skills:lol: More than likely the next line doesn't use eYield, rather the assert was left over code, or eYield want be used in that instance.
I think you underestimated my bug tracking skills :p
The function in question will crash if eYield is out of range as it accesses an unallocated part of m_aiYieldTradedTotal.
Spoiler :
void CvPlayer::setYieldTradedTotal(YieldTypes eYield, int iValue)
{
FAssert(eYield >= 0);
FAssert(eYield < NUM_YIELD_TYPES);

if(iValue != getYieldTradedTotal(eYield))
{
m_aiYieldTradedTotal[eYield] = iValue;​
}​
}

Windows should kill any process accessing memory outside the allocated memory. If you can execute this code without crashing, then you might have hit an address allocated by something else, which is also a serious bug.

Re units splitting plotgroups; it might be best to have civilians not have this effect (ie Professions with <bUnarmed> or Units with <bOnlyDefensive>). Otherwise you'd be leery of letting in any traders due to huge route disruption. Also with Open Borders, the civs in the agreement likely wouldn't break each other's plotgroups when permitted to march through.
Currently a plotgroup can't use a plot if there is a unit present, which isn't invisible and plotgroup owner is at war with unit owner. Another restriction is that plotgroups can't it use a plot where plotgroup owner is at war with plot owner. Adding that it should check bUnarmed as well sounds ok. On the other hand do you think you will use unarmed people to block the enemy during war? Maybe it would be ok to ignore this special case and save the CPU time, which would otherwise be needed to look up profession info.

I have yet to review the code regarding updates related to units. I have to hook into unit movement code, unit generation code (new bandits etc) and so on. I also consider making a cache for units in plots. Civ4 has one, which makes me suspect it would be a good idea to ensure decent performance on large maps.

I've definitely marched into another plotgroup :crazyeye:
Plotgroups are quite different from most other code as it is copy pasted from civ4BTS. "our code" part of it came when I realised I couldn't rely on civ4 exe behaviour and wrote my own "is connected" feature as well as my own plotgroup grow/shrink code.
Most other changes do not have civ4 as reference for working code, which mean we have to figure out on our own how to get it to work.

All in due time my friend, but this is an excellent idea. From now on when I add new features I'll have to make sure it is all moddable. Before, I hardcoded so much not anticipating requests for otherwise:crazyeye:
Yeah I'm pretty destructive to come up with the idea to share the DLL for multiple mods :lol:

A victim of your own success, now the monkeys want in! Now where did I put my hammer...
Here it is :badcomp:
 
I think you underestimated my bug tracking skills :p
The function in question will crash if eYield is out of range as it accesses an unallocated part of m_aiYieldTradedTotal.
Spoiler :
void CvPlayer::setYieldTradedTotal(YieldTypes eYield, int iValue)
{
FAssert(eYield >= 0);
FAssert(eYield < NUM_YIELD_TYPES);

if(iValue != getYieldTradedTotal(eYield))
{
m_aiYieldTradedTotal[eYield] = iValue;​
}​
}

Windows should kill any process accessing memory outside the allocated memory. If you can execute this code without crashing, then you might have hit an address allocated by something else, which is also a serious bug.

Ahh, the code in question is:

Code:
case DIPLOEVENT_REFUSE_TAX_RATE:
		{
			AI_changeMemoryCount(ePlayer, MEMORY_REFUSED_TAX, 1);
			CvPlayer& kPlayer = GET_PLAYER(ePlayer);

			if (kPlayer.isHuman() && GC.getLeaderHeadInfo(kPlayer.getLeaderType()).getVictoryType() == 1)
            {
                if (AI_getAttitude(ePlayer, false) <= ATTITUDE_ANNOYED)
                {
                   [COLOR="Red"] YieldTypes eYield = (YieldTypes) iData1;
					if (eYield != NO_YIELD)
					{
						kPlayer.setYieldTradedTotal(eYield, 0);
					}[/COLOR]

The part marked in red is what is causing the problem. I thought I may have left a "FAssert(eYield < NUM_YIELD_TYPES);" at a random location, but not the case. Well, for some reason the eYield is not being passed correctly from Python, will have to check that out.

Currently a plotgroup can't use a plot if there is a unit present, which isn't invisible and plotgroup owner is at war with unit owner. Another restriction is that plotgroups can't it use a plot where plotgroup owner is at war with plot owner. Adding that it should check bUnarmed as well sounds ok. On the other hand do you think you will use unarmed people to block the enemy during war? Maybe it would be ok to ignore this special case and save the CPU time, which would otherwise be needed to look up profession info.

Well, what if your enemy parks a wagon or treasure unit on your road, should that breakup the plot group? Plotgroups should probably only be broken up by military units when at war.
 
Well, what if your enemy parks a wagon or treasure unit on your road, should that breakup the plot group? Plotgroups should probably only be broken up by military units when at war.
We can always argue on which units to include/exclude and change it later. The code controlling this is CvPlot::isTradeNetwork(). Currently it calls isVisibleEnemyUnit(), which could be altered if needed. Actually it is just a function calling plotCheck() and we can make a brand new function to provide new arguments to plotCheck(), which in turn gives us what we want. This isn't a major issue.

I looked into writing a cache for units present on each plot. I now have a cache telling how many units each player has on a plot. However I just realised that the cache is less useful as I had hoped. The count alone helps very little and invisible is based on which team is watching. Now we need a cache for each player, which might actually take longer to update than the calculation we want to avoid.

I wonder if I completely wasted the time making this or if we can use a cache to something useful. Right now I can't see anything really useful for it anymore :cry:

Maybe it can be a counter of units, which can't be invisible. That way if a unit enters a plot where the count is different from 0, then recalculating plotgroups is useless. However I'm not sure this helps enough to justify keeping such a cache :dunno:
 
We can always argue on which units to include/exclude and change it later. The code controlling this is CvPlot::isTradeNetwork(). Currently it calls isVisibleEnemyUnit(), which could be altered if needed. Actually it is just a function calling plotCheck() and we can make a brand new function to provide new arguments to plotCheck(), which in turn gives us what we want.

Yeah, I actually tinkered with plotCheck some adding in my own variables to differentiate between hostel barbarians and passive ones.

I looked into writing a cache for units present on each plot. I now have a cache telling how many units each player has on a plot. However I just realised that the cache is less useful as I had hoped. The count alone helps very little and invisible is based on which team is watching. Now we need a cache for each player, which might actually take longer to update than the calculation we want to avoid.

This sounds like something that the AI may can use to check for different things, like a player mounting a mass army to attack. I'd leave it for sure, we'll find something to do with it.
 
Ok, I committed the code. Instead of explain it I decided to copy paste the important part. If we place the .add() inside and if, then the if will tell which units to count. I imagine something like millitary units, never hidden from enemy and can't hide nationality could be useful.
Code:
[COLOR="Green"]// function to change units in cache.
// restrictions on which units to count can be added here.
// argument i tells if the unit should be added (1) or removed (-1).
// adding restrictions might demand more calls to this function where units turns these restrictions on/off.
// SAVEGAMES: cache isn't saved and is recalculated on load. Changes will not affect savegame compatibility.[/COLOR]
void CvPlot::changeUnitCache(int i, const CvUnit* pUnit)
{
	[COLOR="Blue"]m_aiUnitCache.add(i, pUnit->getOwnerINLINE());[/COLOR]
}

Ironically this cache will slow down the game right now. The reason is that it is backed up, recalculated and then compared to the backup (assert on failure) each turn. We will disable that eventually, but first we should confirm that it fails to assert ;)
 
Those concepts sound cool. :goodjob: Keep in mind the whole Invisible system works in a somewhat weird way, with a separate XML file in BasicInfos that declares different types of invisibility, and each unit can get one type of <SeeInvisible> in unitinfos. This is potentially cool for things like spies, assassins, wandering minstrels, cloaked vessels etc etc; but if only visible units can block it would mean that you'd want to avoid discovering them.

I'd say for performance/stability it's fair to keep things simple as possible, & break if there is a unit present and plotgroup owner is at war with unit owner (ie an unarmed unit could potentially disrupt trade if it wanted.)
 
Apart from the initial bug from turn 1 &#8220;city lacks a plotgroup&#8221;
Fixed. It turned out that the plotgroup code only executes during the game, which at first sounds ok. However the native settlements are placed before turn 1 and because of that they are not placed during the game. Now the code to update plotgroups on game load (old games) will be called to ensure all cities having plotgroups. This problem is also corrected when loading savegames.

While researching this issue I located a different bug. City placement builds the best type of available road buildable on the plot in question. However it used plot NULL to test this, which mean it was hardcoded to use the worst kind of road nomatter which road you have invented. I fixed this as well. (4 letter fix :lol:)

Those concepts sound cool. :goodjob: Keep in mind the whole Invisible system works in a somewhat weird way, with a separate XML file in BasicInfos that declares different types of invisibility, and each unit can get one type of <SeeInvisible> in unitinfos. This is potentially cool for things like spies, assassins, wandering minstrels, cloaked vessels etc etc; but if only visible units can block it would mean that you'd want to avoid discovering them.

I'd say for performance/stability it's fair to keep things simple as possible, & break if there is a unit present and plotgroup owner is at war with unit owner (ie an unarmed unit could potentially disrupt trade if it wanted.)
I was thinking of making a bool check to tell if a unit can be invisible and then possibly invisible units wouldn't be counted. This mean possibly invisible units can't block plotgroups, which we can then tell in the GUI. It would make sense that a spy will not block trade as it will blow their cover.

This mean we can make shortcuts to plotgroup updates. Say if a unit leaves a plot, then plotgroups will only update if the player has no more units in the plot and the unit and plot has different owners. Otherwise we know plotgroups will not update from this action. No need to check. Similar shortcuts can be used when entering plots. We can likely use the cache for other stuff as well.
 
I finished writing the code to handle domestic sales across plotgroups. It is still not 100% done, but the remaining work is minor. The remaining work is primarily GUI related. I reused the message meaning you will be informed that the sales took place in a psedo-random city. Most often it is the city you built first. The sales are done to the price already stored in your player. I didn't really into where it is used, but when selling in Europe the prices are stored here. However Europe prices are stored in the king player, which makes me believe that the normal player (you) this info is allocated, but unused in vanilla.

The sales work like this:
  1. the game calculates the combined yields offered for sale, demand and cap for all cities in plotgroup.
  2. the game then loops though all yields to find the highest priced yield, which is both in demand and for sale.
  3. sell whatever is found in #2
  4. goto 2 to find a new one if there is still cap remaining and something was sold in #3
  5. add profit to player
  6. loop though cities to remove the sold yields (it was sold from the combined storage in #3)
This way if the cap prevents selling what you could be able to sell, then it will sell the highest priced yields rather than the yields with the lowest IDs.

I tried to increase performance by not looping though all yields each time. It loops from the lowest to the highest yield IDs being sold, meaning it can loop from 20 to 23 instead of 0 to 35. It could be worth considering grouping sellable yields. That would actually also be a good idea for the GUI.

The sales doesn't register anywhere meaning they don't count towards sold yields or provide trade points. This is likely to change eventually.
 
I have been trying the version before the Bitmaps commit. If it is of interest for you, I found that the Excomunication bug still appears; and the bug about “City lacks plotgroup” also reoccurs in turn 200 or so,:

Spoiler :

Assert Failed
File: CvPlayer.cpp
Line: 19451
Expression: pPlotGroup != NULL
Message: city lacks a plotgroup


There is also another bug not included in the bugs reports:

Spoiler :

Assert Failed
File: CvGameTextMgr.cpp
Line: 8692
Expression: iBaseModifier == kCity.getBaseYieldRateModifier(eYieldType)
Message: Yield Modifier in setProductionHelp does not agree with actual value


I have tested the trading post sales, but they only sell the goods IN the city, they don’t sell goods stored in other cities within the plotgroup.
 
I finished writing the code to handle domestic sales across plotgroups.

Ok, Nightinggale, I am going to attempt to fill in this part of the game, with the Demands and such. The demands are tied into the above "domestic sales" code right?

I am having trouble remember how this system works. At the moment, for testing purposes, Town Halls demands 5 Trade Goods. But it is not taking any from storage? How again does this system work? I set up Sales in the Custom house...
 
I am having trouble remember how this system works. At the moment, for testing purposes, Town Halls demands 5 Trade Goods. But it is not taking any from storage? How again does this system work? I set up Sales in the Custom house...
You can only sell one yield for every 100 demand you have. This mean you have to connect 20 cities with a demand of 5 each to sell just one unit of trade goods. You write like you thought you could sell 5 units a turn. To do that, set the demand to 500 ;)
 
You can only sell one yield for every 100 demand you have. This mean you have to connect 20 cities with a demand of 5 each to sell just one unit of trade goods. You write like you thought you could sell 5 units a turn. To do that, set the demand to 500 ;)

Agggh, I knew that :)
 
Top Bottom