PROJECT: Good Specific Terrain Improvements

[Ekmek]

#if defined (ACTIVISION_ORIGINAL)		
		for(i = 0; i < rec->GetNumCantBuildOn(); i++) {
			if(rec->GetCantBuildOnIndex(i) == cell->GetTerrain()) {
				return FALSE;
			}
		}
	}
	
#else  // Is restricted to code added by E 2-Mar-2005 
		if(rec->IsRestrictedToGood == 0) {
			for(i = 0; i < rec->GetNumCantBuildOn(); i++) {
				if(rec->GetCantBuildOnIndex(i) == cell->GetTerrain()) {
					return false;
				}
			}
		} 
		else {
			sint32 good;
			if (g_theWorld->GetGood(pos, good)) {
				for(i = 0; i < rec->GetNumIsRestrictedToGood(); i++) {
					if(rec->GetIsRestrictedToGood(i) == good) {
							return true; 
					}  
				}	 
			return false;
			}
}
#endif

[Ekmek]

Martin, is this finished yet?

[Martin Gühmann]

Originally posted by E
Martin, is this finished yet?

Not quite, first I cannot see what is under and above the block of code you posted. Last time there was a closing brace missing right above the final return, outside of the precompiler block. Now you have one too much in the ACTIVISION_ORIGINAL block.

And then you have to get the indention right, the return true; inside your for-loop has one tab too much and the return false; under the for-loop has one tab too less.

Other than that it seems to be OK.

-Martin

[Martin Gühmann]

OK let's start about the zip file: It contains a folder called Ctp2_code, if you unzip it on a Linux system you have a problem. A Unix file system is case sensitive and the consequence is Ctp2_code != ctp2_code. Well that doesn't matter when you submit it to the repository. Another thing I wonder about is why in the archive are two copies of each *.cdb, same file, same directory, same size.

unit.cdb seems to be ok, but why is SettleImprovement outcommented. However if it wouldn't be outcommented then there would be a problem, a unit could only settle one definite type of improvement, or do you want to add some random effects? Otherwise the is no sense to make it an array. By the EnableAdvance isn't an array, either.

terrimprove.cbd is not OK.

Let's start with the harmless stuff. If you add Wonder and Urban to Class you should also add it to Excludes. You don't know what modders want to do.

What's that:

Record Resource[0] PlantGood //added by E for future imp that places a good instead
Record Resource[1] PlantGood //added by E for future imp that places a good instead
Record Resource[2] PlantGood //added by E for future imp that places a good instead
Record Resource[3] PlantGood //added by E for future imp that places a good instead
Record Resource[4] PlantGood //added by E for future imp that places a good instead

I thought you wanted to use an integer, that can have the values 0-3 indicating the index in the according terrain database array.

That are are Resource database record and this array notation is probably invalid syntax.

Then what do you want to do here? Do you want to allow a maximum of four goods to that a tileimp can be restricted to? Why not all goods in the database?

Record Resource[0..4] IsRestrictedToGood //added by E -- can only build imps if tile has good

wonder.cdb is ok.
advance.cdb seems to be compilable. However CantTrade and CantCapture suggests that normally advances can trade. This is of course wrong they should be CantBeTraded and CantBeCaptured.

And Bridges is at the wrong place it, belongs into the tileimp.txt and should be something like BridgeAdvance, like EnablingAdvance. Probably mot every modder likes it that you need the same advance for building bridges on grassland and for building bridges in the swamp.

Now to the source files. But only in brief for today.
wonder_util.cpp

Spend the description of wonderutil_IsAvailable a conluding new free line. So that it looks better. The description what you have done at the start of the file is missing.

The code for CityStyleOnly should be modelled like the one above for GovernmentType, otherwise the CityStyleOnly precedes CultureOnly only.

m_government_type is not a member of wonderutil, it isn't possible wonderutil isn't a class. And there aren't global, either. The same holds for m_founder and GetCivilisation.

terrainutil.cpp isn't correct either, check the description at the top of the file, check the description of the function you have modified. Get the indention right, and insert at the right place a closing brace, you have erased in the meantime.

So that must be enough for today. It is late and I should go to bed.

-Martin

[Ekmek]

Looking at cdb I saw stuff like this:

	Struct SlaveRaidsData {
		Float Chance, DeathChance
		Int Timer, Amount
		Record Sound Sound
		Record SpecialEffect Effect
	}
	Bit(Struct SlaveRaidsData) SlaveRaids

