PROJECT: Good Specific Terrain Improvements

[Ekmek]

Ok, I think I got all of your editing...

		sint32 good;
		if (g_theWorld->GetGood(pos, good)){
		for(i = 0; i < rec->GetNumIsRestrictedToGood(); i++) {
			if(rec->GetIsRestrictedToGood(i) == good) {
				return false; 
			}
		}
}
	
		for(i = 0; i < rec->GetNumCantBuildOn(); i++) {
			if(rec->GetCantBuildOnIndex(i) == cell->GetTerrain()) {
				return false;
			}
		}
	}
	return true;
}

[Martin Gühmann]

Well, actual I didn't tell you anything about replacing the return true; statement inside your for-loop by a return false; statement. I told you something else. To be precise it was about adding an return false; after your for-loop but still inside your if-block.

Now we have to think about the logic, precisely how should this work. Should it work like this that the original code is executed if there is no IsRestrictedToGood defined. Or should it executed so that the terrain must match and the good must match if IsRestrictedToGood is defined?

-Martin

[Ekmek]

		sint32 good;
		if (g_theWorld->GetGood(pos, good)){
		for(i = 0; i < rec->GetNumIsRestrictedToGood(); i++) {
			if(rec->GetIsRestrictedToGood(i) == good) {
				return true; 
			}  return false; 
		}
}
	
		for(i = 0; i < rec->GetNumCantBuildOn(); i++) {
			if(rec->GetCantBuildOnIndex(i) == cell->GetTerrain()) {
				return false;
			}
		}
	}
	return true;
}

Ok, I think i understood that we added a return false to apply only to the if statement, hadnt seen that before so I might have goofed it.

As for the logic, I tend to say which is easier to do?

But if its about the same effort I'd tend towards:
"the original code is executed if there is no IsRestrictedToGood defined"

[Martin Gühmann]

Originally posted by E
As for the logic, I tend to say which is easier to do?

Well that's not the question here. The question is what makes the best sense.

Originally posted by E
But if its about the same effort I'd tend towards:
"the original code is executed if there is no IsRestrictedToGood defined"

And otherwise your code is executed. I think this makes more sense as well. It is stupid to restrict it to a good that appears only on terrain on that you cannot construct the tileimprovement itsself.

First you have to change the order of your code and NumCantBuildOn code. I think the NumCantBuildOn is executed more often. Then surround the original code with an if-block and your code with an else-block. And the condition for your if is that the number of IsRestrictedToGood is zero or less. (Just to cover all possible and nonpossible cases. )

-Martin

[Ekmek]

Ok, I'll give it a shot...

