PROJECT: ShowOnMap

[Martin Gühmann]

So you got it now. Now we have this piece of code:

MapPoint SpotFound;
SpotFound.IsValid();

So what does return IsValid() in this case?

-Martin

[Martin Gühmann]

So E, what is the problem to find the implementation of MapPoint::IsValid and to check what it returns if the object has the default values? That is a task of five minutes, at most.

-Martin

[Ekmek]

bool MapPoint::IsValid(void) const
{
	return (g_theWorld->IsXwrap() || ((x >= 0) && (x < g_mp_size.x)))
	    && (g_theWorld->IsYwrap() || ((y >= 0) && (y < g_mp_size.y)))
#if !defined(_SMALL_MAPPOINTS)
	    && (z >= 0) && (z < g_mp_size.z)
#endif
	    ;
}

its a bool that sees if mappoint is greater than or equal to zero.


so I guess in the case of spotfound since its default value is 0,0 it will always return true

[Martin Gühmann]

So E, how do you have to initialize SpotFound so that IsValid returns false?

-Martin

[Ekmek]

SpotFound = NULL?

[Martin Gühmann]

No, look again on the IsValid method and give me a set of values for x and y that makes the method return false. That is not so difficuilt.

-Martin

[Ekmek]

Originally posted by Martin Gühmann
No, look again on the IsValid method and give me a set of values for x and y that makes the method return false. That is not so difficuilt.

-Martin

MapPoint SpotFound (-1,-1);

[Martin Gühmann]

You got it, after three months.

By the way what was the code into which this belongs?

-Martin

[Ekmek]

citydata::addwonder

yeah i have to make sure i didnt miss anything so i'll re-read the thread later.

[Martin Gühmann]

Maybe you just look for the pieces of code and read the stuff around, in this way you are much faster to find the relevant stuff like your latest code. So that you can repost it of course with the right initialization of SpotFound.

-Martin

[Ekmek]

I didn't get a chance to recompile and test it yet. Unfortunately RL is starting to eat at my CtP2 time. Anyways this is what I was going to test:

	MapPoint SpotFound(-1,-1);
	CityInfluenceIterator it(point, GetVisionRadius());  //m_sizeIndex 
	
	sint32 s;
	for(s = 0; s < rec->GetNumShowOnMap(); s++) {
	const TerrainImprovementRecord *trec = g_theTerrainImprovementDB->Get(s);
	

		for(it.Start(); !it.End(); it.Next()) {
			Cell *ncell = g_theWorld->GetCell(it.Pos());   //it.Pos  now it.point?
			Cell *ocell = g_theWorld->GetCell(SpotFound);
			if(point == it.Pos())
				continue;

			if(terrainutil_CanPlayerSpecialBuildAt(trec, m_owner, it.Pos())){ 
				if ((!SpotFound.IsValid())
					||(ncell->GetNumDBImprovements() > ocell->GetNumDBImprovements())  
					||(ncell->GetGoldFromTerrain() > ocell->GetGoldFromTerrain()) 
					){
					SpotFound = it.Pos(); 
				}
			}
		}
		g_player[m_owner]->CreateSpecialImprovement(rec->GetShowOnMapIndex(s), SpotFound, 0);
	}

But I want to test a few things I want to make sure it doesn't "layer" wonders on top of each other. I also would like wonders to be permanent features that cant be built on/over. I know we discussed before about removing a wonder after the city is destroyed but I'm inclined to leave it because it will serve as a ruin since the wonder abilities would be gone and you'll just be left with a tileimp that makes extra gold.