Do the Bit make it optional. I'd like to add a bit for the POW code and create unit x number of turns.

I know a lot of this has been SLIC'd but if its easier for modders I think its better and it allows SLIC time to be spent on newer ideas.

I'm working on fixing the code and postingit soon (bytheway not sure why my zip duplicated the cdbs I though it wouldn't show)

[Ekmek]

Originally posted by Martin Gühmann
. The description what you have done at the start of the file is missing.

m_government_type is not a member of wonderutil, it isn't possible wonderutil isn't a class. And there aren't global, either. The same holds for m_founder and GetCivilisation.

terrainutil.cpp isn't correct either, check the description at the top of the file, check the description of the function you have modified. Get the indention right, and insert at the right place a closing brace, you have erased in the meantime.

So that must be enough for today. It is late and I should go to bed.

-Martin

Should I add an Init_Player like in player.cpp to establish the m_govt_type OR should I use the global g_player to get info about the player and then compare it?

I think I got the rest...

[Martin Gühmann]

OK, let's see and have a look on the *.cdb files:

advace.cdb is OK.
building.cdb is OK.
wonder.cdb is OK.
government.cdb is OK.

Now to the problematic files:

terrimprove.cdb:

	Struct Effect {
		Record Terrain[] Terrain
		Bit(Int) BonusFood
		Bit(Int) BonusProduction
		Bit(Int) BonusGold
		Bit(Int) MoveCost
		#Added by Martin Gühmann for Trade redirection
		Bit(Int) Freight
		Bit Radar
		Bit CantPillage //futureUse added by E to prevent pillage of certain imps 
		Bit NeedsWorker //futureUse - can't build unless IsWorker unit is on square
		Bit NeedsWaterSupply //futureUse -- can only build on rivers or adjacent to other IsWaterSupply imps
		Bit Airport
		Bit ListeningPost
		Bit Endgame
		Bit(Int) VisionRange
		Bit(Int) RadarRange
		Bit(Float) DefenseBonus
		Record Resource[] PlantGood //added by E for future imp that places a good instead
		Record Advance EnableAdvance
		Record Advance[] ObsoleteAdvance
		Int ProductionCost
		Int ProductionTime
		Int TilesetIndex
	}

Now to the problematic line:

		Record Resource[] PlantGood //added by E for future imp

How many goods do you want to plant on a certain spot. 1, 2, 20, all you can find in the database? I guess this is a little bit utopic. A spot can only contain only one good, so it must not be an array.

Second problem, what happens if you specify a good that you can't plant on the spot. Should the game crash? Or should nothing happen, should the resources spent for the tileimp be wasted? Of course neither should the game crash nor should the PW wasted.

So again PlantGood should be an integer and not a database entry in the good database, nor should be an array in of entries in the good database. It should be just a plain integer like ProductionCost, ProductionTime or TilesetIndex. But in comparision to those it should be optional like BonusFood, BonusProduction or BonusGold.

It should now be obvious how this line should look.

To the next wrong line in this file:

	Record Sound Sound
	Record Terrain[] CantBuildOn
	Record Government[] GovernmentsModified
	Record CityStyle[] CultureOnly //added by E -- can only build imps if civ has certain citystyle
	Record Government[] GovernmentType //added by E --- a copy of unit Government type code
	Record Resource[0..4] IsRestrictedToGood //added by E -- can only build imps if tile has good
	Bit(Struct Effect) Effect
	Struct Effect[] TerrainEffect

The line in particular:

	Record Resource[0..4] IsRestrictedToGood //added by E -- can only build imps if tile has good

Come on you don't want to restrict the maximum number of goods to that a terrain improvement can be restricted to four. It should be a free array like GovernmentType, CultureOnly or CantBuildOn.

Now let's come to unit.cdb:

	Record Advance             EnableAdvance
	Record Advance[0..5]        ObsoleteAdvance
	Record Government[] GovernmentsModified
	Record Unit[] UpgradeTo //FutureUse to create Upgrade Paths, cost based on difference ShieldCost x Gold?
	Record Terrain[] CanSettleOn //added by E restricts settling to certain terrains
	Record CityStyle[] CultureOnly //added by E -- can only build units if civ has certain citystyle
	Record CityStyle[] CityStyleOnly //added by E--can only build units at cities with certain citystyle 
	Record TerrainImprovement[] SettleImprovement // future use - for units to make specific imps like forts