//----------------------------------------------------------------------------
//
// Name       :  CanPlayerBuildAt
//
// Description:  Checks terrain improvement properties to see if it can build on a tile only if it has a good
//
// Parameters :  sint32 pl		: Variable for player
//		const MapPoint &pos	: Variable for tile on map
//		const TerrainImprovementRecord *rec	:Varriable for improvement flag
//		sint32 good	: Variable for good				
//
// Globals    :   g_theWorld		: The game world properties
//		      g_player   	: Player properties 
//
// Returns    :   BOOL			: Returns TRUE if an improvement can be built on a tile
//						  FALSE if the improvement cannot 
//
// Remark(s)  :   a new improvement attribute IsRestrictedToGood was added by E.
//		      Modders will define this in tileimp.txt as IsRestrictedToGood: X.  
//		      The flag adds an additional option in order to restrict ceratin improvements to goods, adding new options and bonuses. 
//              
//
//----------------------------------------------------------------------------
bool terrainutil_CanPlayerBuildAt(const TerrainImprovementRecord *rec, sint32 pl, const MapPoint &pos)
{
	sint32 i;

	Assert(rec != NULL);
	if(rec == NULL)
		return false;

	Assert(pl >= 0);
	Assert(pl < k_MAX_PLAYERS);
	if(pl < 0 || pl >= k_MAX_PLAYERS)
		return false;

	Assert(g_player[pl]);
	if(!g_player[pl])
		return false;

	Cell *cell = g_theWorld->GetCell(pos);
	Assert(cell);
	if(!cell)
		return false;

	if(cell->GetOwner() == -1) {
		if(rec->GetIntBorderRadius()) {
			if(!g_player[pl]->IsVisible(pos)) {
				
				return false;
			}
		} else {
			return false;
		}
	}

	if(cell->GetOwner() >= 0 && cell->GetOwner() != pl)
	{
#if defined(ACTIVISION_ORIGINAL)	// Non-standard syntax		
		bool haveAlliance = AgreementMatrix.s_agreements.HasAgreement(pl, cell->GetOwner(), PROPOSAL_TREATY_ALLIANCE);
#else
		bool const haveAlliance	= 
			AgreementMatrix::s_agreements.HasAgreement(pl, cell->GetOwner(), PROPOSAL_TREATY_ALLIANCE);
#endif
		if(cell->GetOwner() > 0 && haveAlliance) {
			if(rec->GetClassRoad() ||
				(g_player[pl]->GetGaiaController() && g_player[pl]->GetGaiaController()->GaiaControllerTileImp(rec->GetIndex()))) {
				
				
			} else {
				return false;
			}
		} else {
			return false;
		}
	}

	
	if(g_theWorld->GetCity(pos).IsValid())
		return false;
		
	if(rec->GetClassTerraform()) {
		sint32 terr;
		if(!rec->GetTerraformTerrainIndex(terr))
			return false;

		if(cell->GetTerrain() == terr)
			return false;

		const TerrainRecord *tfrom = g_theTerrainDB->Get(cell->GetTerrain());
		const TerrainRecord *tto = g_theTerrainDB->Get(terr);
		if(tfrom->GetRemoveAdvanceIndex() < 0 || tto->GetAddAdvanceIndex() < 0)
			return false;

		if(!g_player[pl]->HasAdvance(tfrom->GetRemoveAdvanceIndex()) ||
			!g_player[pl]->HasAdvance(tto->GetAddAdvanceIndex())) {
			return false;
		}
	} else {
		
		const TerrainImprovementRecord::Effect *eff;
		eff = terrainutil_GetTerrainEffect(rec, cell->GetTerrain());
		if(!eff)
			return false;
		
		if(!g_player[pl]->HasAdvance(eff->GetEnableAdvanceIndex()))
			return false;
		
		sint32 a;
		for(a = 0; a < eff->GetNumObsoleteAdvance(); a++) {
			if(g_player[pl]->HasAdvance(eff->GetObsoleteAdvanceIndex(a))) {
				return false;
			}
		}
#if defined(ACTIVISION_ORIGINAL)	// Non-standard syntax
		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
		for(i = 0; i < rec->GetNumCantBuildOn(); i++) {
			if(rec->GetCantBuildOnIndex(i) == cell->GetTerrain()) {
				return false;
			}
		}
	}
		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
	return true;
}

[Martin Gühmann]

OK, let's start the function's name is terrainutil_CanPlayerBuildAt and not CanPlayerBuildAt and it returns a bool and not a BOOL.

Then there is no parameter called good.

The first word after Remark(s): should be capitalized as it is the case with all the other colon instances.

Replace all the tabs by spaces in the comment block so that the appearance is the same in all editors with equal distance letters.

Now to the code, actual I didn't tell you to add the preprocessor derectives, yet. But this is needed anyway until we have our server in a good condition.

Remove this // Non-standard syntax comment, as the original code is standart syntax, at least it looks like. Then the last closing brace is one too much in the #if-#else-block, move it outside just above the final return.

Then this line is wrong in your last version it was right:

