A good reason to disable city electric supply until you can get it working right

[neroden]

(1) It's potentially impossible to supply a city if it's contained inside another city.
(2) It's potentially impossible to give correct electricity supply to a factory directly if it's inside a city; you are forced to supply the city whether you want to or not
(3) It is possible for a factory to decide that it's located inside a small city which is contained inside another city.
(4) Therefore, it may be possible for a factory to be completely unsuppliable.

[jamespetts]

Looking at the code, this is, as far as I can discern, impossible. The reason for this is that this requires a factory built in a location that is within the city boundaries of two cities be assigned to one of those two cities, whereas a substation built in a location that is within the city boundaries of those two same cities is assigned to the other of those two cities.

In fact, the assignment of a city to each of those two objects uses the same method, which is deterministic:

Code Select Expand


stadt_t *karte_t::get_city(const koord pos) const
{
stadt_t* city = NULL;
if(pos == koord::invalid)
{
return NULL;
}

if(ist_in_kartengrenzen(pos))
{
for (weighted_vector_tpl<stadt_t*>::const_iterator i = stadt.begin(), end = stadt.end(); i != end; ++i)
{
stadt_t* c = *i;
if(c->is_within_city_limits(pos))
{
city = c;
}
}
}
return city;
}


So, if there is a factory in a city within a city, then either that factory will belong to the outer city, or, if it belongs to the inner city, any substation built in the inner city will also belong to the inner city, and not the outer city.

A useful modification to the get_city method would be to make sure that the inner city is always returned when faced with a city within a city situation.

Edit: How's this:

Code Select Expand


stadt_t *karte_t::get_city(const koord pos) const
{
stadt_t* city = NULL;
if(pos == koord::invalid)
{
return NULL;
}

if(ist_in_kartengrenzen(pos))
{
uint16 cities = 0;
for (weighted_vector_tpl<stadt_t*>::const_iterator i = stadt.begin(), end = stadt.end(); i != end; ++i)
{
stadt_t* c = *i;
if(c->is_within_city_limits(pos))
{
cities ++;
if(cities > 1)
{
// We have a city within a city. Make sure to return the *inner* city.
if(city->is_within_city_limits(c->get_pos()))
{
// "c" is the inner city: c's town hall is within the city limits of "city".
city = c;
}
}
else
{
city = c;
}
}
}
}
return city;
}


?

[neroden]

In fact, the assignment of a city to each of those two objects uses the same method, which is deterministic:

OK, that's good....

A useful modification to the get_city method would be to make sure that the inner city is always returned when faced with a city within a city situation.

Edit: How's this:

I thought of more corner cases where it won't quite work right -- notably two cities each of which has its townhall inside the other.  (Your proposed code would choose the later of the two in the list in that case, but the earlier of the two in the list in the case of two overlapping cities which don't contain each other.... which has the potential for debugging confusion.  The actual corner case which fails is the one with two cities, each of which has its townhall inside the other, *and* the "excess area" of one of them outside the other is completely bounded by a third city.

Eventually we'll want to do something like that, but I think it requires further thought.  For now let's leave the deterministic choice the way it is -- but add a comment explaining the choice, like this:

Code Select Expand


for (weighted_vector_tpl<stadt_t*>::const_iterator i = stadt.begin(), end = stadt.end(); i != end; ++i)
{
// Deterministically chose the *last* city in the list which meets the requirements.
stadt_t* c = *i;


[jamespetts]

In fact, the new code (which I have integrated in -devel) is still deterministic: in a case where city A's city hall is contained in city B's limits, and city B's city hall is contained in city A's limits, then a square that is within both cities' limits will be registered as belonging to city B (on the assumption that the order in the vector is alphabetical). This code does, however, have the added advantage that, when a city is entirely encased in another city, it will always be accessible for electricity supply purposes.

[neroden]

in a case where city A's city hall is contained in city B's limits, and city B's city hall is contained in city A's limits, then a square that is within both cities' limits will be registered as belonging to city B (on the assumption that the order in the vector is alphabetical). This code does, however, have the added advantage that, when a city is entirely encased in another city, it will always be accessible for electricity supply purposes.

Yes, it's an improvement.  I still will have to get around to that city merger code some day.... but I'm trying to clean up integer type conversions right now.