The first line that is wrong is:

	Record Unit[] UpgradeTo //FutureUse to create Upgrade Paths, cost based on difference ShieldCost x Gold?

UpgradeTo is an array, but you cannot upgrade one unit to one unit type on the one hand and to another different unit type at the same time. For instance a unit can't be a settler and a diplomat at the same time. That's impossible. However you rethink it yourself anyways. But again this is a single unit record like EnableAdvance is a single advance record. And therefore it isn't an array, either.

Well wether SettleImprovement should be an array of records or an single record is a design issure.

The following flag name is missleading:

	Bit(Int) CostPopToBuild  //added for future use

It reminds me of BuildingRemovesAPop, probably a better name would be PopCostsToBuild, maybe PopCosts is enough to make it clear.

That was the *.cdb part.

In Player.cpp, the function Player::CanBuildUnit is not correct, you forgot to remove the first ACTIVISION_ORIGINAL right at the start of the function, it even hasn't a coresponding matching #endif.

CityData.cpp:

//----------------------------------------------------------------------------
//
// Name       : CityData::CanBuildBuilding
//
// Description: Checks whether the city can build the building specified by 
//              type.
//
// Parameters : type: The building type for that is checked whether the city  
//              can build it.
//
// Globals    : g_player:        The list of players
//              g_theBuildingDB: The building database
//              g_slicEngine:    The slic engine
//              g_theWorld:      The world prperties
//
// Returns    : Whether the city can build the building specified by type.
//
// Remark(s)  : CityStyleOnly added by E. Limits certain buildings to be built  
//              only at certain cities of certain styles.
//              GovernmentType flag for Buidings limits Buildings to govt type.
//              CultureOnly flag added by E. It allows only civilizations with 
//              the same CityStyle as CultureOnly's style to build that building.
//----------------------------------------------------------------------------
BOOL CityData::CanBuildBuilding(sint32 type) const
{
	if(g_exclusions->IsBuildingExcluded(type))
		return FALSE;

    const BuildingRecord* irec = g_theBuildingDB->Get(type);
	
	
	Assert(irec != NULL);
	if (!irec) 
		return FALSE;

	if(!g_player[m_owner]->HasAdvance(irec->GetEnableAdvanceIndex()) && irec->GetEnableAdvanceIndex() >= 0) {
		return FALSE;
	}

	sint32 o;
	for(o = 0; o < irec->GetNumObsoleteAdvance(); o++) {
		if(g_player[m_owner]->HasAdvance(irec->GetObsoleteAdvanceIndex(o)))
			return FALSE;
	}
	
	MapPoint pos;
	m_home_city.GetPos(pos);
	if(g_theWorld->IsWater(pos)) {
		if(irec->GetCantBuildInSea())
			return FALSE;
	} else {
		if(irec->GetCantBuildOnLand())
			return FALSE;
	}
	
	
	if (irec->GetCoastalBuilding()) {
		if(!g_theWorld->IsNextToWater(pos.x, pos.y))
			return FALSE;
	}

	if(m_built_improvements & uint64((uint64)1 << (uint64)type)) {
		
		return FALSE;
	}

	if((irec->GetNuclearPlant() &&
	   wonderutil_GetNukesEliminated(g_theWonderTracker->GetBuiltWonders()))) {
		return FALSE;
	}

	
	if(irec->GetNumPrerequisiteBuilding() > 0) {
		for(o = 0; o < irec->GetNumPrerequisiteBuilding(); o++) {
			sint32 b = irec->GetPrerequisiteBuildingIndex(o);
			if(!(GetEffectiveBuildings() & ((uint64)1 << (uint64)b)))
				return FALSE;
		}
	}

#if !defined(ACTIVISION_ORIGINAL)

	if(irec->GetNumGovernmentType() > 0) {
		sint32 i;
		bool found = false;
		for(i = 0; i < irec->GetNumGovernmentType(); i++) {
			if(irec->GetGovernmentTypeIndex(i) == m_government_type) {
				found = true;
				break;
			}
		}
		if(!found)
			return FALSE;
	}

	if(irec->GetNumCityStyleOnly() > 0) {
		sint32 s;
		for(s = 0; s < irec->GetNumCityStyleOnly(); s++) {
			if(irec->GetCityStyleOnlyIndex(s) == g_player[m_founder]->m_citystyle) {
				return TRUE;
			}
		}
		return FALSE
	}

	if(irec->GetNumCultureOnly() > 0) {
		sint32 s;
		for(s = 0; s < irec->GetNumCultureOnly(); s++) {
			if(irec->GetCultureOnlyIndex(s) == GetCivilisation()->GetCityStyle()) {
				return TRUE;
			}
		}
		return FALSE;
	}
#endif	

	return g_slicEngine->CallMod(mod_CanCityBuildBuilding, TRUE, m_home_city.m_id, irec->GetIndex());
}