for([b]i > 0[/b]; i < rec->GetNumIsRestrictedToGood(); i++) {

You should get the indention right so that you see where you miss a brace and where is one too much.

And finally put the new but original code into an if-block and your code into an else-block.

And for the condition of the if read this thread, it is not only in my last post.

-Martin

[Ekmek]

Ok, I in corporated your changes....

//----------------------------------------------------------------------------
//
// Name       :  terrainutil_CanPlayerBuildAt
//
// Description:  Checks terrain improvement properties to see if it can build on a tile only if it has a good
//
// Parameters :  sint32 pl		: Variable for player
//		const MapPoint &pos	: Variable for tile on map
//		const TerrainImprovementRecord *rec	:Varriable for improvement flag
// Globals    :   g_theWorld		: The game world properties
//		      g_player   	: Player properties 
//
// Returns    :   bool			: Returns true if an improvement can be built on a tile
//						  false if the improvement cannot 
//
// Remark(s)  :   A new improvement attribute IsRestrictedToGood was added by E.
//		      Modders will define this in tileimp.txt as IsRestrictedToGood: X.  
//		      The flag adds an additional option in order to restrict ceratin improvements to goods, adding new options and bonuses. 
//              
//
//----------------------------------------------------------------------------
bool terrainutil_CanPlayerBuildAt(const TerrainImprovementRecord *rec, sint32 pl, const MapPoint &pos)
{
	sint32 i;

	Assert(rec != NULL);
	if(rec == NULL)
		return false;

	Assert(pl >= 0);
	Assert(pl < k_MAX_PLAYERS);
	if(pl < 0 || pl >= k_MAX_PLAYERS)
		return false;

	Assert(g_player[pl]);
	if(!g_player[pl])
		return false;

	Cell *cell = g_theWorld->GetCell(pos);
	Assert(cell);
	if(!cell)
		return false;

	if(cell->GetOwner() == -1) {
		if(rec->GetIntBorderRadius()) {
			if(!g_player[pl]->IsVisible(pos)) {
				
				return false;
			}
		} else {
			return false;
		}
	}

	if(cell->GetOwner() >= 0 && cell->GetOwner() != pl)
	{
#if defined(ACTIVISION_ORIGINAL)			
		bool haveAlliance = AgreementMatrix.s_agreements.HasAgreement(pl, cell->GetOwner(), PROPOSAL_TREATY_ALLIANCE);
#else
		bool const haveAlliance	= 
			AgreementMatrix::s_agreements.HasAgreement(pl, cell->GetOwner(), PROPOSAL_TREATY_ALLIANCE);
#endif
		if(cell->GetOwner() > 0 && haveAlliance) {
			if(rec->GetClassRoad() ||
				(g_player[pl]->GetGaiaController() && 

g_player[pl]->GetGaiaController()->GaiaControllerTileImp(rec->GetIndex()))) {
				
				
			} else {
				return false;
			}
		} else {
			return false;
		}
	}

	
	if(g_theWorld->GetCity(pos).IsValid())
		return false;
		
	if(rec->GetClassTerraform()) {
		sint32 terr;
		if(!rec->GetTerraformTerrainIndex(terr))
			return false;

		if(cell->GetTerrain() == terr)
			return false;

		const TerrainRecord *tfrom = g_theTerrainDB->Get(cell->GetTerrain());
		const TerrainRecord *tto = g_theTerrainDB->Get(terr);
		if(tfrom->GetRemoveAdvanceIndex() < 0 || tto->GetAddAdvanceIndex() < 0)
			return false;

		if(!g_player[pl]->HasAdvance(tfrom->GetRemoveAdvanceIndex()) ||
			!g_player[pl]->HasAdvance(tto->GetAddAdvanceIndex())) {
			return false;
		}
	} else {
		
		const TerrainImprovementRecord::Effect *eff;
		eff = terrainutil_GetTerrainEffect(rec, cell->GetTerrain());
		if(!eff)
			return false;
		
		if(!g_player[pl]->HasAdvance(eff->GetEnableAdvanceIndex()))
			return false;
		
		sint32 a;
		for(a = 0; a < eff->GetNumObsoleteAdvance(); a++) {
			if(g_player[pl]->HasAdvance(eff->GetObsoleteAdvanceIndex(a))) {
				return false;
			}
		}
#if defined(ACTIVISION_ORIGINAL)	// Non-standard syntax
		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
		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 [b]>[/b] rec->GetNumIsRestrictedToGood(); i++) {
							if(rec->GetIsRestrictedToGood(i) == good) {
								return true; 
							}  return false; 
						}
					}
			}
	}
			
