questions about nosave

[Dwachs]

Following this bug report:

http://forum.simutrans.com/index.php?topic=10289

I have two questions / bug reports about the code:

(1) dings/gebaeude.cc:

Code Select Expand


if( is_factory && (new_offset!=koord(0,0) || ptr.fab->get_besch()->get_haus()->get_tile(0,0,0)==NULL) ) {
...
}

Imho the last check has to be

Code Select Expand


ptr.fab->get_besch()->get_haus()->get_tile(layout,0,0)==NULL

??

(2) imho line 4306 in simworld.cc prevents any attempts to rotate map until it can be saved:

Code Select Expand


nosave_warning = nosave = false; // XXX THIS LINE SHOULD BE DELETED ?!
for( int i=0;  i<4  &&  nosave_warning;  i++  ) { // NOT EXECUTED AS nosave_warning==false
rotate90(); ...
}
// seems not successful
if(nosave_warning) { // NOT EXECUTED AS still nosave_warning==false
// but then we try to rotate until only warnings => some buildings may be broken, but factories should be fine
for( int i=0;  i<4  &&  nosave;  i++  ) {
rotate90(); ...
}
...
}
// only broken buildings => just warn
if(nosave_warning) { // NOT EXECUTED AS still nosave_warning==false
dbg->error( "karte_t::speichern()","Some buildings may be broken by saving!" );
}


[prissi]

On the first topic: I think you are correct. Tile (0,0) is only missing for the old fish swarms. THis it should check for the current layout.

For speichern, I think you are right: The first line simply has to be deleted, or broken maps will be never detected.

[Dwachs]

This nosave logic seems to be broken.

Edit: I committed a fix with r5881.

Now I will try to eliminate this restriction about the (0,0) tile...

Edit2: Attached patch to eliminate the special treatment for (0,0) tile of factories.

Please test! Rotating & saving works like a charm ^^

[TurfIt]

Tile (0,0) is also missing in the pak128 cotton farm. Don't like where the farm was built - just rotate the map, save/reload and presto, the farm has moved!
The edit2 patch seems to fix things.  No more migrating farms.

[prissi]

If you save a factory without a valid (0,0) tile, it will have no connections to any factories delivering/recieving. I.e. nothing will be displayed in its consumers, and nothering in its suppliers (tough consumer will still accept those goods.) Because those factories will be looked up by their coordinate, which is the (0,0) tile after reconstructing them. Thus factories with an empty (0,0) tile must not be saved. Or did someone fixed this?

Similar problem should occur with townhalls without a valid (0,0) tile after rotation.

I would rather recommend to save games should in a non-broken state.

EDIT: not fixed, build a cotton farm rotation two. YOu can built on the empty tile. Then save it, reload => former field (0,0) empty, and rotated to tile (0,0) top position. Sorry, just too tired, migth look at it tomorrow.

[Dwachs]

I did not think about the factory links ...

The patch above saves two coordinates:
(1) one for the (0,0) tile - even if there is not building, this is necessary to build the factory correctly after loading
(2) one coordinate of a building of the factory. This coordinate is returned by get_pos(), so should survive save & reload.
Will have to investigate.

[prissi]

If a factory is bulit without tile(0,0) a dummy is built there. Thus does not happen for a rotated factory. Thus whe it get rebuilded after rotation, the new dummy tile will delete whatever is below. Thus better not saving a factory with tile (0,0) empty.

[Dwachs]

If a factory is bulit without tile(0,0) a dummy is built there. Thus does not happen for a rotated factory. Thus whe it get rebuilded after rotation, the new dummy tile will delete whatever is below. Thus better not saving a factory with tile (0,0) empty.

The patch omits this dummy tile for factories.

I still have to check factory connections and non-rectangular special buildings like headquarters, townhalls, attractions.

A related question: are there objects / pak files out there, where a building has a NULL tile? It seems that

Code Select Expand


haus_besch_t::get_tile(..)

will never return a NULL pointer.

[Dwachs]

Here is a new patch. It completely eliminate the necessity of having a valid (0,0) tile for factories. Savegame version also needs not to be increased.

[Dwachs]

Are there any objections against committing this patch ?

[VS]

None from me, as I have exactly thus broken factory...

[TurfIt]

Looks good. I tested with the pak128 cotton plantation; Loaded/saved multiple times with it built in all four rotations, and with the map being rotated around and around.

[prissi]

I has a problem, when a good is delivered after rotation to a factory that has not a valid tile in that rotation. But that was the case before. (Meaning that all tiles must be the same in all rotations, i.e. L must alsways occupy three tile in L shape.)


**  *    *  **
*   **  **   *