On Sat, 4 Feb 2017 00:32:30 +0100 [email protected] wrote: > > I do not have a strong opinion there. Generally though its best to keep > > the map editor minimal, as it is rare for anyone to work on it:-). > > I meanwhile think its actually more about preventing malformed data from > being saved. > Some parts of the code already try to prevent it, but there are a few > loopholes which I now know need to be prevented from allowing broken rivers. > One example is removing a river tile and the neighbours keeping broken > connections.
Ugh. Yes, that is probably worth preventing. BTW I just got to playing about with the map editor and made some rivers. I did not have any problems, and it is definitely less aggravating to use than the older design. Glad you got to that. > > > > the compatibility code > > It looks like a bug to me to call updateRiverConnections and I guess > also updateRoadConnections from the compatibility code inside the > checkIntegrity method, because they make changes to the data when > they might not be allowed to, as the fix parameter is only checked later > for some other changes. I agree that sounds buggy. IIRC the semantics of updateRiverConnections changed a few times and probably desynced with its use in the integrity checker. > I wonder if caching the connections really does improve performance > and if TileImprovement.connected could be removed to simplify the code? Put up a patch or pull req and I will add it to the overnight test queue. Looking at it again, I am hopeful the effect will be hard to measure. Definitely a good thing to have got away from a hex string representation. > There is all kinds of other entanglement like putting together a > style string, updating the connections, then converting it back > to a string, then into TileImprovementStyle class, all inside > a loop repeatedly changing the same tiles from different directions. > If you select a whole rectangle of tiles in the map editor to > add or remove rivers or change their styles, it loops over the tiles > one tile changes its neighbours, the loop continues and they get > changes again leading to chaotic unpredictable changes to the > river styles. Everything would need to be turned inside out to > first add all improvements, then add missing connections to all > tiles styles, then revome superfluous/broken connections on all tiles, > then update the connection cache, to have predictable results > (it would also avoid doing much of the work many times), but that > would require changing much code. That may even have come up at the time. Once again, IIRC I think we were just relieved to have something that worked, improved the save format, and dealt with the backwards compatibility. Appetite for further thrashing was limited by a desire to push forward to a release. > If more places would use TileImprovementStyle instead of strings > containing a style it would improve the code. The class could then > internally handle manipulating the style strings, instead of many > different places knowing the format of these strings. I believe that was the intent. As you note, the work was incomplete. Cheers, Mike Pope
pgp4CnXd9E6hz.pgp
Description: OpenPGP digital signature
------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________ Freecol-developers mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/freecol-developers