#endif
	return true;
}

I think I got the BOLD part right. Not sure I understood the less than your looking for. Also I think I added an else like you said was needed...

[Martin Gühmann]

Originally posted by E
I think I got the BOLD part right. Not sure I understood the less than your looking for.

Well that's true for the bold part, that wasn't so difficuilt, but for rest... Just to make it clear, you have to type the code on your own.

Originally posted by E
Also I think I added an else like you said was needed...

I didn't say that an else was needed inside of the for-loop, I told you to move your part of the new code into an else-block and the original part into an corresponding if-block.

And since I have so often repeated the condition of this if, I leave it up to you to reread the according posts.

And two other things I told you as well were to remove this second Non-standard syntax comment and to move the final closing brace inside the #if-#else-block just below of this block.

OK again in four point:

1. if-else, original code - your code
2. Figure out the condition of the if.
3. Remove second "// Non-standard syntax" comment
4. Closing brace

-Martin

[Ekmek]

//----------------------------------------------------------------------------
//
// Name       :  terrainutil_CanPlayerBuildAt
//
// Description:  Checks terrain improvement properties to see if it can build on a tile only if it has a good
//
// Parameters :  sint32 pl		: Variable for player
//		const MapPoint &pos	: Variable for tile on map
//		const TerrainImprovementRecord *rec	:Varriable for improvement flag
// Globals    :   g_theWorld		: The game world properties
//		      g_player   	: Player properties 
//
// Returns    :   bool			: Returns true if an improvement can be built on a tile
//						  false if the improvement cannot 
//
// Remark(s)  :   A new improvement attribute IsRestrictedToGood was added by E.
//		      Modders will define this in tileimp.txt as IsRestrictedToGood: X.  
//		      The flag adds an additional option in order to restrict ceratin improvements to goods, adding new options and bonuses. 
//              
//
//----------------------------------------------------------------------------
bool terrainutil_CanPlayerBuildAt(const TerrainImprovementRecord *rec, sint32 pl, const MapPoint &pos)
{
	sint32 i;

	Assert(rec != NULL);
	if(rec == NULL)
		return false;

	Assert(pl >= 0);
	Assert(pl < k_MAX_PLAYERS);
	if(pl < 0 || pl >= k_MAX_PLAYERS)
		return false;

	Assert(g_player[pl]);
	if(!g_player[pl])
		return false;

	Cell *cell = g_theWorld->GetCell(pos);
	Assert(cell);
	if(!cell)
		return false;

	if(cell->GetOwner() == -1) {
		if(rec->GetIntBorderRadius()) {
			if(!g_player[pl]->IsVisible(pos)) {
				
				return false;
			}
		} else {
			return false;
		}
	}

	if(cell->GetOwner() >= 0 && cell->GetOwner() != pl)
	{
#if defined(ACTIVISION_ORIGINAL)			
		bool haveAlliance = AgreementMatrix.s_agreements.HasAgreement(pl, cell->GetOwner(), PROPOSAL_TREATY_ALLIANCE);
#else
		bool const haveAlliance	= 
			AgreementMatrix::s_agreements.HasAgreement(pl, cell->GetOwner(), PROPOSAL_TREATY_ALLIANCE);
#endif
		if(cell->GetOwner() > 0 && haveAlliance) {
			if(rec->GetClassRoad() ||
				(g_player[pl]->GetGaiaController() && 

g_player[pl]->GetGaiaController()->GaiaControllerTileImp(rec->GetIndex()))) {
				
				
			} else {
				return false;
			}
		} else {
			return false;
		}
	}

	
	if(g_theWorld->GetCity(pos).IsValid())
		return false;
		
	if(rec->GetClassTerraform()) {
		sint32 terr;
		if(!rec->GetTerraformTerrainIndex(terr))
			return false;

		if(cell->GetTerrain() == terr)
			return false;

		const TerrainRecord *tfrom = g_theTerrainDB->Get(cell->GetTerrain());
		const TerrainRecord *tto = g_theTerrainDB->Get(terr);
		if(tfrom->GetRemoveAdvanceIndex() < 0 || tto->GetAddAdvanceIndex() < 0)
			return false;

		if(!g_player[pl]->HasAdvance(tfrom->GetRemoveAdvanceIndex()) ||
			!g_player[pl]->HasAdvance(tto->GetAddAdvanceIndex())) {
			return false;
		}
	} else {
		
		const TerrainImprovementRecord::Effect *eff;
		eff = terrainutil_GetTerrainEffect(rec, cell->GetTerrain());
		if(!eff)
			return false;
		
		if(!g_player[pl]->HasAdvance(eff->GetEnableAdvanceIndex()))
			return false;
		
		sint32 a;
		for(a = 0; a < eff->GetNumObsoleteAdvance(); a++) {
			if(g_player[pl]->HasAdvance(eff->GetObsoleteAdvanceIndex(a))) {
				return false;
			}
		}
#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){
					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 = -1; i > rec->GetNumIsRestrictedToGood(); i++) {
				if(rec->GetIsRestrictedToGood(i) == good) {
					return true; 
				}  return false; 
			}
		}
	}
			