[Ekmek]

	MapPoint SpotFound(-1,-1);
	CityInfluenceIterator it(point, GetVisionRadius());  //m_sizeIndex 
	
	sint32 s;
	for(s = 0; s < rec->GetNumShowOnMap(); s++) {
	const TerrainImprovementRecord *trec = g_theTerrainImprovementDB->Get(s);
	

		for(it.Start(); !it.End(); it.Next()) {
			Cell *ncell = g_theWorld->GetCell(it.Pos());   //it.Pos  now it.point?
			Cell *ocell = g_theWorld->GetCell(SpotFound);
			if(point == it.Pos())
				continue;

			if(terrainutil_CanPlayerSpecialBuildAt(trec, m_owner, it.Pos())){ 
				if ((!SpotFound.IsValid())
					||(ncell->GetNumDBImprovements() > ocell->GetNumDBImprovements())  
					||(ncell->GetGoldFromTerrain() > ocell->GetGoldFromTerrain()) 
					){
					SpotFound = it.Pos(); 
				}
			}
		}
		g_player[m_owner]->CreateSpecialImprovement(rec->GetShowOnMapIndex(s), SpotFound, 0);
	}

well I did this and when i tested it did nothing. the logic seems right and I followed the pseudocode. Is it because there are two many 'OR' conditions? I don't think it should be but when I added an addition spotfound line outside the bracket (in the case all the conditions are false ) it kind of worked. Well it worked like it did before. We have to be missing something

[Martin Gühmann]

Maybe you should also post the pseudo code so that we can compare. However your code doesn't look right but that is a little bit hard to see if the indentiation is not right. So that means that is the first thing you have to do before we continue. If you have problems with the whitespace go to the menu bar in MSVS click there on Edit -> Advanced -> View Whitespace and then get it right.

And by the way don't blame the forum for the incorrect display. I saw this in the code you have submitted and actually I fixed it too often for you. So it is not the forum but it is you!

-Martin

[Ekmek]

	// EMOD Visible wonders 4-25-2006
	MapPoint SpotFound(-1,-1);
	CityInfluenceIterator it(point, GetVisionRadius());  //m_sizeIndex GetVisionRadius()
	
	sint32 s;
	for(s = 0; s < rec->GetNumShowOnMap(); s++) {
		const TerrainImprovementRecord *trec = g_theTerrainImprovementDB->Get(s);
		for(it.Start(); !it.End(); it.Next()) {
			Cell *ncell = g_theWorld->GetCell(it.Pos());   //it.Pos  now it.point?
			Cell *ocell = g_theWorld->GetCell(SpotFound);
			if(point == it.Pos())
				continue;

			if(terrainutil_CanPlayerSpecialBuildAt(trec, m_owner, it.Pos())){ 
				if ((!SpotFound.IsValid())
				||(ncell->GetNumDBImprovements() > ocell->GetNumDBImprovements())  
				||(ncell->GetGoldFromTerrain() > ocell->GetGoldFromTerrain()) 
				){
					SpotFound = it.Pos(); 
				}
				SpotFound = it.Pos(); 
			}
		}
		g_player[m_owner]->CreateSpecialImprovement(rec->GetShowOnMapIndex(s), SpotFound, 0);
	}

//for all tiles in the city radius do
//    if current tile allows building of wonder imp do
//        if no good tile has been found yet
//        or current tile is better than found tile do
//            foundTile := currentTile
//        end
//    end
//end

the pseudocode you posted before is outcommented at the botoom. I fixed the indenting, but if its still wrong then maybe I'm not understanding an indention rule somewhere

[Martin Gühmann]

Well the indenting looks fine now.

Now you do something like this:

for all tiles in the city radius do
    if current tile allows building of wonder imp do
        if no good tile has been found yet
        or current tile is better than found tile do
            foundTile := currentTile
        end
        foundTile := currentTile
    end
end

That's stupid, because why doing the inner if stuff at all.

So far I think the pseudocode is ok, but some implementation details are missing, that result in a small change of the pseudo code:

for all tiles in the city radius do

    if current tile is city tile do // Do this first before you do anything else
        continue

    if current tile allows building of wonder imp do
        if a good tile has been found do

            Get new cell from currentTile
            Get old cell from foundTile

            if currentTile has less tileimps than foundTile do
                foundTile := currentTile
            end
        else do // No good tile has been found yet
            foundTile := currentTile
        end
    end
end

Well we should start with just one criterion that says us whether a given tile is better than another tile. We can still add criteria. However gold output isn't such a criterion.

-Martin