In this function we have a typo in the description of the function. In the line of the g_theWorld. Just because I saw it and "prperties" is also one of my favourites. And so I realize that it is actual my typo. But fix it anyway. Now to the more important stuff.

The code about CityStyleOnly should be modelled like the one for GovernmentType.

Again m_government_type is a member of player. So you need the accoring function of the Player class. The same hold for GetCivilisation. The according player is the owner of the city. And for the CityStyleOnly part you have to compare it against the city style of the city not against the city style of the founder. The city style of the city itsself can be changed by the cheat editor. The same is true for the code in the CanBuildUnit code and there as well the code should be modelled like the code of GovernmentType.

Well that isn't everthing, but for today it is enough.

-Martin

[Ekmek]

[SIZE=1] Originally posted by Martin Gühmann
The code about CityStyleOnly should be modelled like the one for GovernmentType.

Again m_government_type is a member of player. So you need the accoring function of the Player class. The same hold for GetCivilisation. The according player is the owner of the city. And for the CityStyleOnly part you have to compare it against the city style of the city not against the city style of the founder. The city style of the city itsself can be changed by the cheat editor. The same is true for the code in the CanBuildUnit code and there as well the code should be modelled like the code of GovernmentType.

Well that isn't everthing, but for today it is enough.

-Martin

I understood most of your correections. As for UpgradeTo, I leave it as you suggested but as I dug into the cdbs again I think it might later be better at a "struc" so the moder could define how much the upgrade cost is as opposed to adding a calculation (and it could make the cost in PW and/or gold)

It was a bit unclear to me about "function of the player class" kind of new terms to me but I took a stab at it. Like wise the modelling after govt type, not sure which you wanted modelled but I look for the differences and changed to what govt type had, but I don't really understand what the difference is. anyways here's the code:

#if !defined(ACTIVISION_ORIGINAL)

	if(irec->GetNumGovernmentType() > 0) {
		sint32 i;
		bool found = false;
		for(i = 0; i < irec->GetNumGovernmentType(); i++) {
			if(irec->GetGovernmentTypeIndex(i) == g_player[m_owner]->m_government_type) {
				found = true;
				break;
			}
		}
		if(!found)
			return FALSE;
	}

	if(irec->GetNumCityStyleOnly() > 0) {
		sint32 s;
		for(s = 0; s < irec->GetNumCityStyleOnly(); s++) {
			if(irec->GetCityStyleOnlyIndex(s) == g_player[m_founder]->m_citystyle) {
				found = true;
				break;
			}
		}
		if(!found)
			return FALSE;
	}

	if(irec->GetNumCultureOnly() > 0) {
		sint32 s;
		for(s = 0; s < irec->GetNumCultureOnly(); s++) {
			if(irec->GetCultureOnlyIndex(s) == g_player[m_owner]->GetCivilisation()->GetCityStyle()) {
				found = true;
				break;
			}
		}
		if(!found)
			return FALSE;
	}
#endif

[Martin Gühmann]

Well, I suppose the piece of code belongs into the function I have posted above.

I don't check this now deeply. But the part about CityStyleOnl is still wrong. First the Player class does not has a m_CityStyle member. And getting the founder's style isn't a good idea, either. Since I can place cities with the cheat editor of whatever style I whish.

So again you have to use the m_cityStyle member of the CityData class. For the CityData class check the citydata.h and there you find the keyword class and behind it CityData. In brief a class is some kind of container. It contains fields like m_cityData and methods like AddShieldsToBuilding. In the past I mixed up the terms function and method. Maybe you can define methods as member functions. But we don't have to go to deeply into the termology.