#endif
	return true;
}

I think where I'm confsed on your if-else statement is that I'm not clear if you are asking for a #if and #else or an if and else for the code. It just didn't come across if its a comment thing or a coding thing.

But I'm hoping I got you this time.

[Martin Gühmann]

Originally posted by E
But I'm hoping I got you this time.

No, you didn't got me.

Originally posted by E
I think where I'm confsed on your if-else statement is that I'm not clear if you are asking for a #if and #else or an if and else for the code. It just didn't come across if its a comment thing or a coding thing.

Again the precompiler statements start all with a #. The precompiler is a text replacement tool or in this case a text selection tool. That means if the condition behind the #if is true the following text is used otherwise it is cut out and the text following the #else is used.

Now do the following: Form two versions of the following code fragment one that the preprocessor generates if the statement defined(ACTIVISION_ORIGINAL) is true and one if this statement is false. And finally post these two code fragments. You have to get this!

#if defined(ACTIVISION_ORIGINAL)	
		for(i = 0; i < rec->GetNumCantBuildOn(); i++) {
			if(rec->GetCantBuildOnIndex(i) == cell->GetTerrain()) {
				return false;
			}
		}
	[b]}[/b]
#else   // Is restricted to code added by E 2-Mar-2005

		sint32 good;
		if (g_theWorld->GetGood(pos, good)){
			for(i = [b]-1[/b]; i [b]>[/b] rec->GetNumIsRestrictedToGood(); i++) {
				if(rec->GetIsRestrictedToGood(i) == good) {
					return true; 
				}  return false; 
			}
		}
	[b]}[/b]
			
#endif
	return true;
}

Now let's come to the code that is selected by the preprocessor and finally compiled:

I marked some stuff here. Last time I told you to move out the final closing brace in the coresponding preprocessor blocks under the preprocessor derectives. This time I made them bold. This is just duplicated code and actual not really changed.

Now I marked two other things that were already all right. The first one is the -1, fortunatly the GetIsRestrictedToGood has a guard against invalid array access, otherwise you would crash the game. Unfortunatly returns the function in this case 0 and that is not so good if the good on the according map the first one in the good database.

The second one is this bigger operator. Well you didn't turn this into an endless loop, but it will run for a long time, not a very good idea. To make this clear I never have told you modify any for-loops.

Now again what we want to do, if the number of IsRestrictedToGood is 0 then the original code should be executed. Otherwise your new code has to be executed. That means you have to use an if-statement and an else-statement. And don't mix these up with the preprocessor derectives they start with a #.

And now I am more precise as I actual intended. You should do something like this:

#if defined(ACTIVISION_ORIGINAL)
//Original code no need to change it
//Except the final closing brace
#else
		if(/*Condition you have to figure out and it was given often enough in this thread*/){
			//Original code
		}
		else{
			//Your new code
		}
