[Freecol-developers] River styles

[Michael T. Pope]

On Fri, 10 Feb 2017 22:56:50 +0100
***@genial.ms wrote:
> I upgraded the code for checking roads/rivers have no style problems,
> as I had looked at it so much recently and the compat code is going
> to be upgraded soon.
> Its at: https://github.com/wintertime/FreeCol/tree/cleanup-improvements
> I tried to get it to work correctly, but doublechecking and more testing
> would be useful, as I always avoided touching the serialization code
> until now.

Will test.

> I also pushed a commit fixing null style rivers into the FreeCol repo,
> after I found some with the updated code inside one of my own saves.
> I also used your savegame for testing for all kinds of river/road problems
> and there is null roads, rivers with broken connection, rivers
> with 0000 style, but no rivers with null style. :)
> I dont really need another save with null style rivers, but I'm not sure
> if null style rivers actually existed or if I could simplify that code?

I am not sure either now. The change to river styles was not my code.
Later when trying to fix the broken maps I recall being deeply confused
about what used to be valid, so I tried to be generous in what we read
(so potentially including nonexisting cases), and conservative in what we
wrote.

> In my own save I also found a few null roads were duplicated, is that a bug?

I think so, but not a serious one.

> I only saw that when checking the output from the updated readAttributes
> for correctness in the logfile. It looks like other parts of the game
> might not be accessing the duplicate, as it keeps the null style and gets
> resaved that way. I'll attach the save and log file.
> Examples you can search for:
> tile:3139
> tile:9240

Which save game? I do not see any duplicates in the attached one for tile:3139.

> Btw., I could not get the integrity checker to output something useful:
> $ ant validate-savegame -Dsavegame=../../freecol/save/14e3a40e_NiederlÀndisch_1508.fsg

OK, that one is validating the game against the schema. I will look into
what is happening there. However that is a different thing from the
integrity checker, as in the checkIntegrity stuff in the code, which I get
to with the --check-savegame argument on the FreeCol command line.

Cheers,
Mike Pope

[Michael T. Pope]

On Sun, 12 Feb 2017 12:02:55 +1030
"Michael T. Pope" <***@computer.org> wrote:
> > Btw., I could not get the integrity checker to output something useful:
> > $ ant validate-savegame -Dsavegame=../../freecol/save/14e3a40e_NiederlÀndisch_1508.fsg
>
> OK, that one is validating the game against the schema. I will look into
> what is happening there.

Simple omission in the schema. Fixed in git.40feb43. Your game now
passes schema-validation.

Meanwhile, the integrity tester fails due to bad tile improvements, but the
log shows that the fixes worked[1]. The same applies to the game attached
to BR#3025 --- do you know what the problem was there?

Cheers,
Mike Pope

[1] git.94c72d7 improves the --check-savegame messages to distinguish the
failed-but-fixed case and to point to the log file.

[w***@genial.ms]

Hi,

> Gesendet: Sonntag, 12. Februar 2017 um 03:16 Uhr
> Von: "Michael T. Pope" <***@computer.org>
> An: freecol-***@lists.sourceforge.net
> Betreff: Re: [Freecol-developers] River styles
>
> On Sun, 12 Feb 2017 12:02:55 +1030
> "Michael T. Pope" <***@computer.org> wrote:
> > > Btw., I could not get the integrity checker to output something useful:
> > > $ ant validate-savegame -Dsavegame=../../freecol/save/14e3a40e_Niederländisch_1508.fsg
> >
> > OK, that one is validating the game against the schema. I will look into
> > what is happening there.
>
> Simple omission in the schema. Fixed in git.40feb43. Your game now
> passes schema-validation.

Sorry, but for me (git.70910a33) not:
$ ant validate-savegame -Dsavegame=../../freecol/save/14e3a40e_Niederländisch_1508.fsg
Buildfile: C:\Users\Sebastian\Documents\NetBeansProjects\FreeCol\build.xml

init:

compile:

build:

validate-savegame:
[java] Processing file ..\..\freecol\save\14e3a40e_Niederl▒ndisch_1508.fsg
[java] cvc-complex-type.2.4.a: Ung▒ltiger Content wurde beginnend mit Element 'serverGame' gefunden. '{serverPlayer, serverUnit, serverIndianSettlement}' wird erwartet. at line=15 column=350

BUILD SUCCESSFUL
Total time: 1 second


> Meanwhile, the integrity tester fails due to bad tile improvements, but the
> log shows that the fixes worked[1]. The same applies to the game attached
> to BR#3025 --- do you know what the problem was there?
>
> [1] git.94c72d7 improves the --check-savegame messages to distinguish the
> failed-but-fixed case and to point to the log file.

You mean later than git.f3288756 ? TileImprovement.checkIntegrity is
relying on update[Road/River]Style methods to fix something and only
reports something if these changed something.
That means it could not report problems earlier, but null roads exist
in nearly all savegames, which means 99.99% of all savegames will
show "failed, but fixed" now (unless someone exterminated all Indians,
removing the unconnected virtual roads inside their settlements,
and all European roads have a neighbouring road to connect to,
including virtual ones for colonies).
You can pretty much ignore all messages with
"WARNING: Fixing improvement style from null to 00000000 at "
for the moment. I assume there was some other problem in BR#3025,
but more information on it would have been nice.
You could try the branch from my github on some savegames, as it
catches and prints a few other problems with the improvement styles,
but most likely will __not__ catch the problem from BR#3025, too.