OK, and another aspect. To be precise a class is not a container but the contruction plan of such a container.

And an object is an instace of such a class.

Now each city has its CityData the container of all the pieces of information about a concret city. And of course its methods to manipulate the pieces of information.

-Martin

[Ekmek]

Martin, I checked out the *.h files for citydata and player and found the m_citystyle (in citydata.h) and m_civilization (in player.h). Now that I can find where to get the m_ stuff do you have an example for it so I know what it should look like or is the government type a good enough example?


also would it be something like g_CivilizationData to use these classes in CivilzationData.h:

//----------------------------------------------------------------------------
// Class declarations
//----------------------------------------------------------------------------


#endif	// _MSC_VER

class CivilisationData : public GAMEOBJ
	{
	public:
		
		

		PLAYER_INDEX	m_owner ;									

		uint8			m_cityname_count[k_MAX_CITY_NAMES] ;		
		
		CIV_INDEX		m_civ ;										

		GENDER			m_gender;

		sint32			m_cityStyle;								

		MBCHAR			m_leader_name[k_MAX_NAME_LEN],
                        m_personality_description[k_MAX_NAME_LEN],
						m_civilisation_name[k_MAX_NAME_LEN],
						m_country_name[k_MAX_NAME_LEN],
						m_singular_name[k_MAX_NAME_LEN] ;

I'm mainly interested in m_civilisation_name and m_citystyle

[Martin Gühmann]

Originally posted by E
Martin, I checked out the *.h files for citydata and player and found the m_citystyle (in citydata.h) and m_civilization (in player.h). Now that I can find where to get the m_ stuff do you have an example for it so I know what it should look like or is the government type a good enough example?

As it is a member variable you can use the this pointer to access it, if you are inside of a member function of CityData in this case.

The syntax would be something like this (explicitely):

this->m_citystyle;

Implicitely it is just:

m_citystyle;

The way as it is done in all the files.

Originally posted by E
also would it be something like g_CivilizationData to use these classes in CivilzationData.h:

//----------------------------------------------------------------------------
// Class declarations
//----------------------------------------------------------------------------


#endif	// _MSC_VER

class CivilisationData : public GAMEOBJ
	{
	public:
		
		

		PLAYER_INDEX	m_owner ;									

		uint8			m_cityname_count[k_MAX_CITY_NAMES] ;		
		
		CIV_INDEX		m_civ ;										

		GENDER			m_gender;

		sint32			m_cityStyle;								

		MBCHAR			m_leader_name[k_MAX_NAME_LEN],
                        m_personality_description[k_MAX_NAME_LEN],
						m_civilisation_name[k_MAX_NAME_LEN],
						m_country_name[k_MAX_NAME_LEN],
						m_singular_name[k_MAX_NAME_LEN] ;

I'm mainly interested in m_civilisation_name and m_citystyle

I think the stuff you are looking for is rather called g_theCivilisationDB of class something, and it should have a method to retrieve the StringID of the civ name, and once you have this you can compare it with the entry in Unit.txt easily.

-Martin

[Ekmek]

Martin so i should have something like this for the citystyle:

g_citydata->m_citystyle

I haven't seen a g_citydata in the code before...

[Martin Gühmann]

Originally posted by E
Martin so i should have something like this for the citystyle:

g_citydata->m_citystyle

I haven't seen a g_citydata in the code before...

No, this doesn't work. And there is no g_citydata either. Or any other global CityData object. Each city has its own CityData object, if you are inside such an object you can simply use m_citystyle. If you are outside of a CityData member function you have to get such an CityData object first from that you can retrieve the m_citydata then.

-Martin

[Ekmek]

hmmm...
ok a little confused on how to grab objects (unless its GetCity)

I think your saying that I have to use a GetCity()->m_citystyle to get inside the object and then use the m_array stuff.

I'll check my C++ for dummies. Any good examples in the code where I could identify the differences in object, class, function

This is good stuff you may want to add it to your C++ Lecture....

[Martin Gühmann]

Originally posted by E
I think your saying that I have to use a GetCity()->m_citystyle to get inside the object and then use the m_array stuff.

If you are in a method belonging to the CityData object you don't need anything with Get or this. So first complete the CityData::CanBuildBuilding method and of course the *.cdb files.

Originally posted by E
This is good stuff you may want to add it to your C++ Lecture....

Maybe I should continue this lecture.

-Martin