#endif

So here again in brief I copy the remaining things from the last list:

1. if-else, original code - your code
2. Figure out the condition of the if.
3. Closing brace
4. Fix the stuf you have broken.
5. And do the preprocessor execise.

[Ekmek]

//----------------------------------------------------------------------------
//
// Name       :  terrainutil_CanPlayerBuildAt
//
// Description:  Checks terrain improvement properties to see if it can build on a tile only if it has a good
//
// Parameters :  sint32 pl		: Variable for player
//		const MapPoint &pos	: Variable for tile on map
//		const TerrainImprovementRecord *rec	:Varriable for improvement flag
// Globals    :   g_theWorld		: The game world properties
//		      g_player   	: Player properties 
//
// Returns    :   bool			: Returns true if an improvement can be built on a tile
//						  false if the improvement cannot 
//
// Remark(s)  :   A new improvement attribute IsRestrictedToGood was added by E.
//		      Modders will define this in tileimp.txt as IsRestrictedToGood: X.  
//		      The flag adds an additional option in order to restrict ceratin improvements to goods, adding new options and bonuses. 
//              
//
//----------------------------------------------------------------------------
bool terrainutil_CanPlayerBuildAt(const TerrainImprovementRecord *rec, sint32 pl, const MapPoint &pos)
{
	sint32 i;

	Assert(rec != NULL);
	if(rec == NULL)
		return false;

	Assert(pl >= 0);
	Assert(pl < k_MAX_PLAYERS);
	if(pl < 0 || pl >= k_MAX_PLAYERS)
		return false;

	Assert(g_player[pl]);
	if(!g_player[pl])
		return false;

	Cell *cell = g_theWorld->GetCell(pos);
	Assert(cell);
	if(!cell)
		return false;

	if(cell->GetOwner() == -1) {
		if(rec->GetIntBorderRadius()) {
			if(!g_player[pl]->IsVisible(pos)) {
				
				return false;
			}
		} else {
			return false;
		}
	}

	if(cell->GetOwner() >= 0 && cell->GetOwner() != pl)
	{
#if defined(ACTIVISION_ORIGINAL)			
		bool haveAlliance = AgreementMatrix.s_agreements.HasAgreement(pl, cell->GetOwner(), PROPOSAL_TREATY_ALLIANCE);
#else
		bool const haveAlliance	= 
			AgreementMatrix::s_agreements.HasAgreement(pl, cell->GetOwner(), PROPOSAL_TREATY_ALLIANCE);
#endif
		if(cell->GetOwner() > 0 && haveAlliance) {
			if(rec->GetClassRoad() ||
				(g_player[pl]->GetGaiaController() && 

g_player[pl]->GetGaiaController()->GaiaControllerTileImp(rec->GetIndex()))) {
				
				
			} else {
				return false;
			}
		} else {
			return false;
		}
	}

	
	if(g_theWorld->GetCity(pos).IsValid())
		return false;
		
	if(rec->GetClassTerraform()) {
		sint32 terr;
		if(!rec->GetTerraformTerrainIndex(terr))
			return false;

		if(cell->GetTerrain() == terr)
			return false;

		const TerrainRecord *tfrom = g_theTerrainDB->Get(cell->GetTerrain());
		const TerrainRecord *tto = g_theTerrainDB->Get(terr);
		if(tfrom->GetRemoveAdvanceIndex() < 0 || tto->GetAddAdvanceIndex() < 0)
			return false;

		if(!g_player[pl]->HasAdvance(tfrom->GetRemoveAdvanceIndex()) ||
			!g_player[pl]->HasAdvance(tto->GetAddAdvanceIndex())) {
			return false;
		}
	} else {
		
		const TerrainImprovementRecord::Effect *eff;
		eff = terrainutil_GetTerrainEffect(rec, cell->GetTerrain());
		if(!eff)
			return false;
		
		if(!g_player[pl]->HasAdvance(eff->GetEnableAdvanceIndex()))
			return false;
		
		sint32 a;
		for(a = 0; a < eff->GetNumObsoleteAdvance(); a++) {
			if(g_player[pl]->HasAdvance(eff->GetObsoleteAdvanceIndex(a))) {
				return false;
			}
		}
#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){
		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 = 1; i [b]>[/b] rec->GetNumIsRestrictedToGood(); i++) {
							if(rec->GetIsRestrictedToGood(i) == good) {
								return true; 
							}  return false; 
						}
					}
			}
	}
			