Theres the philosophical question left on if the null rivers were
a bug in updateRoadStyle or just the accepted old savegame format.
The old message treats it as a bug now, but one might argue that
if its just been the normal save format it could just silently
fix it until 0.13.0.


Greetings,

wintertime

[w***@genial.ms]

Hi,

> Gesendet: Sonntag, 12. Februar 2017 um 02:32 Uhr
> Von: "Michael T. Pope" <***@computer.org>
> An: ***@genial.ms
> Cc: "FreeCol Developers" <freecol-***@lists.sourceforge.net>
> Betreff: Re: [Freecol-developers] River styles
>
> Will test.

Nice.

> I am not sure either now. The change to river styles was not my code.
> Later when trying to fix the broken maps I recall being deeply confused
> about what used to be valid, so I tried to be generous in what we read
> (so potentially including nonexisting cases), and conservative in what we
> wrote.

I remember getting some NPE when I tried removing a null check in the
display code a few months ago, but that might have been caused by
the checking having replaced 0000 with null. I guess its safer to keep
a null check.

> > In my own save I also found a few null roads were duplicated, is that a bug?
>
> I think so, but not a serious one.
>
> > I only saw that when checking the output from the updated readAttributes
> > for correctness in the logfile. It looks like other parts of the game
> > might not be accessing the duplicate, as it keeps the null style and gets
> > resaved that way. I'll attach the save and log file.
> > Examples you can search for:
> > tile:3139
> > tile:9240
>
> Which save game? I do not see any duplicates in the attached one for tile:3139.

I was going from the log file I had attached, where these tiles appeared
to be read in twice. The savegame name should be in the log and the same
I had attached, 14e3a40e_Niederländisch_1508.fsg - is it not?
I had unpacked the save, looked at and searched the xml, then thought I
had seen 2 road improvements for that tile number inside it (not the
tile itself, the snippets were for searching the log but have the tile
number for the save), but my eyes may have been confused by the
huge pile of run-together characters.

> > Btw., I could not get the integrity checker to output something useful:
> > $ ant validate-savegame -Dsavegame=../../freecol/save/14e3a40e_Niederländisch_1508.fsg
>
> OK, that one is validating the game against the schema. I will look into
> what is happening there. However that is a different thing from the
> integrity checker, as in the checkIntegrity stuff in the code, which I get
> to with the --check-savegame argument on the FreeCol command line.

Ah yes, that makes sense.


Greetings,

wintertime

[Michael T. Pope]

On Sun, 12 Feb 2017 12:02:55 +1030
"Michael T. Pope" <***@computer.org> wrote:
On Fri, 10 Feb 2017 22:56:50 +0100
***@genial.ms wrote:
> I upgraded the code for checking roads/rivers have no style problems,
> as I had looked at it so much recently and the compat code is going
> to be upgraded soon.
> Its at: https://github.com/wintertime/FreeCol/tree/cleanup-improvements
> I tried to get it to work correctly, but doublechecking and more testing
> would be useful, as I always avoided touching the serialization code
> until now.

Ran some tests and could not break anything, including the harder serialization
tests. You are right that a lot of games now have integrity that is
failed-but-fixed, but I think this is the Right Thing to do. We should be
clear as to what format is expected. So this is looking good.

> Sorry, but for me (git.70910a33) not:
> $ ant validate-savegame -Dsavegame=../../freecol/save/14e3a40e_NiederlÀndisch_1508.fsg

Ugh. validate-savegame fails if you mistype the file name:-P. Should be
fixed in git.2500200. Apparently the saved game schema is long neglected.
More fixes there are almost certainly needed.

Cheers,
Mike Pope

[Michael T. Pope]

On Mon, 13 Feb 2017 22:09:57 +1030
"Michael T. Pope" <***@computer.org> wrote:
> Ugh. validate-savegame fails if you mistype the file name:-P.

Some attempts to make validate-savegame return a meaningful error state
went in with git.cb08aa8.

>[Schema fail]
> More fixes there are almost certainly needed.

git.80ba436 does a large update to the schema. I can now successfully
validate every 0.11.x game in my collection of interesting games, plus
all the games attached to open bugs that I have downloaded (except
Fenyo's modded game in BR#2030:-). All 0.10.x annotations and
accompanying code have gone away, or in a few cases been promoted
to 0.11.x where they had not been fully excised before 0.11.0 was
released.

If anyone finds a non-validating 0.11.x game, please attach or let me know
where to find it. Preferrably while the neurons that understand the
schema are still fresh:-). Alternately, if anyone has an interest in
XSD, I found several places where the code and schema had diverged,
and doubt I found all of them where the schema accepts too much, so
other eyes on this would be welcome.

Cheers,
Mike Pope