#endif
	return true;
                }

checking on the preprocesor stuff

[Martin Gühmann]

First thing you have to still to do the preprocessor exercise. Then the else block is again inside the for-loop of the original code. Then you didn't fix your for-loop, well you removed the - in front of the 1. So you start now to check the second element of the good's list. Not a good idea especially if this list just contain one element. Then you didn't replaced that > operator by an smaller operator to restore the state you already had. The function still needs ages to check whether the condition is right.

Then the final closing brace in the preprocessor blocks is still there and not outside, I even marked it last time.

Well at least you added the if in comparision to the correspnding else at the right place. But the condition is wrong. You should check whether the number of the elements of IsRestrictedToGood is 0, that can't be so difficuilt to get this you use the right function already in your code, and you got it right for the culture only stuff.

And finally or better to this first, get the indention right, that helps you as well.

OK, here the list in brief:

1. Indention
2. Do the preprocessor exercise
3. Move out the closing brace that is too much in the preprocessor blocks
4. Find the correct condition for the if
5. Put the else block onto the right level and correct the indention first!
6. Fix the for loop, it does'n start at one and it is a smaller than a greater

-Martin

[Ekmek]

oops wrong thread...

[Ekmek]

Taking another crack at this...

//----------------------------------------------------------------------------
//
// Name       :  terrainutil_CanPlayerBuildAt
//
// Description:  Checks terrain improvement properties to see if it can build on a tile only if it has a good
//
// Parameters :  sint32 pl		: Variable for player
//		const MapPoint &pos	: Variable for tile on map
//		const TerrainImprovementRecord *rec	:Varriable for improvement flag
// Globals    :   g_theWorld		: The game world properties
//		      g_player   	: Player properties 
//
// Returns    :   bool			: Returns true if an improvement can be built on a tile
//						  false if the improvement cannot 
//
// Remark(s)  :   A new improvement attribute IsRestrictedToGood was added by E.
//		      Modders will define this in tileimp.txt as IsRestrictedToGood: X.  
//		      The flag adds an additional option in order to restrict ceratin improvements to goods, adding new options and bonuses. 
//              
//
//----------------------------------------------------------------------------
bool terrainutil_CanPlayerBuildAt(const TerrainImprovementRecord *rec, sint32 pl, const MapPoint &pos)
{
	sint32 i;

	Assert(rec != NULL);
	if(rec == NULL)
		return false;

	Assert(pl >= 0);
	Assert(pl < k_MAX_PLAYERS);
	if(pl < 0 || pl >= k_MAX_PLAYERS)
		return false;

	Assert(g_player[pl]);
	if(!g_player[pl])
		return false;

	Cell *cell = g_theWorld->GetCell(pos);
	Assert(cell);
	if(!cell)
		return false;

	if(cell->GetOwner() == -1) {
		if(rec->GetIntBorderRadius()) {
			if(!g_player[pl]->IsVisible(pos)) {
				
				return false;
			}
		} else {
			return false;
		}
	}

	if(cell->GetOwner() >= 0 && cell->GetOwner() != pl)
	{
#if defined(ACTIVISION_ORIGINAL)			
		bool haveAlliance = AgreementMatrix.s_agreements.HasAgreement(pl, cell->GetOwner(), PROPOSAL_TREATY_ALLIANCE);
#else
		bool const haveAlliance	= 
			AgreementMatrix::s_agreements.HasAgreement(pl, cell->GetOwner(), PROPOSAL_TREATY_ALLIANCE);
#endif
		if(cell->GetOwner() > 0 && haveAlliance) {
			if(rec->GetClassRoad() ||
				(g_player[pl]->GetGaiaController() && 

g_player[pl]->GetGaiaController()->GaiaControllerTileImp(rec->GetIndex()))) {
				
				
			} else {
				return false;
			}
		} else {
			return false;
		}
	}

	
	if(g_theWorld->GetCity(pos).IsValid())
		return false;
		
	if(rec->GetClassTerraform()) {
		sint32 terr;
		if(!rec->GetTerraformTerrainIndex(terr))
			return false;

		if(cell->GetTerrain() == terr)
			return false;

		const TerrainRecord *tfrom = g_theTerrainDB->Get(cell->GetTerrain());
		const TerrainRecord *tto = g_theTerrainDB->Get(terr);
		if(tfrom->GetRemoveAdvanceIndex() < 0 || tto->GetAddAdvanceIndex() < 0)
			return false;

		if(!g_player[pl]->HasAdvance(tfrom->GetRemoveAdvanceIndex()) ||
			!g_player[pl]->HasAdvance(tto->GetAddAdvanceIndex())) {
			return false;
		}
	} else {
		
		const TerrainImprovementRecord::Effect *eff;
		eff = terrainutil_GetTerrainEffect(rec, cell->GetTerrain());
		if(!eff)
			return false;
		
		if(!g_player[pl]->HasAdvance(eff->GetEnableAdvanceIndex()))
			return false;
		
		sint32 a;
		for(a = 0; a < eff->GetNumObsoleteAdvance(); a++) {
			if(g_player[pl]->HasAdvance(eff->GetObsoleteAdvanceIndex(a))) {
				return false;
			}
		}
#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 [b]
		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[/b]
	return true;
}

I was going to put: if(rec->IsRestrictedToGood > 0) like in the culture code, but I re-read one of your lines that said if IsRestrictedToGood is 0 then it should execute the original. I'm not sure you intended me to put an '=' there to do it, butI put it out there to find out why its '=' or if it should be a '>' like the other code how does it execute the first code only?

[Martin Gühmann]

Originally posted by E
I was going to put: if(rec->IsRestrictedToGood > 0) like in the culture code, but I re-read one of your lines that said if IsRestrictedToGood is 0 then it should execute the original.

Actual I said something about the number of IsRestrictedToGood that's something else. I thought it is obviously in order to get the number of IsRestrictedToGood, to use the function that has the first syllable of number in its name.

Originally posted by E
I'm not sure you intended me to put an '=' there to do it, butI put it out there to find out why its '=' or if it should be a '>' like the other code how does it execute the first code only?

Actual I neither intended you to put an '=' nor a '>' but a '=='. First is an assignment the second is a bigger and the third is an equal.

The first operator assignes the value of the right variable to the left variable. Or in other words the value of the left variable becomes the value of the right variable.

Bigger is an operator that checks whether the right variable is bigger than the left variable. If it is the case it returns true otherwise false. And the third the equal checks whether the varaiables on both sides have (or contain) the same value(s).

The last two operators return a boolean value, true or false.

Since we have here an alternative, on the one hand the code that is present in the original and on the other hand the code code you wrote, we have to do a decission, between the codes.

Internally the IsRestrictedToGood items are stored in an array. So what happens if there is not a single IsRestrictedToGood defined. The answer is that array is empty and its size is therefore zero.

Now two the code:
First thing is that you changed the preprocessor derectives, as we want to replace here original code we have to go with the #if-#else-#endif derectives. As it was in the last version. Well you removed the closing brace that was too much inside the preprocessor blocks, but you didn't add it outside just above the final return.

Then the indention is not fixed.

Another thing I just notice is that the return false inside your code is at the wrong place. It is inside the for-loop but, it should be outside, just right below the for-loop. So move it down one line.

And you have to edit the description, especially you have to add some line breaks and replace the tabs by spaces. For the line breaks orientate you on the lines with the: ---- The text should not end to much behind the end of the ------.

-Martin

-Martin